[PATCH 0/5] nfs-utils: various changes

[PATCH 0/5] nfs-utils: various changes

[Jim Rees]

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

[PATCH 1/5] add blkmapd and spnfsd to list of build targets to ignore

[Jim Rees]

Signed-off-by: Jim Rees <rees@umich.edu>
---
 .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

[PATCH 2/5] Remove blkmapd config file, which is no longer used.

[Jim Rees]

Signed-off-by: Jim Rees <rees@umich.edu>
---
 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

[PATCH 3/5] disk signature fixes

[Jim Rees]

re-write decode_blk_signature to keep fd open
pretty print disk signature
remove unneeded syslog info messages

Signed-off-by: Jim Rees <rees@umich.edu>
---
 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

[PATCH 4/5] various minor cleanups

[Jim Rees]

Signed-off-by: Jim Rees <rees@umich.edu>
---
 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

[PATCH 5/5] device mapping fixes

[Jim Rees]

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 <rees@umich.edu>
---
 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

Re: [PATCH 4/5] various minor cleanups

[Benny Halevy]

On 2010-11-30 21:14, Jim Rees wrote:
> Signed-off-by: Jim Rees <rees@umich.edu>
> ---
>  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)

Re: [PATCH 1/5] add blkmapd and spnfsd to list of build targets to ignore

[Benny Halevy]

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 <rees@umich.edu>
> ---
>  .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

Re: [PATCH 2/5] Remove blkmapd config file, which is no longer used.

[Benny Halevy]

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 <rees@umich.edu>
> ---
>  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/*

Re: [PATCH 4/5] various minor cleanups

[Benny Halevy]

On 2010-12-02 15:59, Benny Halevy wrote:
> On 2010-11-30 21:14, Jim Rees wrote:
>> Signed-off-by: Jim Rees <rees@umich.edu>
>> ---
>>  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] SQUASHME: blkmapd: fix pretty_sig short sig endianess agnosticity

[Benny Halevy]

Signed-off-by: Benny Halevy <bhalevy@panasas.com>
---

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

[PATCH] SQUASHME: decorate truncated signatures with "..."

[Benny Halevy]

Signed-off-by: Benny Halevy <bhalevy@panasas.com>
---
 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

Re: [PATCH 4/5] various minor cleanups

[Jim Rees]

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.

Re: [PATCH 4/5] various minor cleanups

[Benny Halevy]

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

Re: [PATCH 4/5] various minor cleanups

[Jim Rees]

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

Re: [PATCH] SQUASHME: blkmapd: fix pretty_sig short sig endianess agnosticity

[Jim Rees]

Benny Halevy wrote:

  Signed-off-by: Benny Halevy <bhalevy@panasas.com>
  ---
  
  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.

Re: [PATCH] SQUASHME: blkmapd: fix pretty_sig short sig endianess agnosticity

[Benny Halevy]

On 2010-12-02 18:24, Jim Rees wrote:
> Benny Halevy wrote:
> 
>   Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>   ---
>   
>   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) {

Re: [PATCH] SQUASHME: blkmapd: fix pretty_sig short sig endianess agnosticity

[Jim Rees]

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.