This series incorporates fixes to the block layout daemon from the last few
weeks of testing.
The first two patches I sent before, and I think they are in one of the tags
but they don't seem to have made it to origin/master.
The rest are split into three categories, disk signature fixes, device
mapping fixes, and cleanups. I would be happy to split these up into
smaller chunks, but I think it's maybe a bit easier to review this way.
Jim Rees (5):
add blkmapd and spnfsd to list of build targets to ignore
Remove blkmapd config file, which is no longer used.
disk signature fixes
various minor cleanups
device mapping fixes
.gitignore | 2 +
utils/blkmapd/device-discovery.c | 17 ++--
utils/blkmapd/device-discovery.h | 11 +-
utils/blkmapd/device-inq.c | 7 +-
utils/blkmapd/device-process.c | 153 +++++++++++++++------------
utils/blkmapd/dm-device.c | 209 +++++++++++++++++++------------------
utils/blkmapd/etc/blkmapd.conf | 10 --
7 files changed, 211 insertions(+), 198 deletions(-)
delete mode 100644 utils/blkmapd/etc/blkmapd.conf
Signed-off-by: Jim Rees <[email protected]>
---
.gitignore | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/.gitignore b/.gitignore
index 4bff9e3..6530ae0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -36,6 +36,7 @@ support/include/stamp-h1
lib*.a
tools/rpcgen/rpcgen
tools/rpcdebug/rpcdebug
+utils/blkmapd/blkmapd
utils/exportfs/exportfs
utils/idmapd/idmapd
utils/lockd/lockd
@@ -48,6 +49,7 @@ utils/rquotad/rquotad
utils/rquotad/rquota.h
utils/rquotad/rquota_xdr.c
utils/showmount/showmount
+utils/spnfsd/spnfsd
utils/statd/statd
tools/locktest/testlk
tools/getiversion/getiversion
--
1.7.1
Use dm_is_dm_major() to detect pseudo devices, and don't add them
If mapped device already exists, possibly from a previous run, increment
device number and try again
plug fd leak in verify_sig
fix null name in dm_device_remove_byname()
small fixes and debug info
Signed-off-by: Jim Rees <[email protected]>
---
utils/blkmapd/device-discovery.c | 16 +++++------
utils/blkmapd/device-discovery.h | 2 +-
utils/blkmapd/device-process.c | 39 ++++++++++++--------------
utils/blkmapd/dm-device.c | 55 +++++++++++++++++++++++++------------
4 files changed, 63 insertions(+), 49 deletions(-)
diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c
index c2675b8..5656be3 100644
--- a/utils/blkmapd/device-discovery.c
+++ b/utils/blkmapd/device-discovery.c
@@ -46,6 +46,7 @@
#include <unistd.h>
#include <libgen.h>
#include <errno.h>
+#include <libdevmapper.h>
#include "device-discovery.h"
@@ -154,10 +155,13 @@ void bl_add_disk(char *filepath)
dev = sb.st_rdev;
serial = bldev_read_serial(fd, filepath);
- ap_state = bldev_read_ap_state(fd);
+ if (dm_is_dm_major(major(dev)))
+ ap_state = BL_PATH_STATE_PSEUDO;
+ else
+ ap_state = bldev_read_ap_state(fd);
close(fd);
- if (ap_state == BL_PATH_STATE_PASSIVE)
+ if (ap_state != BL_PATH_STATE_ACTIVE)
return;
for (disk = visible_disk_list; disk != NULL; disk = disk->next) {
@@ -176,13 +180,6 @@ void bl_add_disk(char *filepath)
BL_LOG_INFO("%s: %s\n", __func__, filepath);
- /*
- * Not sure how to identify a pseudo device created by
- * device-mapper, so leave /dev/mapper for now.
- */
- if (strncmp(filepath, "/dev/mapper", 11) == 0)
- ap_state = BL_PATH_STATE_PSEUDO;
-
/* add path */
path = malloc(sizeof(struct bl_disk_path));
if (!path) {
@@ -308,6 +305,7 @@ int bl_disk_inquiry_process(int fd)
}
head->status = BL_DEVICE_REQUEST_PROC;
+
switch (head->type) {
case BL_DEVICE_MOUNT:
/*
diff --git a/utils/blkmapd/device-discovery.h b/utils/blkmapd/device-discovery.h
index a0937b1..8fe3b29 100644
--- a/utils/blkmapd/device-discovery.h
+++ b/utils/blkmapd/device-discovery.h
@@ -106,7 +106,7 @@ struct pipefs_hdr {
uint32_t msgid;
uint8_t type;
uint8_t flags;
- uint16_t totallen; /* length of entire message, including hdr */
+ uint16_t totallen; /* length of message including hdr */
uint32_t status;
};
diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
index 4ce47e1..0c5060b 100644
--- a/utils/blkmapd/device-process.c
+++ b/utils/blkmapd/device-process.c
@@ -52,10 +52,10 @@
static char *pretty_sig(char *sig, uint32_t siglen)
{
static char rs[100];
- unsigned long i;
+ unsigned long i = 0;
if (siglen <= sizeof i) {
- memcpy(&i, sig, sizeof i);
+ memcpy(&i, sig, siglen);
sprintf(rs, "0x%0lx", i);
} else {
if (siglen > sizeof rs - 1)
@@ -75,7 +75,7 @@ uint32_t *blk_overflow(uint32_t * p, uint32_t * end, size_t nbytes)
return p;
}
-static int decode_blk_signature(uint32_t ** pp, uint32_t * end,
+static int decode_blk_signature(uint32_t **pp, uint32_t * end,
struct bl_sig *sig)
{
int i;
@@ -164,25 +164,27 @@ read_cmp_blk_sig(struct bl_disk *disk, int fd, struct bl_sig_comp *comp)
static int verify_sig(struct bl_disk *disk, struct bl_sig *sig)
{
const char *dev_name = disk->valid_path->full_path;
- struct bl_sig_comp *comp;
- int fd, i, ret;
+ int fd, i, rv;
fd = open(dev_name, O_RDONLY | O_LARGEFILE);
if (fd < 0) {
- BL_LOG_ERR("%s could not be opened for read\n", dev_name);
+ BL_LOG_ERR("%s: %s could not be opened for read\n", __func__,
+ dev_name);
return 0;
}
+ rv = 1;
+
for (i = 0; i < sig->si_num_comps; i++) {
- comp = &sig->si_comps[i];
- ret = read_cmp_blk_sig(disk, fd, comp);
- if (ret)
- return 0;
+ if (read_cmp_blk_sig(disk, fd, &sig->si_comps[i])) {
+ rv = 0;
+ break;
+ }
}
if (fd >= 0)
close(fd);
- return 1;
+ return rv;
}
/*
@@ -197,13 +199,6 @@ static int map_sig_to_device(struct bl_sig *sig, struct bl_volume *vol)
int mapped = 0;
struct bl_disk *disk = visible_disk_list;
char *filepath = 0;
- struct bl_disk *lolDisk = disk;
-
- while (lolDisk) {
- BL_LOG_INFO("%s: visible_disk_list: %s\n", __func__,
- lolDisk->valid_path->full_path);
- lolDisk = lolDisk->next;
- }
/* scan disk list to find out match device */
while (disk) {
@@ -267,6 +262,8 @@ static int decode_blk_volume(uint32_t **pp, uint32_t *end,
BLK_READBUF(p, end, 4);
READ32(vol->bv_type);
+ BL_LOG_INFO("%s: bv_type %d\n", __func__, vol->bv_type);
+
switch (vol->bv_type) {
case BLOCK_VOLUME_SIMPLE:
*array_cnt = 0;
@@ -347,13 +344,13 @@ uint64_t process_deviceinfo(const char *dev_addr_buf,
p = (uint32_t *) dev_addr_buf;
end = (uint32_t *) ((char *)p + dev_addr_len);
+
/* Decode block volume */
BLK_READBUF(p, end, 4);
READ32(num_vols);
- if (num_vols <= 0) {
- BL_LOG_ERR("Error: number of vols: %d\n", num_vols);
+ BL_LOG_INFO("%s: %d vols\n", __func__, num_vols);
+ if (num_vols <= 0)
goto out_err;
- }
vols = (struct bl_volume *)malloc(num_vols * sizeof(struct bl_volume));
if (!vols) {
diff --git a/utils/blkmapd/dm-device.c b/utils/blkmapd/dm-device.c
index 5a9f5ec..d720086 100644
--- a/utils/blkmapd/dm-device.c
+++ b/utils/blkmapd/dm-device.c
@@ -31,6 +31,7 @@
#include <stdio.h>
#include <stdlib.h>
+#include <unistd.h>
#include <string.h>
#include <syslog.h>
#include <fcntl.h>
@@ -46,8 +47,6 @@
#endif
#define DM_PARAMS_LEN 512 /* XXX: is this enough for target? */
-#define DM_DIR "/dev/mapper"
-#define DM_DIR_LEN12
#define TYPE_HAS_DEV(type) ((type == BLOCK_VOLUME_SIMPLE) || \
(type == BLOCK_VOLUME_PSEUDO))
@@ -65,6 +64,10 @@ struct bl_dm_tree {
struct bl_dm_tree *next;
};
+static const char dm_name[] = "pnfs_vol_%u";
+
+static unsigned int dev_count;
+
static inline struct bl_dm_table *bl_dm_table_alloc(void)
{
return (struct bl_dm_table *)calloc(1, sizeof(struct bl_dm_table));
@@ -104,7 +107,7 @@ static struct bl_dm_tree *find_bl_dm_tree(uint64_t dev)
for (p = bl_tree_head; p; p = p->next) {
if (p->dev == dev)
- break;
+ break;
}
return p;
}
@@ -146,7 +149,7 @@ static void add_to_bl_dm_tree(struct bl_dm_tree *tree)
* return dev no for created device
*/
static uint64_t
-dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
+dm_device_create_mapped(const char *dev_name, struct bl_dm_table *p)
{
struct dm_task *dmt;
struct dm_info dminfo;
@@ -162,20 +165,23 @@ dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
goto err_out;
while (p) {
- ret = dm_task_add_target(dmt, p->offset, p->size,
- p->target_type, p->params);
+ ret =
+ dm_task_add_target(dmt, p->offset, p->size, p->target_type,
+ p->params);
if (!ret)
goto err_out;
p = p->next;
}
- ret = dm_task_run(dmt) &&
- dm_task_get_info(dmt, &dminfo) && dminfo.exists;
+ ret = dm_task_run(dmt) && dm_task_get_info(dmt, &dminfo)
+ && dminfo.exists;
if (!ret)
goto err_out;
dm_task_update_nodes();
+ BL_LOG_ERR("%s: %s %d/%d\n", __func__, dev_name,
+ (int)dminfo.major, (int)dminfo.minor);
err_out:
dm_task_destroy(dmt);
@@ -192,6 +198,8 @@ static int dm_device_remove_byname(const char *dev_name)
struct dm_task *dmt;
int ret = 0;
+ BL_LOG_INFO("%s: %s\n", __func__, dev_name);
+
dmt = dm_task_create(DM_DEVICE_REMOVE);
if (!dmt)
return 0;
@@ -234,7 +242,7 @@ int dm_device_remove(uint64_t dev)
while (dmnames) {
if (dmnames->dev == dev) {
- name = dmnames->name;
+ name = strdup(dmnames->name);
break;
}
dmnames = (void *)dmnames + dmnames->next;
@@ -252,24 +260,24 @@ int dm_device_remove(uint64_t dev)
dm_task_destroy(dmt);
/* Start to remove device */
- if (name)
+ if (name) {
ret = dm_device_remove_byname(name);
+ free(name);
+ }
return ret;
}
-static unsigned long dev_count;
-
-static void dm_devicelist_remove(unsigned long start, unsigned long end)
+static void dm_devicelist_remove(unsigned int start, unsigned int end)
{
char dev_name[DM_DEV_NAME_LEN];
- unsigned long count;
+ unsigned int count;
if (start >= dev_count || end <= 1 || start >= end - 1)
return;
for (count = end - 1; count > start; count--) {
- sprintf(dev_name, "pnfs_vol_%lu", count - 1);
+ snprintf(dev_name, sizeof dev_name, dm_name, count - 1);
dm_device_remove_byname(dev_name);
}
@@ -352,11 +360,19 @@ int dm_device_remove_all(uint64_t *dev)
return ret;
}
+static int dm_device_exists(char *dev_name)
+{
+ char fullname[DM_DEV_NAME_LEN];
+
+ snprintf(fullname, sizeof fullname, "/dev/mapper/%s", dev_name);
+ return (access(fullname, F_OK) >= 0);
+}
+
/* TODO: check the value for DM_DEV_NAME_LEN, DM_TYPE_LEN, DM_PARAMS_LEN */
uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
{
uint64_t size, dev = 0;
- unsigned long count = dev_count;
+ unsigned int count = dev_count;
int volnum, i, pos;
struct bl_volume *node;
char *tmp;
@@ -466,9 +482,12 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
BL_LOG_ERR("%s: Out of memory\n", __func__);
goto out;
}
- sprintf(dev_name, "pnfs_vol_%lu", dev_count++);
+ do {
+ snprintf(dev_name, DM_DEV_NAME_LEN, dm_name,
+ dev_count++);
+ } while (dm_device_exists(dev_name));
- dev = dm_single_device_create(dev_name, bl_table_head);
+ dev = dm_device_create_mapped(dev_name, bl_table_head);
if (!dev) {
/* Delete previous temporary devices */
dm_devicelist_remove(count, dev_count);
--
1.7.1
Signed-off-by: Jim Rees <[email protected]>
---
utils/blkmapd/device-discovery.c | 1 +
utils/blkmapd/device-discovery.h | 11 +--
utils/blkmapd/device-inq.c | 7 +-
utils/blkmapd/device-process.c | 31 ++++---
utils/blkmapd/dm-device.c | 164 +++++++++++++++++--------------------
5 files changed, 103 insertions(+), 111 deletions(-)
diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c
index 0497a11..c2675b8 100644
--- a/utils/blkmapd/device-discovery.c
+++ b/utils/blkmapd/device-discovery.c
@@ -39,6 +39,7 @@
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
+#include <syslog.h>
#include <dirent.h>
#include <ctype.h>
#include <fcntl.h>
diff --git a/utils/blkmapd/device-discovery.h b/utils/blkmapd/device-discovery.h
index 8cf88b8..a0937b1 100644
--- a/utils/blkmapd/device-discovery.h
+++ b/utils/blkmapd/device-discovery.h
@@ -28,11 +28,10 @@
#define BL_DEVICE_DISCOVERY_H
#include <stdint.h>
-#include <syslog.h>
enum blk_vol_type {
BLOCK_VOLUME_SIMPLE = 0, /* maps to a single LU */
- BLOCK_VOLUME_SLICE = 1, /* slice of another volume */
+ BLOCK_VOLUME_SLICE = 1, /* slice of another volume */
BLOCK_VOLUME_CONCAT = 2, /* concatenation of multiple volumes */
BLOCK_VOLUME_STRIPE = 3, /* striped across multiple volumes */
BLOCK_VOLUME_PSEUDO = 4,
@@ -45,15 +44,15 @@ struct bl_volume {
struct bl_volume **bv_vols;
int bv_vol_n;
union {
- dev_t bv_dev; /* for BLOCK_VOLUME_SIMPLE(PSEUDO) */
+ dev_t bv_dev; /* for BLOCK_VOLUME_SIMPLE(PSEUDO) */
off_t bv_stripe_unit; /* for BLOCK_VOLUME_STRIPE(CONCAT) */
off_t bv_offset; /* for BLOCK_VOLUME_SLICE */
} param;
};
struct bl_sig_comp {
- int64_t bs_offset; /* In bytes */
- uint32_t bs_length; /* In bytes */
+ uint64_t bs_offset; /* In bytes */
+ uint32_t bs_length; /* In bytes */
char *bs_string;
};
@@ -107,7 +106,7 @@ struct pipefs_hdr {
uint32_t msgid;
uint8_t type;
uint8_t flags;
- uint16_t totallen; /* length of entire message, including hdr */
+ uint16_t totallen; /* length of entire message, including hdr */
uint32_t status;
};
diff --git a/utils/blkmapd/device-inq.c b/utils/blkmapd/device-inq.c
index e1c0128..eabc70c 100644
--- a/utils/blkmapd/device-inq.c
+++ b/utils/blkmapd/device-inq.c
@@ -39,11 +39,12 @@
#include <stdlib.h>
#include <stdio.h>
+#include <unistd.h>
#include <string.h>
+#include <syslog.h>
#include <dirent.h>
#include <ctype.h>
#include <fcntl.h>
-#include <unistd.h>
#include <libgen.h>
#include <errno.h>
@@ -174,10 +175,10 @@ extern enum bl_path_state_e bldev_read_ap_state(int fd)
struct bl_serial *bldev_read_serial(int fd, const char *filename)
{
struct bl_serial *serial_out = NULL;
- int status = 0, pos, len;
+ int status = 0;
char *buffer;
struct bl_dev_id *dev_root, *dev_id;
- unsigned int current_id = 0;
+ unsigned int pos, len, current_id = 0;
status = bldev_inquire_pages(fd, 0x83, &buffer);
if (status)
diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
index 4482bd5..4ce47e1 100644
--- a/utils/blkmapd/device-process.c
+++ b/utils/blkmapd/device-process.c
@@ -33,29 +33,33 @@
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-#include <libdevmapper.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/user.h>
+#include <arpa/inet.h>
+#include <linux/kdev_t.h>
+
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <sys/user.h>
+#include <syslog.h>
#include <fcntl.h>
#include <errno.h>
-#include <arpa/inet.h>
-#include <linux/kdev_t.h>
+
#include "device-discovery.h"
-static char *pretty_sig(char *sig, int siglen)
+static char *pretty_sig(char *sig, uint32_t siglen)
{
static char rs[100];
- unsigned int i;
+ unsigned long i;
- if (siglen <= 4) {
+ if (siglen <= sizeof i) {
memcpy(&i, sig, sizeof i);
- sprintf(rs, "0x%0x", i);
+ sprintf(rs, "0x%0lx", i);
} else {
+ if (siglen > sizeof rs - 1)
+ siglen = sizeof rs - 1;
memcpy(rs, sig, siglen);
rs[siglen] = '\0';
}
@@ -65,6 +69,7 @@ static char *pretty_sig(char *sig, int siglen)
uint32_t *blk_overflow(uint32_t * p, uint32_t * end, size_t nbytes)
{
uint32_t *q = p + ((nbytes + 3) >> 2);
+
if (q > end || q < p)
return NULL;
return p;
@@ -73,8 +78,8 @@ uint32_t *blk_overflow(uint32_t * p, uint32_t * end, size_t nbytes)
static int decode_blk_signature(uint32_t ** pp, uint32_t * end,
struct bl_sig *sig)
{
- int i, siglen;
- uint32_t *p = *pp;
+ int i;
+ uint32_t siglen, *p = *pp;
BLK_READBUF(p, end, 4);
READ32(sig->si_num_comps);
@@ -288,7 +293,7 @@ static int decode_blk_volume(uint32_t **pp, uint32_t *end,
off_t chunksize = vol->param.bv_stripe_unit;
if ((chunksize == 0) ||
((chunksize & (chunksize - 1)) != 0) ||
- (chunksize < (PAGE_SIZE >> 9)))
+ (chunksize < (off_t)(PAGE_SIZE >> 9)))
return -EIO;
BLK_READBUF(p, end, 4);
READ32(vol->bv_vol_n);
diff --git a/utils/blkmapd/dm-device.c b/utils/blkmapd/dm-device.c
index 14ad131..5a9f5ec 100644
--- a/utils/blkmapd/dm-device.c
+++ b/utils/blkmapd/dm-device.c
@@ -24,15 +24,19 @@
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-#include <libdevmapper.h>
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <linux/kdev_t.h>
+
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
-#include <sys/types.h>
-#include <sys/stat.h>
+#include <syslog.h>
#include <fcntl.h>
#include <errno.h>
-#include <linux/kdev_t.h>
+#include <libdevmapper.h>
+
#include "device-discovery.h"
#define DM_DEV_NAME_LEN 256
@@ -66,9 +70,10 @@ static inline struct bl_dm_table *bl_dm_table_alloc(void)
return (struct bl_dm_table *)calloc(1, sizeof(struct bl_dm_table));
}
-void bl_dm_table_free(struct bl_dm_table *bl_table_head)
+static void bl_dm_table_free(struct bl_dm_table *bl_table_head)
{
- struct bl_dm_table *p = bl_table_head;
+ struct bl_dm_table *p;
+
while (bl_table_head) {
p = bl_table_head->next;
free(bl_table_head);
@@ -76,41 +81,39 @@ void bl_dm_table_free(struct bl_dm_table *bl_table_head)
}
}
-void add_to_bl_dm_table(struct bl_dm_table **bl_table_head,
+static void add_to_bl_dm_table(struct bl_dm_table **bl_table_head,
struct bl_dm_table *table)
{
- struct bl_dm_table *pre;
+ struct bl_dm_table *p;
+
if (!*bl_table_head) {
*bl_table_head = table;
return;
}
- pre = *bl_table_head;
- while (pre->next)
- pre = pre->next;
- pre->next = table;
- return;
+ p = *bl_table_head;
+ while (p->next)
+ p = p->next;
+ p->next = table;
}
struct bl_dm_tree *bl_tree_head;
-struct bl_dm_tree *find_bl_dm_tree(uint64_t dev)
+static struct bl_dm_tree *find_bl_dm_tree(uint64_t dev)
{
- struct bl_dm_tree *p = bl_tree_head;
- while (p) {
+ struct bl_dm_tree *p;
+
+ for (p = bl_tree_head; p; p = p->next) {
if (p->dev == dev)
- return p;
- p = p->next;
+ break;
}
- return NULL;
+ return p;
}
-void del_from_bl_dm_tree(uint64_t dev)
+static void del_from_bl_dm_tree(uint64_t dev)
{
- struct bl_dm_tree *pre = bl_tree_head;
- struct bl_dm_tree *p;
+ struct bl_dm_tree *p, *pre = bl_tree_head;
- p = pre;
- while (p) {
+ for (p = pre; p; p = p->next) {
if (p->dev == dev) {
pre->next = p->next;
if (p == bl_tree_head)
@@ -119,29 +122,31 @@ void del_from_bl_dm_tree(uint64_t dev)
break;
}
pre = p;
- p = pre->next;
}
}
-void add_to_bl_dm_tree(struct bl_dm_tree *tree)
+static void add_to_bl_dm_tree(struct bl_dm_tree *tree)
{
- struct bl_dm_tree *pre;
+ struct bl_dm_tree *p;
+
if (!bl_tree_head) {
bl_tree_head = tree;
return;
}
- pre = bl_tree_head;
- while (pre->next)
- pre = pre->next;
- pre->next = tree;
+ p = bl_tree_head;
+ while (p->next)
+ p = p->next;
+ p->next = tree;
return;
}
-/* Create device via device mapper
+/*
+ * Create device via device mapper
* return 0 when creation failed
* return dev no for created device
*/
-uint64_t dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
+static uint64_t
+dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
{
struct dm_task *dmt;
struct dm_info dminfo;
@@ -182,21 +187,19 @@ uint64_t dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
return MKDEV(dminfo.major, dminfo.minor);
}
-int dm_device_remove_byname(const char *dev_name)
+static int dm_device_remove_byname(const char *dev_name)
{
struct dm_task *dmt;
int ret = 0;
dmt = dm_task_create(DM_DEVICE_REMOVE);
if (!dmt)
- return -ENODEV;
+ return 0;
ret = dm_task_set_name(dmt, dev_name) && dm_task_run(dmt);
dm_task_update_nodes();
-
- if (dmt)
- dm_task_destroy(dmt);
+ dm_task_destroy(dmt);
return ret;
}
@@ -205,61 +208,64 @@ int dm_device_remove(uint64_t dev)
{
struct dm_task *dmt;
struct dm_names *dmnames;
- char *names = NULL;
- int ret = -1;
+ char *name = NULL;
+ int ret = 0;
/* Look for dev_name via dev, if dev_name could be transferred here,
we could jump to DM_DEVICE_REMOVE directly */
+
dmt = dm_task_create(DM_DEVICE_LIST);
if (!dmt) {
BL_LOG_ERR("dm_task creation failed\n");
- return -ENODEV;
+ goto out;
}
ret = dm_task_run(dmt);
if (!ret) {
BL_LOG_ERR("dm_task_run failed\n");
- goto error;
+ goto out;
}
dmnames = dm_task_get_names(dmt);
if (!dmnames || !dmnames->dev) {
BL_LOG_ERR("dm_task_get_names failed\n");
- goto error;
+ goto out;
}
- do {
+ while (dmnames) {
if (dmnames->dev == dev) {
- names = dmnames->name;
+ name = dmnames->name;
break;
}
dmnames = (void *)dmnames + dmnames->next;
- } while (dmnames);
+ }
- if (!names) {
+ if (!name) {
BL_LOG_ERR("Could not find device\n");
- goto error;
+ goto out;
}
dm_task_update_nodes();
- error:
- dm_task_destroy(dmt);
+ out:
+ if (dmt)
+ dm_task_destroy(dmt);
/* Start to remove device */
- if (names)
- ret = dm_device_remove_byname(names);
+ if (name)
+ ret = dm_device_remove_byname(name);
+
return ret;
}
static unsigned long dev_count;
-void dm_devicelist_remove(unsigned long start, unsigned long end)
+static void dm_devicelist_remove(unsigned long start, unsigned long end)
{
char dev_name[DM_DEV_NAME_LEN];
unsigned long count;
- if ((start >= dev_count) || (end <= 1) || (start >= end - 1))
+ if (start >= dev_count || end <= 1 || start >= end - 1)
return;
for (count = end - 1; count > start; count--) {
@@ -270,7 +276,7 @@ void dm_devicelist_remove(unsigned long start, unsigned long end)
return;
}
-void bl_dm_remove_tree(uint64_t dev)
+static void bl_dm_remove_tree(uint64_t dev)
{
struct bl_dm_tree *p;
@@ -282,28 +288,28 @@ void bl_dm_remove_tree(uint64_t dev)
del_from_bl_dm_tree(dev);
}
-void bl_dm_create_tree(uint64_t dev)
+static int bl_dm_create_tree(uint64_t dev)
{
struct dm_tree *tree;
struct bl_dm_tree *bl_tree;
bl_tree = find_bl_dm_tree(dev);
if (bl_tree)
- return; /* XXX: error? */
+ return 1;
tree = dm_tree_create();
if (!tree)
- return;
+ return 0;
if (!dm_tree_add_dev(tree, MAJOR(dev), MINOR(dev))) {
dm_tree_free(tree);
- return;
+ return 0;
}
bl_tree = malloc(sizeof(struct bl_dm_tree));
if (!bl_tree) {
dm_tree_free(tree);
- return;
+ return 0;
}
bl_tree->dev = dev;
@@ -311,29 +317,7 @@ void bl_dm_create_tree(uint64_t dev)
bl_tree->next = NULL;
add_to_bl_dm_tree(bl_tree);
- return;
-}
-
-uint64_t dm_device_nametodev(char *dev_name)
-{
- struct dm_task *dmt;
- int ret = 0;
- struct dm_info dminfo;
-
- dmt = dm_task_create(DM_DEVICE_INFO);
- if (!dmt)
- return -ENODEV;
-
- ret = dm_task_set_name(dmt, dev_name) &&
- dm_task_run(dmt) && dm_task_get_info(dmt, &dminfo);
-
- if (dmt)
- dm_task_destroy(dmt);
-
- if (!ret)
- return 0;
-
- return MKDEV(dminfo.major, dminfo.minor);
+ return 1;
}
int dm_device_remove_all(uint64_t *dev)
@@ -364,6 +348,7 @@ int dm_device_remove_all(uint64_t *dev)
ret = dm_tree_deactivate_children(node, uuid, strlen(uuid));
dm_task_update_nodes();
bl_dm_remove_tree(bl_dev);
+
return ret;
}
@@ -372,7 +357,7 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
{
uint64_t size, dev = 0;
unsigned long count = dev_count;
- int number = 0, i, pos;
+ int volnum, i, pos;
struct bl_volume *node;
char *tmp;
struct bl_dm_table *table = NULL;
@@ -381,8 +366,8 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
char *dev_name = NULL;
/* Create pseudo device here */
- while (number < num_vols) {
- node = &vols[number];
+ for (volnum = 0; volnum < num_vols; volnum++) {
+ node = &vols[volnum];
switch (node->bv_type) {
case BLOCK_VOLUME_SIMPLE:
/* Do not need to create device here */
@@ -492,16 +477,17 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
node->param.bv_dev = dev;
/* TODO: extend use with PSEUDO later */
node->bv_type = BLOCK_VOLUME_PSEUDO;
+
continued:
- number++;
if (bl_table_head)
bl_dm_table_free(bl_table_head);
bl_table_head = NULL;
}
out:
- if (bl_table_head)
+ if (bl_table_head) {
bl_dm_table_free(bl_table_head);
- bl_table_head = NULL;
+ bl_table_head = NULL;
+ }
if (dev)
bl_dm_create_tree(dev);
if (dev_name)
--
1.7.1
re-write decode_blk_signature to keep fd open
pretty print disk signature
remove unneeded syslog info messages
Signed-off-by: Jim Rees <[email protected]>
---
utils/blkmapd/device-process.c | 105 +++++++++++++++++++++++-----------------
1 files changed, 60 insertions(+), 45 deletions(-)
diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
index a543769..4482bd5 100644
--- a/utils/blkmapd/device-process.c
+++ b/utils/blkmapd/device-process.c
@@ -47,6 +47,21 @@
#include <linux/kdev_t.h>
#include "device-discovery.h"
+static char *pretty_sig(char *sig, int siglen)
+{
+ static char rs[100];
+ unsigned int i;
+
+ if (siglen <= 4) {
+ memcpy(&i, sig, sizeof i);
+ sprintf(rs, "0x%0x", i);
+ } else {
+ memcpy(rs, sig, siglen);
+ rs[siglen] = '\0';
+ }
+ return rs;
+}
+
uint32_t *blk_overflow(uint32_t * p, uint32_t * end, size_t nbytes)
{
uint32_t *q = p + ((nbytes + 3) >> 2);
@@ -55,10 +70,10 @@ uint32_t *blk_overflow(uint32_t * p, uint32_t * end, size_t nbytes)
return p;
}
-static int decode_blk_signature(uint32_t **pp, uint32_t *end,
+static int decode_blk_signature(uint32_t ** pp, uint32_t * end,
struct bl_sig *sig)
{
- int i, tmp;
+ int i, siglen;
uint32_t *p = *pp;
BLK_READBUF(p, end, 4);
@@ -73,19 +88,21 @@ static int decode_blk_signature(uint32_t **pp, uint32_t *end,
goto out_err;
}
for (i = 0; i < sig->si_num_comps; i++) {
+ struct bl_sig_comp *comp = &sig->si_comps[i];
+
BLK_READBUF(p, end, 12);
- READ64(sig->si_comps[i].bs_offset);
- READ32(tmp);
- sig->si_comps[i].bs_length = tmp;
- BLK_READBUF(p, end, tmp);
+ READ64(comp->bs_offset);
+ READ32(siglen);
+ comp->bs_length = siglen;
+ BLK_READBUF(p, end, siglen);
/* Note we rely here on fact that sig is used immediately
* for mapping, then thrown away.
*/
- sig->si_comps[i].bs_string = (char *)p;
+ comp->bs_string = (char *)p;
BL_LOG_INFO("%s: si_comps[%d]: bs_length %d, bs_string %s\n",
- __func__, i, sig->si_comps[i].bs_length,
- sig->si_comps[i].bs_string);
- p += ((tmp + 3) >> 2);
+ __func__, i, siglen,
+ pretty_sig(comp->bs_string, siglen));
+ p += ((siglen + 3) >> 2);
}
*pp = p;
return 0;
@@ -93,50 +110,45 @@ static int decode_blk_signature(uint32_t **pp, uint32_t *end,
return -EIO;
}
-/* Read signature from device
- * return 0: read successfully
- * return -1: error
+/*
+ * Read signature from device and compare to sig_comp
+ * return: 0=match, 1=no match, -1=error
*/
-int
-read_cmp_blk_sig(const char *dev_name, struct bl_sig_comp *comp,
- int64_t bs_offset)
+static int
+read_cmp_blk_sig(struct bl_disk *disk, int fd, struct bl_sig_comp *comp)
{
- int fd, ret = -1;
+ const char *dev_name = disk->valid_path->full_path;
+ int ret = -1;
+ ssize_t siglen = comp->bs_length;
+ int64_t bs_offset = comp->bs_offset;
char *sig = NULL;
- fd = open(dev_name, O_RDONLY | O_LARGEFILE);
- if (fd < 0) {
- BL_LOG_ERR("%s could not be opened for read\n", dev_name);
- goto error;
- }
-
- sig = (char *)malloc(comp->bs_length);
+ sig = (char *)malloc(siglen);
if (!sig) {
BL_LOG_ERR("%s: Out of memory\n", __func__);
- goto error;
+ goto out;
}
+ if (bs_offset < 0)
+ bs_offset += (((int64_t) disk->size) << 9);
if (lseek64(fd, bs_offset, SEEK_SET) == -1) {
BL_LOG_ERR("File %s lseek error\n", dev_name);
- goto error;
+ goto out;
}
- if (read(fd, sig, comp->bs_length) != comp->bs_length) {
+ if (read(fd, sig, siglen) != siglen) {
BL_LOG_ERR("File %s read error\n", dev_name);
- goto error;
+ goto out;
}
- BL_LOG_INFO
- ("%s: %s sig: %s, bs_string: %s, bs_length: %d, bs_offset: %lld\n",
- __func__, dev_name, sig, comp->bs_string, comp->bs_length,
- (long long)bs_offset);
- ret = memcmp(sig, comp->bs_string, comp->bs_length);
+ ret = memcmp(sig, comp->bs_string, siglen);
+ if (!ret)
+ BL_LOG_INFO("%s: %s sig %s at %lld\n", __func__, dev_name,
+ pretty_sig(sig, siglen), (long long)bs_offset);
- error:
+ out:
if (sig)
free(sig);
- if (fd >= 0)
- close(fd);
return ret;
}
@@ -146,22 +158,25 @@ read_cmp_blk_sig(const char *dev_name, struct bl_sig_comp *comp,
*/
static int verify_sig(struct bl_disk *disk, struct bl_sig *sig)
{
+ const char *dev_name = disk->valid_path->full_path;
struct bl_sig_comp *comp;
- int i, ret;
- int64_t bs_offset;
+ int fd, i, ret;
+
+ fd = open(dev_name, O_RDONLY | O_LARGEFILE);
+ if (fd < 0) {
+ BL_LOG_ERR("%s could not be opened for read\n", dev_name);
+ return 0;
+ }
for (i = 0; i < sig->si_num_comps; i++) {
comp = &sig->si_comps[i];
- bs_offset = comp->bs_offset;
- if (bs_offset < 0)
- bs_offset += (((int64_t) disk->size) << 9);
- BL_LOG_INFO("%s: bs_offset: %lld\n",
- __func__, (long long) bs_offset);
- ret = read_cmp_blk_sig(disk->valid_path->full_path,
- comp, bs_offset);
+ ret = read_cmp_blk_sig(disk, fd, comp);
if (ret)
return 0;
}
+
+ if (fd >= 0)
+ close(fd);
return 1;
}
--
1.7.1
Signed-off-by: Jim Rees <[email protected]>
---
utils/blkmapd/etc/blkmapd.conf | 10 ----------
1 files changed, 0 insertions(+), 10 deletions(-)
delete mode 100644 utils/blkmapd/etc/blkmapd.conf
diff --git a/utils/blkmapd/etc/blkmapd.conf b/utils/blkmapd/etc/blkmapd.conf
deleted file mode 100644
index da70d94..0000000
--- a/utils/blkmapd/etc/blkmapd.conf
+++ /dev/null
@@ -1,10 +0,0 @@
-# This is an example config file
-
-# Look at all /dev/sd* devices
-# /dev/sd or /dev/sd*
-/dev/sd*
-
-# Look at all /dev/mapper/* devices
-# /dev/mapper/* or
-# /dev/mapper/
-/dev/mapper/*
--
1.7.1
Benny Halevy wrote:
Why can't it always dump the signature in hex, even if it happens to be ASCII?
I need it in ascii because that's how all my other tools print it out. When
I mis-place a disk, I start by looking at it on the server:
[nasadmin@emc-0 ~]$ nas_disk -l
id inuse sizeMB storageID-devID type name servers
1 y 11263 APM00064403224-0000 CLSTD root_disk 1,2
2 y 11263 APM00064403224-0001 CLSTD root_ldisk 1,2
3 y 2047 APM00064403224-0002 CLSTD d3 1,2
4 y 2047 APM00064403224-0003 CLSTD d4 1,2
5 y 2047 APM00064403224-0004 CLSTD d5 1,2
6 y 32767 APM00064403224-0005 CLSTD d6 1,2
7 n 451387 APM00064403224-0010 CLSTD d7 1,2
8 y 451387 APM00064403224-0011 CLSTD d8 1,2
Then on the client:
pdsi7# mpfsinq
Celerra signature vendor product_id device serial number or pathinfo
APM000644032240000-0008 DGC RAID 5 60:06:01:60:80:40:1a:00:be:91:96:89:d5:26:dd:11
path = /dev/sdh Active SP-b0 /dev/sg7
path = /dev/sdg Passive SP-a0 /dev/sg6
APM000644032240000-0007 DGC RAID 5 60:06:01:60:55:d1:19:00:1e:12:0e:87:d5:26:dd:11
path = /dev/sde Active SP-a0 /dev/sg4
path = /dev/sdf Passive SP-b0 /dev/sg5
And finally in the blkmapd log:
blkmapd: process_deviceinfo: 28 vols
blkmapd: decode_blk_volume: bv_type 0
blkmapd: decode_blk_signature: si_comps[0]: bs_length 4, bs_string 0x7
blkmapd: decode_blk_signature: si_comps[1]: bs_length 32, bs_string APM000644032240000
blkmapd: read_cmp_blk_sig: /dev/sde sig 0x7 at 473313851392
blkmapd: read_cmp_blk_sig: /dev/sde sig APM000644032240000 at 473313851492
blkmapd: decode_blk_volume: bv_type 1
blkmapd: decode_blk_volume: bv_type 0
blkmapd: decode_blk_signature: si_comps[0]: bs_length 4, bs_string 0x8
blkmapd: decode_blk_signature: si_comps[1]: bs_length 32, bs_string APM000644032240000
blkmapd: read_cmp_blk_sig: /dev/sdh sig 0x8 at 473313851392
blkmapd: read_cmp_blk_sig: /dev/sdh sig APM000644032240000 at 473313851492
Signed-off-by: Benny Halevy <[email protected]>
---
utils/blkmapd/device-process.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
index 0584bf9..ea8b8ec 100644
--- a/utils/blkmapd/device-process.c
+++ b/utils/blkmapd/device-process.c
@@ -64,10 +64,12 @@ static char *pretty_sig(char *sig, uint32_t siglen)
}
sprintf(rs, "0x%0llx", sigval);
} else {
- if (siglen > sizeof rs - 1)
- siglen = sizeof rs - 1;
+ if (siglen > sizeof rs - 4) {
+ siglen = sizeof rs - 4;
+ sprintf(&rs[siglen], "...");
+ } else
+ rs[siglen] = '\0';
memcpy(rs, sig, siglen);
- rs[siglen] = '\0';
}
return rs;
}
--
1.7.2.3
On 2010-12-02 15:59, Benny Halevy wrote:
> On 2010-11-30 21:14, Jim Rees wrote:
>> Signed-off-by: Jim Rees <[email protected]>
>> ---
>> utils/blkmapd/device-discovery.c | 1 +
>> utils/blkmapd/device-discovery.h | 11 +--
>> utils/blkmapd/device-inq.c | 7 +-
>> utils/blkmapd/device-process.c | 31 ++++---
>> utils/blkmapd/dm-device.c | 164 +++++++++++++++++--------------------
>> 5 files changed, 103 insertions(+), 111 deletions(-)
>>
>> diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c
>> index 0497a11..c2675b8 100644
>> --- a/utils/blkmapd/device-discovery.c
>> +++ b/utils/blkmapd/device-discovery.c
>> @@ -39,6 +39,7 @@
>> #include <stdlib.h>
>> #include <stdio.h>
>> #include <string.h>
>> +#include <syslog.h>
>> #include <dirent.h>
>> #include <ctype.h>
>> #include <fcntl.h>
>> diff --git a/utils/blkmapd/device-discovery.h b/utils/blkmapd/device-discovery.h
>> index 8cf88b8..a0937b1 100644
>> --- a/utils/blkmapd/device-discovery.h
>> +++ b/utils/blkmapd/device-discovery.h
>> @@ -28,11 +28,10 @@
>> #define BL_DEVICE_DISCOVERY_H
>>
>> #include <stdint.h>
>> -#include <syslog.h>
>>
>> enum blk_vol_type {
>> BLOCK_VOLUME_SIMPLE = 0, /* maps to a single LU */
>> - BLOCK_VOLUME_SLICE = 1, /* slice of another volume */
>> + BLOCK_VOLUME_SLICE = 1, /* slice of another volume */
>> BLOCK_VOLUME_CONCAT = 2, /* concatenation of multiple volumes */
>> BLOCK_VOLUME_STRIPE = 3, /* striped across multiple volumes */
>> BLOCK_VOLUME_PSEUDO = 4,
>> @@ -45,15 +44,15 @@ struct bl_volume {
>> struct bl_volume **bv_vols;
>> int bv_vol_n;
>> union {
>> - dev_t bv_dev; /* for BLOCK_VOLUME_SIMPLE(PSEUDO) */
>> + dev_t bv_dev; /* for BLOCK_VOLUME_SIMPLE(PSEUDO) */
>> off_t bv_stripe_unit; /* for BLOCK_VOLUME_STRIPE(CONCAT) */
>> off_t bv_offset; /* for BLOCK_VOLUME_SLICE */
>> } param;
>> };
>>
>> struct bl_sig_comp {
>> - int64_t bs_offset; /* In bytes */
>> - uint32_t bs_length; /* In bytes */
>> + uint64_t bs_offset; /* In bytes */
>> + uint32_t bs_length; /* In bytes */
>> char *bs_string;
>> };
>>
>> @@ -107,7 +106,7 @@ struct pipefs_hdr {
>> uint32_t msgid;
>> uint8_t type;
>> uint8_t flags;
>> - uint16_t totallen; /* length of entire message, including hdr */
>> + uint16_t totallen; /* length of entire message, including hdr */
>> uint32_t status;
>> };
>>
>> diff --git a/utils/blkmapd/device-inq.c b/utils/blkmapd/device-inq.c
>> index e1c0128..eabc70c 100644
>> --- a/utils/blkmapd/device-inq.c
>> +++ b/utils/blkmapd/device-inq.c
>> @@ -39,11 +39,12 @@
>>
>> #include <stdlib.h>
>> #include <stdio.h>
>> +#include <unistd.h>
>> #include <string.h>
>> +#include <syslog.h>
>> #include <dirent.h>
>> #include <ctype.h>
>> #include <fcntl.h>
>> -#include <unistd.h>
>> #include <libgen.h>
>> #include <errno.h>
>>
>> @@ -174,10 +175,10 @@ extern enum bl_path_state_e bldev_read_ap_state(int fd)
>> struct bl_serial *bldev_read_serial(int fd, const char *filename)
>> {
>> struct bl_serial *serial_out = NULL;
>> - int status = 0, pos, len;
>> + int status = 0;
>> char *buffer;
>> struct bl_dev_id *dev_root, *dev_id;
>> - unsigned int current_id = 0;
>> + unsigned int pos, len, current_id = 0;
>>
>> status = bldev_inquire_pages(fd, 0x83, &buffer);
>> if (status)
>> diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
>> index 4482bd5..4ce47e1 100644
>> --- a/utils/blkmapd/device-process.c
>> +++ b/utils/blkmapd/device-process.c
>> @@ -33,29 +33,33 @@
>> * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> */
>>
>> -#include <libdevmapper.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include <sys/user.h>
>> +#include <arpa/inet.h>
>> +#include <linux/kdev_t.h>
>> +
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <string.h>
>> #include <unistd.h>
>> -#include <sys/types.h>
>> -#include <sys/stat.h>
>> -#include <sys/user.h>
>> +#include <syslog.h>
>> #include <fcntl.h>
>> #include <errno.h>
>> -#include <arpa/inet.h>
>> -#include <linux/kdev_t.h>
>> +
>> #include "device-discovery.h"
>>
>> -static char *pretty_sig(char *sig, int siglen)
>> +static char *pretty_sig(char *sig, uint32_t siglen)
>> {
>> static char rs[100];
>> - unsigned int i;
>> + unsigned long i;
>>
>> - if (siglen <= 4) {
>> + if (siglen <= sizeof i) {
>> memcpy(&i, sig, sizeof i);
>> - sprintf(rs, "0x%0x", i);
>> + sprintf(rs, "0x%0lx", i);
>
> What about machine endianess?
> The MDS and clients may be of different gender, no?
> Also, on 64 bit machines, you may copy 8 bytes while the signature
> is 4-bytes long so you may copy junk into i.
>
> Benny
>
>> } else {
>> + if (siglen > sizeof rs - 1)
>> + siglen = sizeof rs - 1;
Hmm, for courtesy purposes, how about ending the truncated
signature with "..."?
Benny
>> memcpy(rs, sig, siglen);
>> rs[siglen] = '\0';
>> }
>> @@ -65,6 +69,7 @@ static char *pretty_sig(char *sig, int siglen)
>> uint32_t *blk_overflow(uint32_t * p, uint32_t * end, size_t nbytes)
>> {
>> uint32_t *q = p + ((nbytes + 3) >> 2);
>> +
>> if (q > end || q < p)
>> return NULL;
>> return p;
>> @@ -73,8 +78,8 @@ uint32_t *blk_overflow(uint32_t * p, uint32_t * end, size_t nbytes)
>> static int decode_blk_signature(uint32_t ** pp, uint32_t * end,
>> struct bl_sig *sig)
>> {
>> - int i, siglen;
>> - uint32_t *p = *pp;
>> + int i;
>> + uint32_t siglen, *p = *pp;
>>
>> BLK_READBUF(p, end, 4);
>> READ32(sig->si_num_comps);
>> @@ -288,7 +293,7 @@ static int decode_blk_volume(uint32_t **pp, uint32_t *end,
>> off_t chunksize = vol->param.bv_stripe_unit;
>> if ((chunksize == 0) ||
>> ((chunksize & (chunksize - 1)) != 0) ||
>> - (chunksize < (PAGE_SIZE >> 9)))
>> + (chunksize < (off_t)(PAGE_SIZE >> 9)))
>> return -EIO;
>> BLK_READBUF(p, end, 4);
>> READ32(vol->bv_vol_n);
>> diff --git a/utils/blkmapd/dm-device.c b/utils/blkmapd/dm-device.c
>> index 14ad131..5a9f5ec 100644
>> --- a/utils/blkmapd/dm-device.c
>> +++ b/utils/blkmapd/dm-device.c
>> @@ -24,15 +24,19 @@
>> * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>> * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> */
>> -#include <libdevmapper.h>
>> +
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include <linux/kdev_t.h>
>> +
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <string.h>
>> -#include <sys/types.h>
>> -#include <sys/stat.h>
>> +#include <syslog.h>
>> #include <fcntl.h>
>> #include <errno.h>
>> -#include <linux/kdev_t.h>
>> +#include <libdevmapper.h>
>> +
>> #include "device-discovery.h"
>>
>> #define DM_DEV_NAME_LEN 256
>> @@ -66,9 +70,10 @@ static inline struct bl_dm_table *bl_dm_table_alloc(void)
>> return (struct bl_dm_table *)calloc(1, sizeof(struct bl_dm_table));
>> }
>>
>> -void bl_dm_table_free(struct bl_dm_table *bl_table_head)
>> +static void bl_dm_table_free(struct bl_dm_table *bl_table_head)
>> {
>> - struct bl_dm_table *p = bl_table_head;
>> + struct bl_dm_table *p;
>> +
>> while (bl_table_head) {
>> p = bl_table_head->next;
>> free(bl_table_head);
>> @@ -76,41 +81,39 @@ void bl_dm_table_free(struct bl_dm_table *bl_table_head)
>> }
>> }
>>
>> -void add_to_bl_dm_table(struct bl_dm_table **bl_table_head,
>> +static void add_to_bl_dm_table(struct bl_dm_table **bl_table_head,
>> struct bl_dm_table *table)
>> {
>> - struct bl_dm_table *pre;
>> + struct bl_dm_table *p;
>> +
>> if (!*bl_table_head) {
>> *bl_table_head = table;
>> return;
>> }
>> - pre = *bl_table_head;
>> - while (pre->next)
>> - pre = pre->next;
>> - pre->next = table;
>> - return;
>> + p = *bl_table_head;
>> + while (p->next)
>> + p = p->next;
>> + p->next = table;
>> }
>>
>> struct bl_dm_tree *bl_tree_head;
>>
>> -struct bl_dm_tree *find_bl_dm_tree(uint64_t dev)
>> +static struct bl_dm_tree *find_bl_dm_tree(uint64_t dev)
>> {
>> - struct bl_dm_tree *p = bl_tree_head;
>> - while (p) {
>> + struct bl_dm_tree *p;
>> +
>> + for (p = bl_tree_head; p; p = p->next) {
>> if (p->dev == dev)
>> - return p;
>> - p = p->next;
>> + break;
>> }
>> - return NULL;
>> + return p;
>> }
>>
>> -void del_from_bl_dm_tree(uint64_t dev)
>> +static void del_from_bl_dm_tree(uint64_t dev)
>> {
>> - struct bl_dm_tree *pre = bl_tree_head;
>> - struct bl_dm_tree *p;
>> + struct bl_dm_tree *p, *pre = bl_tree_head;
>>
>> - p = pre;
>> - while (p) {
>> + for (p = pre; p; p = p->next) {
>> if (p->dev == dev) {
>> pre->next = p->next;
>> if (p == bl_tree_head)
>> @@ -119,29 +122,31 @@ void del_from_bl_dm_tree(uint64_t dev)
>> break;
>> }
>> pre = p;
>> - p = pre->next;
>> }
>> }
>>
>> -void add_to_bl_dm_tree(struct bl_dm_tree *tree)
>> +static void add_to_bl_dm_tree(struct bl_dm_tree *tree)
>> {
>> - struct bl_dm_tree *pre;
>> + struct bl_dm_tree *p;
>> +
>> if (!bl_tree_head) {
>> bl_tree_head = tree;
>> return;
>> }
>> - pre = bl_tree_head;
>> - while (pre->next)
>> - pre = pre->next;
>> - pre->next = tree;
>> + p = bl_tree_head;
>> + while (p->next)
>> + p = p->next;
>> + p->next = tree;
>> return;
>> }
>>
>> -/* Create device via device mapper
>> +/*
>> + * Create device via device mapper
>> * return 0 when creation failed
>> * return dev no for created device
>> */
>> -uint64_t dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
>> +static uint64_t
>> +dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
>> {
>> struct dm_task *dmt;
>> struct dm_info dminfo;
>> @@ -182,21 +187,19 @@ uint64_t dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
>> return MKDEV(dminfo.major, dminfo.minor);
>> }
>>
>> -int dm_device_remove_byname(const char *dev_name)
>> +static int dm_device_remove_byname(const char *dev_name)
>> {
>> struct dm_task *dmt;
>> int ret = 0;
>>
>> dmt = dm_task_create(DM_DEVICE_REMOVE);
>> if (!dmt)
>> - return -ENODEV;
>> + return 0;
>>
>> ret = dm_task_set_name(dmt, dev_name) && dm_task_run(dmt);
>>
>> dm_task_update_nodes();
>> -
>> - if (dmt)
>> - dm_task_destroy(dmt);
>> + dm_task_destroy(dmt);
>>
>> return ret;
>> }
>> @@ -205,61 +208,64 @@ int dm_device_remove(uint64_t dev)
>> {
>> struct dm_task *dmt;
>> struct dm_names *dmnames;
>> - char *names = NULL;
>> - int ret = -1;
>> + char *name = NULL;
>> + int ret = 0;
>>
>> /* Look for dev_name via dev, if dev_name could be transferred here,
>> we could jump to DM_DEVICE_REMOVE directly */
>> +
>> dmt = dm_task_create(DM_DEVICE_LIST);
>> if (!dmt) {
>> BL_LOG_ERR("dm_task creation failed\n");
>> - return -ENODEV;
>> + goto out;
>> }
>>
>> ret = dm_task_run(dmt);
>> if (!ret) {
>> BL_LOG_ERR("dm_task_run failed\n");
>> - goto error;
>> + goto out;
>> }
>>
>> dmnames = dm_task_get_names(dmt);
>> if (!dmnames || !dmnames->dev) {
>> BL_LOG_ERR("dm_task_get_names failed\n");
>> - goto error;
>> + goto out;
>> }
>>
>> - do {
>> + while (dmnames) {
>> if (dmnames->dev == dev) {
>> - names = dmnames->name;
>> + name = dmnames->name;
>> break;
>> }
>> dmnames = (void *)dmnames + dmnames->next;
>> - } while (dmnames);
>> + }
>>
>> - if (!names) {
>> + if (!name) {
>> BL_LOG_ERR("Could not find device\n");
>> - goto error;
>> + goto out;
>> }
>>
>> dm_task_update_nodes();
>>
>> - error:
>> - dm_task_destroy(dmt);
>> + out:
>> + if (dmt)
>> + dm_task_destroy(dmt);
>>
>> /* Start to remove device */
>> - if (names)
>> - ret = dm_device_remove_byname(names);
>> + if (name)
>> + ret = dm_device_remove_byname(name);
>> +
>> return ret;
>> }
>>
>> static unsigned long dev_count;
>>
>> -void dm_devicelist_remove(unsigned long start, unsigned long end)
>> +static void dm_devicelist_remove(unsigned long start, unsigned long end)
>> {
>> char dev_name[DM_DEV_NAME_LEN];
>> unsigned long count;
>>
>> - if ((start >= dev_count) || (end <= 1) || (start >= end - 1))
>> + if (start >= dev_count || end <= 1 || start >= end - 1)
>> return;
>>
>> for (count = end - 1; count > start; count--) {
>> @@ -270,7 +276,7 @@ void dm_devicelist_remove(unsigned long start, unsigned long end)
>> return;
>> }
>>
>> -void bl_dm_remove_tree(uint64_t dev)
>> +static void bl_dm_remove_tree(uint64_t dev)
>> {
>> struct bl_dm_tree *p;
>>
>> @@ -282,28 +288,28 @@ void bl_dm_remove_tree(uint64_t dev)
>> del_from_bl_dm_tree(dev);
>> }
>>
>> -void bl_dm_create_tree(uint64_t dev)
>> +static int bl_dm_create_tree(uint64_t dev)
>> {
>> struct dm_tree *tree;
>> struct bl_dm_tree *bl_tree;
>>
>> bl_tree = find_bl_dm_tree(dev);
>> if (bl_tree)
>> - return; /* XXX: error? */
>> + return 1;
>>
>> tree = dm_tree_create();
>> if (!tree)
>> - return;
>> + return 0;
>>
>> if (!dm_tree_add_dev(tree, MAJOR(dev), MINOR(dev))) {
>> dm_tree_free(tree);
>> - return;
>> + return 0;
>> }
>>
>> bl_tree = malloc(sizeof(struct bl_dm_tree));
>> if (!bl_tree) {
>> dm_tree_free(tree);
>> - return;
>> + return 0;
>> }
>>
>> bl_tree->dev = dev;
>> @@ -311,29 +317,7 @@ void bl_dm_create_tree(uint64_t dev)
>> bl_tree->next = NULL;
>> add_to_bl_dm_tree(bl_tree);
>>
>> - return;
>> -}
>> -
>> -uint64_t dm_device_nametodev(char *dev_name)
>> -{
>> - struct dm_task *dmt;
>> - int ret = 0;
>> - struct dm_info dminfo;
>> -
>> - dmt = dm_task_create(DM_DEVICE_INFO);
>> - if (!dmt)
>> - return -ENODEV;
>> -
>> - ret = dm_task_set_name(dmt, dev_name) &&
>> - dm_task_run(dmt) && dm_task_get_info(dmt, &dminfo);
>> -
>> - if (dmt)
>> - dm_task_destroy(dmt);
>> -
>> - if (!ret)
>> - return 0;
>> -
>> - return MKDEV(dminfo.major, dminfo.minor);
>> + return 1;
>> }
>>
>> int dm_device_remove_all(uint64_t *dev)
>> @@ -364,6 +348,7 @@ int dm_device_remove_all(uint64_t *dev)
>> ret = dm_tree_deactivate_children(node, uuid, strlen(uuid));
>> dm_task_update_nodes();
>> bl_dm_remove_tree(bl_dev);
>> +
>> return ret;
>> }
>>
>> @@ -372,7 +357,7 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
>> {
>> uint64_t size, dev = 0;
>> unsigned long count = dev_count;
>> - int number = 0, i, pos;
>> + int volnum, i, pos;
>> struct bl_volume *node;
>> char *tmp;
>> struct bl_dm_table *table = NULL;
>> @@ -381,8 +366,8 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
>> char *dev_name = NULL;
>>
>> /* Create pseudo device here */
>> - while (number < num_vols) {
>> - node = &vols[number];
>> + for (volnum = 0; volnum < num_vols; volnum++) {
>> + node = &vols[volnum];
>> switch (node->bv_type) {
>> case BLOCK_VOLUME_SIMPLE:
>> /* Do not need to create device here */
>> @@ -492,16 +477,17 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
>> node->param.bv_dev = dev;
>> /* TODO: extend use with PSEUDO later */
>> node->bv_type = BLOCK_VOLUME_PSEUDO;
>> +
>> continued:
>> - number++;
>> if (bl_table_head)
>> bl_dm_table_free(bl_table_head);
>> bl_table_head = NULL;
>> }
>> out:
>> - if (bl_table_head)
>> + if (bl_table_head) {
>> bl_dm_table_free(bl_table_head);
>> - bl_table_head = NULL;
>> + bl_table_head = NULL;
>> + }
>> if (dev)
>> bl_dm_create_tree(dev);
>> if (dev_name)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Benny Halevy wrote:
>> -static char *pretty_sig(char *sig, int siglen)
>> +static char *pretty_sig(char *sig, uint32_t siglen)
>> {
>> static char rs[100];
>> - unsigned int i;
>> + unsigned long i;
>>
>> - if (siglen <= 4) {
>> + if (siglen <= sizeof i) {
>> memcpy(&i, sig, sizeof i);
>> - sprintf(rs, "0x%0x", i);
>> + sprintf(rs, "0x%0lx", i);
>
> What about machine endianess?
> The MDS and clients may be of different gender, no?
> Also, on 64 bit machines, you may copy 8 bytes while the signature
> is 4-bytes long so you may copy junk into i.
>
> Benny
>
>> } else {
>> + if (siglen > sizeof rs - 1)
>> + siglen = sizeof rs - 1;
Hmm, for courtesy purposes, how about ending the truncated
signature with "..."?
I am glad you are paying attention! I am aware of the shortcomings of
pretty_sig(). In addition to the problems you noted, it also assumes that a
signature over 8 bytes long is representable as a text string, which is not
guaranteed. The code it replaced was worse.
I put this in because for debugging I need to be able to follow a signature
all the way from my EMC server to the devmapper. pretty_sig() simply prints
the signature in a way that I can match it up with the signature on the
server.
I don't want to spend a lot of time on this, but I also am uneasy leaving
EMC-specific code in nfs-utils, especially since it can blow up if you use
it against a non-EMC server. My inclination is to remove this debugging
code when I no longer need it. I guess at the very least I should put in a
comment. I am open to suggestions.
Benny Halevy wrote:
Signed-off-by: Benny Halevy <[email protected]>
---
How about this?
Benny
utils/blkmapd/device-process.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
index 0c5060b..0584bf9 100644
--- a/utils/blkmapd/device-process.c
+++ b/utils/blkmapd/device-process.c
@@ -52,11 +52,17 @@
static char *pretty_sig(char *sig, uint32_t siglen)
{
static char rs[100];
- unsigned long i = 0;
+ uint64_t sigval;
- if (siglen <= sizeof i) {
- memcpy(&i, sig, siglen);
- sprintf(rs, "0x%0lx", i);
+ if (siglen <= sizeof(sigval)) {
+ int i;
+
+ sigval = 0;
+ for (i = 0; i < siglen; i++) {
+ sigval <<= 8;
+ sigval += ((unsigned char *)sig)[i];
+ }
+ sprintf(rs, "0x%0llx", sigval);
} else {
if (siglen > sizeof rs - 1)
siglen = sizeof rs - 1;
I would prefer to print it as little-endian, since that's how it's supplied
by the EMC server.
Signed-off-by: Benny Halevy <[email protected]>
---
How about this?
Benny
utils/blkmapd/device-process.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
index 0c5060b..0584bf9 100644
--- a/utils/blkmapd/device-process.c
+++ b/utils/blkmapd/device-process.c
@@ -52,11 +52,17 @@
static char *pretty_sig(char *sig, uint32_t siglen)
{
static char rs[100];
- unsigned long i = 0;
+ uint64_t sigval;
- if (siglen <= sizeof i) {
- memcpy(&i, sig, siglen);
- sprintf(rs, "0x%0lx", i);
+ if (siglen <= sizeof(sigval)) {
+ int i;
+
+ sigval = 0;
+ for (i = 0; i < siglen; i++) {
+ sigval <<= 8;
+ sigval += ((unsigned char *)sig)[i];
+ }
+ sprintf(rs, "0x%0llx", sigval);
} else {
if (siglen > sizeof rs - 1)
siglen = sizeof rs - 1;
--
1.7.2.3
On 2010-12-02 16:41, Jim Rees wrote:
> Benny Halevy wrote:
>
> >> -static char *pretty_sig(char *sig, int siglen)
> >> +static char *pretty_sig(char *sig, uint32_t siglen)
> >> {
> >> static char rs[100];
> >> - unsigned int i;
> >> + unsigned long i;
> >>
> >> - if (siglen <= 4) {
> >> + if (siglen <= sizeof i) {
> >> memcpy(&i, sig, sizeof i);
> >> - sprintf(rs, "0x%0x", i);
> >> + sprintf(rs, "0x%0lx", i);
> >
> > What about machine endianess?
> > The MDS and clients may be of different gender, no?
> > Also, on 64 bit machines, you may copy 8 bytes while the signature
> > is 4-bytes long so you may copy junk into i.
> >
> > Benny
> >
> >> } else {
> >> + if (siglen > sizeof rs - 1)
> >> + siglen = sizeof rs - 1;
>
> Hmm, for courtesy purposes, how about ending the truncated
> signature with "..."?
>
> I am glad you are paying attention! I am aware of the shortcomings of
> pretty_sig(). In addition to the problems you noted, it also assumes that a
> signature over 8 bytes long is representable as a text string, which is not
> guaranteed. The code it replaced was worse.
>
> I put this in because for debugging I need to be able to follow a signature
> all the way from my EMC server to the devmapper. pretty_sig() simply prints
> the signature in a way that I can match it up with the signature on the
> server.
>
> I don't want to spend a lot of time on this, but I also am uneasy leaving
> EMC-specific code in nfs-utils, especially since it can blow up if you use
> it against a non-EMC server. My inclination is to remove this debugging
> code when I no longer need it. I guess at the very least I should put in a
> comment. I am open to suggestions.
Why can't it always dump the signature in hex, even if it happens to be ASCII?
Benny
Benny Halevy wrote:
Like this?
diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
index ea8b8ec..0d8705f 100644
--- a/utils/blkmapd/device-process.c
+++ b/utils/blkmapd/device-process.c
@@ -58,10 +58,8 @@ static char *pretty_sig(char *sig, uint32_t siglen)
int i;
sigval = 0;
- for (i = 0; i < siglen; i++) {
- sigval <<= 8;
- sigval += ((unsigned char *)sig)[i];
- }
+ for (i = 0; i < siglen; i++)
+ sigval |= ((unsigned char *)sig)[i] << (i * 8);
sprintf(rs, "0x%0llx", sigval);
} else {
if (siglen > sizeof rs - 4) {
That works for me, yes.
On 2010-11-30 21:14, Jim Rees wrote:
> Signed-off-by: Jim Rees <[email protected]>
> ---
> utils/blkmapd/device-discovery.c | 1 +
> utils/blkmapd/device-discovery.h | 11 +--
> utils/blkmapd/device-inq.c | 7 +-
> utils/blkmapd/device-process.c | 31 ++++---
> utils/blkmapd/dm-device.c | 164 +++++++++++++++++--------------------
> 5 files changed, 103 insertions(+), 111 deletions(-)
>
> diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c
> index 0497a11..c2675b8 100644
> --- a/utils/blkmapd/device-discovery.c
> +++ b/utils/blkmapd/device-discovery.c
> @@ -39,6 +39,7 @@
> #include <stdlib.h>
> #include <stdio.h>
> #include <string.h>
> +#include <syslog.h>
> #include <dirent.h>
> #include <ctype.h>
> #include <fcntl.h>
> diff --git a/utils/blkmapd/device-discovery.h b/utils/blkmapd/device-discovery.h
> index 8cf88b8..a0937b1 100644
> --- a/utils/blkmapd/device-discovery.h
> +++ b/utils/blkmapd/device-discovery.h
> @@ -28,11 +28,10 @@
> #define BL_DEVICE_DISCOVERY_H
>
> #include <stdint.h>
> -#include <syslog.h>
>
> enum blk_vol_type {
> BLOCK_VOLUME_SIMPLE = 0, /* maps to a single LU */
> - BLOCK_VOLUME_SLICE = 1, /* slice of another volume */
> + BLOCK_VOLUME_SLICE = 1, /* slice of another volume */
> BLOCK_VOLUME_CONCAT = 2, /* concatenation of multiple volumes */
> BLOCK_VOLUME_STRIPE = 3, /* striped across multiple volumes */
> BLOCK_VOLUME_PSEUDO = 4,
> @@ -45,15 +44,15 @@ struct bl_volume {
> struct bl_volume **bv_vols;
> int bv_vol_n;
> union {
> - dev_t bv_dev; /* for BLOCK_VOLUME_SIMPLE(PSEUDO) */
> + dev_t bv_dev; /* for BLOCK_VOLUME_SIMPLE(PSEUDO) */
> off_t bv_stripe_unit; /* for BLOCK_VOLUME_STRIPE(CONCAT) */
> off_t bv_offset; /* for BLOCK_VOLUME_SLICE */
> } param;
> };
>
> struct bl_sig_comp {
> - int64_t bs_offset; /* In bytes */
> - uint32_t bs_length; /* In bytes */
> + uint64_t bs_offset; /* In bytes */
> + uint32_t bs_length; /* In bytes */
> char *bs_string;
> };
>
> @@ -107,7 +106,7 @@ struct pipefs_hdr {
> uint32_t msgid;
> uint8_t type;
> uint8_t flags;
> - uint16_t totallen; /* length of entire message, including hdr */
> + uint16_t totallen; /* length of entire message, including hdr */
> uint32_t status;
> };
>
> diff --git a/utils/blkmapd/device-inq.c b/utils/blkmapd/device-inq.c
> index e1c0128..eabc70c 100644
> --- a/utils/blkmapd/device-inq.c
> +++ b/utils/blkmapd/device-inq.c
> @@ -39,11 +39,12 @@
>
> #include <stdlib.h>
> #include <stdio.h>
> +#include <unistd.h>
> #include <string.h>
> +#include <syslog.h>
> #include <dirent.h>
> #include <ctype.h>
> #include <fcntl.h>
> -#include <unistd.h>
> #include <libgen.h>
> #include <errno.h>
>
> @@ -174,10 +175,10 @@ extern enum bl_path_state_e bldev_read_ap_state(int fd)
> struct bl_serial *bldev_read_serial(int fd, const char *filename)
> {
> struct bl_serial *serial_out = NULL;
> - int status = 0, pos, len;
> + int status = 0;
> char *buffer;
> struct bl_dev_id *dev_root, *dev_id;
> - unsigned int current_id = 0;
> + unsigned int pos, len, current_id = 0;
>
> status = bldev_inquire_pages(fd, 0x83, &buffer);
> if (status)
> diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
> index 4482bd5..4ce47e1 100644
> --- a/utils/blkmapd/device-process.c
> +++ b/utils/blkmapd/device-process.c
> @@ -33,29 +33,33 @@
> * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> -#include <libdevmapper.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/user.h>
> +#include <arpa/inet.h>
> +#include <linux/kdev_t.h>
> +
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <sys/user.h>
> +#include <syslog.h>
> #include <fcntl.h>
> #include <errno.h>
> -#include <arpa/inet.h>
> -#include <linux/kdev_t.h>
> +
> #include "device-discovery.h"
>
> -static char *pretty_sig(char *sig, int siglen)
> +static char *pretty_sig(char *sig, uint32_t siglen)
> {
> static char rs[100];
> - unsigned int i;
> + unsigned long i;
>
> - if (siglen <= 4) {
> + if (siglen <= sizeof i) {
> memcpy(&i, sig, sizeof i);
> - sprintf(rs, "0x%0x", i);
> + sprintf(rs, "0x%0lx", i);
What about machine endianess?
The MDS and clients may be of different gender, no?
Also, on 64 bit machines, you may copy 8 bytes while the signature
is 4-bytes long so you may copy junk into i.
Benny
> } else {
> + if (siglen > sizeof rs - 1)
> + siglen = sizeof rs - 1;
> memcpy(rs, sig, siglen);
> rs[siglen] = '\0';
> }
> @@ -65,6 +69,7 @@ static char *pretty_sig(char *sig, int siglen)
> uint32_t *blk_overflow(uint32_t * p, uint32_t * end, size_t nbytes)
> {
> uint32_t *q = p + ((nbytes + 3) >> 2);
> +
> if (q > end || q < p)
> return NULL;
> return p;
> @@ -73,8 +78,8 @@ uint32_t *blk_overflow(uint32_t * p, uint32_t * end, size_t nbytes)
> static int decode_blk_signature(uint32_t ** pp, uint32_t * end,
> struct bl_sig *sig)
> {
> - int i, siglen;
> - uint32_t *p = *pp;
> + int i;
> + uint32_t siglen, *p = *pp;
>
> BLK_READBUF(p, end, 4);
> READ32(sig->si_num_comps);
> @@ -288,7 +293,7 @@ static int decode_blk_volume(uint32_t **pp, uint32_t *end,
> off_t chunksize = vol->param.bv_stripe_unit;
> if ((chunksize == 0) ||
> ((chunksize & (chunksize - 1)) != 0) ||
> - (chunksize < (PAGE_SIZE >> 9)))
> + (chunksize < (off_t)(PAGE_SIZE >> 9)))
> return -EIO;
> BLK_READBUF(p, end, 4);
> READ32(vol->bv_vol_n);
> diff --git a/utils/blkmapd/dm-device.c b/utils/blkmapd/dm-device.c
> index 14ad131..5a9f5ec 100644
> --- a/utils/blkmapd/dm-device.c
> +++ b/utils/blkmapd/dm-device.c
> @@ -24,15 +24,19 @@
> * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
> -#include <libdevmapper.h>
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <linux/kdev_t.h>
> +
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> +#include <syslog.h>
> #include <fcntl.h>
> #include <errno.h>
> -#include <linux/kdev_t.h>
> +#include <libdevmapper.h>
> +
> #include "device-discovery.h"
>
> #define DM_DEV_NAME_LEN 256
> @@ -66,9 +70,10 @@ static inline struct bl_dm_table *bl_dm_table_alloc(void)
> return (struct bl_dm_table *)calloc(1, sizeof(struct bl_dm_table));
> }
>
> -void bl_dm_table_free(struct bl_dm_table *bl_table_head)
> +static void bl_dm_table_free(struct bl_dm_table *bl_table_head)
> {
> - struct bl_dm_table *p = bl_table_head;
> + struct bl_dm_table *p;
> +
> while (bl_table_head) {
> p = bl_table_head->next;
> free(bl_table_head);
> @@ -76,41 +81,39 @@ void bl_dm_table_free(struct bl_dm_table *bl_table_head)
> }
> }
>
> -void add_to_bl_dm_table(struct bl_dm_table **bl_table_head,
> +static void add_to_bl_dm_table(struct bl_dm_table **bl_table_head,
> struct bl_dm_table *table)
> {
> - struct bl_dm_table *pre;
> + struct bl_dm_table *p;
> +
> if (!*bl_table_head) {
> *bl_table_head = table;
> return;
> }
> - pre = *bl_table_head;
> - while (pre->next)
> - pre = pre->next;
> - pre->next = table;
> - return;
> + p = *bl_table_head;
> + while (p->next)
> + p = p->next;
> + p->next = table;
> }
>
> struct bl_dm_tree *bl_tree_head;
>
> -struct bl_dm_tree *find_bl_dm_tree(uint64_t dev)
> +static struct bl_dm_tree *find_bl_dm_tree(uint64_t dev)
> {
> - struct bl_dm_tree *p = bl_tree_head;
> - while (p) {
> + struct bl_dm_tree *p;
> +
> + for (p = bl_tree_head; p; p = p->next) {
> if (p->dev == dev)
> - return p;
> - p = p->next;
> + break;
> }
> - return NULL;
> + return p;
> }
>
> -void del_from_bl_dm_tree(uint64_t dev)
> +static void del_from_bl_dm_tree(uint64_t dev)
> {
> - struct bl_dm_tree *pre = bl_tree_head;
> - struct bl_dm_tree *p;
> + struct bl_dm_tree *p, *pre = bl_tree_head;
>
> - p = pre;
> - while (p) {
> + for (p = pre; p; p = p->next) {
> if (p->dev == dev) {
> pre->next = p->next;
> if (p == bl_tree_head)
> @@ -119,29 +122,31 @@ void del_from_bl_dm_tree(uint64_t dev)
> break;
> }
> pre = p;
> - p = pre->next;
> }
> }
>
> -void add_to_bl_dm_tree(struct bl_dm_tree *tree)
> +static void add_to_bl_dm_tree(struct bl_dm_tree *tree)
> {
> - struct bl_dm_tree *pre;
> + struct bl_dm_tree *p;
> +
> if (!bl_tree_head) {
> bl_tree_head = tree;
> return;
> }
> - pre = bl_tree_head;
> - while (pre->next)
> - pre = pre->next;
> - pre->next = tree;
> + p = bl_tree_head;
> + while (p->next)
> + p = p->next;
> + p->next = tree;
> return;
> }
>
> -/* Create device via device mapper
> +/*
> + * Create device via device mapper
> * return 0 when creation failed
> * return dev no for created device
> */
> -uint64_t dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
> +static uint64_t
> +dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
> {
> struct dm_task *dmt;
> struct dm_info dminfo;
> @@ -182,21 +187,19 @@ uint64_t dm_single_device_create(const char *dev_name, struct bl_dm_table * p)
> return MKDEV(dminfo.major, dminfo.minor);
> }
>
> -int dm_device_remove_byname(const char *dev_name)
> +static int dm_device_remove_byname(const char *dev_name)
> {
> struct dm_task *dmt;
> int ret = 0;
>
> dmt = dm_task_create(DM_DEVICE_REMOVE);
> if (!dmt)
> - return -ENODEV;
> + return 0;
>
> ret = dm_task_set_name(dmt, dev_name) && dm_task_run(dmt);
>
> dm_task_update_nodes();
> -
> - if (dmt)
> - dm_task_destroy(dmt);
> + dm_task_destroy(dmt);
>
> return ret;
> }
> @@ -205,61 +208,64 @@ int dm_device_remove(uint64_t dev)
> {
> struct dm_task *dmt;
> struct dm_names *dmnames;
> - char *names = NULL;
> - int ret = -1;
> + char *name = NULL;
> + int ret = 0;
>
> /* Look for dev_name via dev, if dev_name could be transferred here,
> we could jump to DM_DEVICE_REMOVE directly */
> +
> dmt = dm_task_create(DM_DEVICE_LIST);
> if (!dmt) {
> BL_LOG_ERR("dm_task creation failed\n");
> - return -ENODEV;
> + goto out;
> }
>
> ret = dm_task_run(dmt);
> if (!ret) {
> BL_LOG_ERR("dm_task_run failed\n");
> - goto error;
> + goto out;
> }
>
> dmnames = dm_task_get_names(dmt);
> if (!dmnames || !dmnames->dev) {
> BL_LOG_ERR("dm_task_get_names failed\n");
> - goto error;
> + goto out;
> }
>
> - do {
> + while (dmnames) {
> if (dmnames->dev == dev) {
> - names = dmnames->name;
> + name = dmnames->name;
> break;
> }
> dmnames = (void *)dmnames + dmnames->next;
> - } while (dmnames);
> + }
>
> - if (!names) {
> + if (!name) {
> BL_LOG_ERR("Could not find device\n");
> - goto error;
> + goto out;
> }
>
> dm_task_update_nodes();
>
> - error:
> - dm_task_destroy(dmt);
> + out:
> + if (dmt)
> + dm_task_destroy(dmt);
>
> /* Start to remove device */
> - if (names)
> - ret = dm_device_remove_byname(names);
> + if (name)
> + ret = dm_device_remove_byname(name);
> +
> return ret;
> }
>
> static unsigned long dev_count;
>
> -void dm_devicelist_remove(unsigned long start, unsigned long end)
> +static void dm_devicelist_remove(unsigned long start, unsigned long end)
> {
> char dev_name[DM_DEV_NAME_LEN];
> unsigned long count;
>
> - if ((start >= dev_count) || (end <= 1) || (start >= end - 1))
> + if (start >= dev_count || end <= 1 || start >= end - 1)
> return;
>
> for (count = end - 1; count > start; count--) {
> @@ -270,7 +276,7 @@ void dm_devicelist_remove(unsigned long start, unsigned long end)
> return;
> }
>
> -void bl_dm_remove_tree(uint64_t dev)
> +static void bl_dm_remove_tree(uint64_t dev)
> {
> struct bl_dm_tree *p;
>
> @@ -282,28 +288,28 @@ void bl_dm_remove_tree(uint64_t dev)
> del_from_bl_dm_tree(dev);
> }
>
> -void bl_dm_create_tree(uint64_t dev)
> +static int bl_dm_create_tree(uint64_t dev)
> {
> struct dm_tree *tree;
> struct bl_dm_tree *bl_tree;
>
> bl_tree = find_bl_dm_tree(dev);
> if (bl_tree)
> - return; /* XXX: error? */
> + return 1;
>
> tree = dm_tree_create();
> if (!tree)
> - return;
> + return 0;
>
> if (!dm_tree_add_dev(tree, MAJOR(dev), MINOR(dev))) {
> dm_tree_free(tree);
> - return;
> + return 0;
> }
>
> bl_tree = malloc(sizeof(struct bl_dm_tree));
> if (!bl_tree) {
> dm_tree_free(tree);
> - return;
> + return 0;
> }
>
> bl_tree->dev = dev;
> @@ -311,29 +317,7 @@ void bl_dm_create_tree(uint64_t dev)
> bl_tree->next = NULL;
> add_to_bl_dm_tree(bl_tree);
>
> - return;
> -}
> -
> -uint64_t dm_device_nametodev(char *dev_name)
> -{
> - struct dm_task *dmt;
> - int ret = 0;
> - struct dm_info dminfo;
> -
> - dmt = dm_task_create(DM_DEVICE_INFO);
> - if (!dmt)
> - return -ENODEV;
> -
> - ret = dm_task_set_name(dmt, dev_name) &&
> - dm_task_run(dmt) && dm_task_get_info(dmt, &dminfo);
> -
> - if (dmt)
> - dm_task_destroy(dmt);
> -
> - if (!ret)
> - return 0;
> -
> - return MKDEV(dminfo.major, dminfo.minor);
> + return 1;
> }
>
> int dm_device_remove_all(uint64_t *dev)
> @@ -364,6 +348,7 @@ int dm_device_remove_all(uint64_t *dev)
> ret = dm_tree_deactivate_children(node, uuid, strlen(uuid));
> dm_task_update_nodes();
> bl_dm_remove_tree(bl_dev);
> +
> return ret;
> }
>
> @@ -372,7 +357,7 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
> {
> uint64_t size, dev = 0;
> unsigned long count = dev_count;
> - int number = 0, i, pos;
> + int volnum, i, pos;
> struct bl_volume *node;
> char *tmp;
> struct bl_dm_table *table = NULL;
> @@ -381,8 +366,8 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
> char *dev_name = NULL;
>
> /* Create pseudo device here */
> - while (number < num_vols) {
> - node = &vols[number];
> + for (volnum = 0; volnum < num_vols; volnum++) {
> + node = &vols[volnum];
> switch (node->bv_type) {
> case BLOCK_VOLUME_SIMPLE:
> /* Do not need to create device here */
> @@ -492,16 +477,17 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
> node->param.bv_dev = dev;
> /* TODO: extend use with PSEUDO later */
> node->bv_type = BLOCK_VOLUME_PSEUDO;
> +
> continued:
> - number++;
> if (bl_table_head)
> bl_dm_table_free(bl_table_head);
> bl_table_head = NULL;
> }
> out:
> - if (bl_table_head)
> + if (bl_table_head) {
> bl_dm_table_free(bl_table_head);
> - bl_table_head = NULL;
> + bl_table_head = NULL;
> + }
> if (dev)
> bl_dm_create_tree(dev);
> if (dev_name)
On 2010-12-02 18:24, Jim Rees wrote:
> Benny Halevy wrote:
>
> Signed-off-by: Benny Halevy <[email protected]>
> ---
>
> How about this?
>
> Benny
>
> utils/blkmapd/device-process.c | 14 ++++++++++----
> 1 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
> index 0c5060b..0584bf9 100644
> --- a/utils/blkmapd/device-process.c
> +++ b/utils/blkmapd/device-process.c
> @@ -52,11 +52,17 @@
> static char *pretty_sig(char *sig, uint32_t siglen)
> {
> static char rs[100];
> - unsigned long i = 0;
> + uint64_t sigval;
>
> - if (siglen <= sizeof i) {
> - memcpy(&i, sig, siglen);
> - sprintf(rs, "0x%0lx", i);
> + if (siglen <= sizeof(sigval)) {
> + int i;
> +
> + sigval = 0;
> + for (i = 0; i < siglen; i++) {
> + sigval <<= 8;
> + sigval += ((unsigned char *)sig)[i];
> + }
> + sprintf(rs, "0x%0llx", sigval);
> } else {
> if (siglen > sizeof rs - 1)
> siglen = sizeof rs - 1;
>
> I would prefer to print it as little-endian, since that's how it's supplied
> by the EMC server.
Like this?
diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c
index ea8b8ec..0d8705f 100644
--- a/utils/blkmapd/device-process.c
+++ b/utils/blkmapd/device-process.c
@@ -58,10 +58,8 @@ static char *pretty_sig(char *sig, uint32_t siglen)
int i;
sigval = 0;
- for (i = 0; i < siglen; i++) {
- sigval <<= 8;
- sigval += ((unsigned char *)sig)[i];
- }
+ for (i = 0; i < siglen; i++)
+ sigval |= ((unsigned char *)sig)[i] << (i * 8);
sprintf(rs, "0x%0llx", sigval);
} else {
if (siglen > sizeof rs - 4) {
Merged in pnfs-nfs-utils-1-2-4-rc3, thanks!
Benny
On 2010-11-30 21:13, Jim Rees wrote:
> Signed-off-by: Jim Rees <[email protected]>
> ---
> .gitignore | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 4bff9e3..6530ae0 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -36,6 +36,7 @@ support/include/stamp-h1
> lib*.a
> tools/rpcgen/rpcgen
> tools/rpcdebug/rpcdebug
> +utils/blkmapd/blkmapd
> utils/exportfs/exportfs
> utils/idmapd/idmapd
> utils/lockd/lockd
> @@ -48,6 +49,7 @@ utils/rquotad/rquotad
> utils/rquotad/rquota.h
> utils/rquotad/rquota_xdr.c
> utils/showmount/showmount
> +utils/spnfsd/spnfsd
> utils/statd/statd
> tools/locktest/testlk
> tools/getiversion/getiversion
Merged in pnfs-nfs-utils-1-2-4-rc3, thanks!
Benny
On 2010-11-30 21:13, Jim Rees wrote:
> Signed-off-by: Jim Rees <[email protected]>
> ---
> utils/blkmapd/etc/blkmapd.conf | 10 ----------
> 1 files changed, 0 insertions(+), 10 deletions(-)
> delete mode 100644 utils/blkmapd/etc/blkmapd.conf
>
> diff --git a/utils/blkmapd/etc/blkmapd.conf b/utils/blkmapd/etc/blkmapd.conf
> deleted file mode 100644
> index da70d94..0000000
> --- a/utils/blkmapd/etc/blkmapd.conf
> +++ /dev/null
> @@ -1,10 +0,0 @@
> -# This is an example config file
> -
> -# Look at all /dev/sd* devices
> -# /dev/sd or /dev/sd*
> -/dev/sd*
> -
> -# Look at all /dev/mapper/* devices
> -# /dev/mapper/* or
> -# /dev/mapper/
> -/dev/mapper/*