Skip to contentSkip to conversation

nfs-utils: fix stripe device size calculation for layouts not a multiple of stripe unit

PATCH In review Jim Rees 2 participants 2 messages Feb 23, 2011 – Feb 24, 2011 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

Jim Rees

author
#0 lore raw

PATCH 1/1 nfs-utils: fix stripe device size calculation for layouts not a multiple of stripe unit

If a complex layout has a stripe volume in it, and that volume has a size that is not a multiple of the stripe unit, the total layout size will be wrong because it is based on the truncated stripe volume size. Fix this by using the truncated size for making the mapped device, and the untruncated size in the table for subsequent devices to use.

Also remove the warning for stripe volumes not a multiple of the stripe width, since this is legal and apparently common.

While we're at it, use variable names consistent with the terminology in rfc5661 section 13.2.

---
 utils/blkmapd/device-process.c |   10 ++++------
 utils/blkmapd/dm-device.c      |   21 +++++----------------
 2 files changed, 9 insertions(+), 22 deletions(-)
utils/blkmapd/device-process.c +4 -6
index 9a78457..79e596d 100644
@@ -295,10 +295,10 @@ decode_blk_volume(uint32_t **pp, uint32_t *end, struct bl_volume *vols, int voln
 	case BLOCK_VOLUME_STRIPE:
 		BLK_READBUF(p, end, 8);
 		READ_SECTOR(vol->param.bv_stripe_unit);
-		off_t chunksize = vol->param.bv_stripe_unit;
+		off_t stripe_unit = vol->param.bv_stripe_unit;
 		/* Check limitations imposed by device-mapper */
-		if ((chunksize & (chunksize - 1)) != 0
-		    || chunksize < (off_t) (PAGE_SIZE >> 9))
+		if ((stripe_unit & (stripe_unit - 1)) != 0
+		    || stripe_unit < (off_t) (PAGE_SIZE >> 9))
 			return -EIO;
 		BLK_READBUF(p, end, 4);
 		READ32(vol->bv_vol_n);
@@ -306,7 +306,7 @@ decode_blk_volume(uint32_t **pp, uint32_t *end, struct bl_volume *vols, int voln
 			return -EIO;
 		*array_cnt = vol->bv_vol_n;
 		BL_LOG_INFO("%s: stripe %d nvols=%d unit=%ld\n", __func__, voln,
-			    vol->bv_vol_n, (long)chunksize);
+			    vol->bv_vol_n, (long)stripe_unit);
 		status = set_vol_array(&p, end, vols, voln);
 		if (status)
 			return status;
@@ -317,9 +317,7 @@ decode_blk_volume(uint32_t **pp, uint32_t *end, struct bl_volume *vols, int voln
 				return -EIO;
 			}
 		}
-		/* Truncate size to a stripe unit boundary */
 		vol->bv_size = vol->bv_vols[0]->bv_size * vol->bv_vol_n;
-		vol->bv_size &= ~(chunksize - 1);
 		break;
 	case BLOCK_VOLUME_CONCAT:
 		BLK_READBUF(p, end, 4);
utils/blkmapd/dm-device.c +5 -16
index c4fe4e9..0f4f148 100644
@@ -369,7 +369,7 @@ static int dm_device_exists(char *dev_name)
 /* 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, stripe_unit, stripe_size, nstripes, dev = 0;
+	uint64_t size, stripe_unit, dev = 0;
 	unsigned int count = dev_count;
 	int volnum, i, pos;
 	struct bl_volume *node;
@@ -414,21 +414,10 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
 			if (!table)
 				goto out;
 			table->offset = 0;
-			stripe_unit = node->param.bv_stripe_unit << 9;
-			stripe_size = stripe_unit * node->bv_vol_n;
-			nstripes = node->bv_size * node->bv_vol_n / stripe_size;
-			/* Make sure total size is a multiple of stripe size */
-			size = node->bv_size;
-			if (size % stripe_size != 0) {
-				/* XXX Should this be an error? */
-				BL_LOG_WARNING(
-					"%s: %d units of %llu bytes is not a multiple of %lld stripe size\n",
-					__func__, node->bv_vol_n,
-					(long long unsigned) node->bv_size,
-					(long long unsigned) stripe_size);
-				size = nstripes * stripe_size;
-			}
-			table->size = size;
+			/* Truncate size to a stripe unit boundary */
+			stripe_unit = node->param.bv_stripe_unit;
+			table->size =
+			    node->bv_size - (node->bv_size % stripe_unit);
 			strcpy(table->target_type, "striped");
 			sprintf(table->params, "%d %llu %n", node->bv_vol_n,
 				(long long unsigned) stripe_unit, &pos);
Signature · 1 line
1.7.1

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

Looks good to me.
Merged at pnfs-nfs-utils-1-2-4-rc5-2011-02-23

Benny

Quoted 94 lines · Jim Rees

If a complex layout has a stripe volume in it, and that volume has a size that is not a multiple of the stripe unit, the total layout size will be wrong because it is based on the truncated stripe volume size. Fix this by using the truncated size for making the mapped device, and the untruncated size in the table for subsequent devices to use.

Also remove the warning for stripe volumes not a multiple of the stripe width, since this is legal and apparently common.

While we're at it, use variable names consistent with the terminology in rfc5661 section 13.2.

---
 utils/blkmapd/device-process.c |   10 ++++------
 utils/blkmapd/dm-device.c      |   21 +++++----------------
 2 files changed, 9 insertions(+), 22 deletions(-)
utils/blkmapd/device-process.c +4 -6
index 9a78457..79e596d 100644
@@ -295,10 +295,10 @@ decode_blk_volume(uint32_t **pp, uint32_t *end, struct bl_volume *vols, int voln
 	case BLOCK_VOLUME_STRIPE:
 		BLK_READBUF(p, end, 8);
 		READ_SECTOR(vol->param.bv_stripe_unit);
-		off_t chunksize = vol->param.bv_stripe_unit;
+		off_t stripe_unit = vol->param.bv_stripe_unit;
 		/* Check limitations imposed by device-mapper */
-		if ((chunksize & (chunksize - 1)) != 0
-		    || chunksize < (off_t) (PAGE_SIZE >> 9))
+		if ((stripe_unit & (stripe_unit - 1)) != 0
+		    || stripe_unit < (off_t) (PAGE_SIZE >> 9))
 			return -EIO;
 		BLK_READBUF(p, end, 4);
 		READ32(vol->bv_vol_n);
@@ -306,7 +306,7 @@ decode_blk_volume(uint32_t **pp, uint32_t *end, struct bl_volume *vols, int voln
 			return -EIO;
 		*array_cnt = vol->bv_vol_n;
 		BL_LOG_INFO("%s: stripe %d nvols=%d unit=%ld\n", __func__, voln,
-			    vol->bv_vol_n, (long)chunksize);
+			    vol->bv_vol_n, (long)stripe_unit);
 		status = set_vol_array(&p, end, vols, voln);
 		if (status)
 			return status;
@@ -317,9 +317,7 @@ decode_blk_volume(uint32_t **pp, uint32_t *end, struct bl_volume *vols, int voln
 				return -EIO;
 			}
 		}
-		/* Truncate size to a stripe unit boundary */
 		vol->bv_size = vol->bv_vols[0]->bv_size * vol->bv_vol_n;
-		vol->bv_size &= ~(chunksize - 1);
 		break;
 	case BLOCK_VOLUME_CONCAT:
 		BLK_READBUF(p, end, 4);
utils/blkmapd/dm-device.c +5 -16
index c4fe4e9..0f4f148 100644
@@ -369,7 +369,7 @@ static int dm_device_exists(char *dev_name)
 /* 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, stripe_unit, stripe_size, nstripes, dev = 0;
+	uint64_t size, stripe_unit, dev = 0;
 	unsigned int count = dev_count;
 	int volnum, i, pos;
 	struct bl_volume *node;
@@ -414,21 +414,10 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols)
 			if (!table)
 				goto out;
 			table->offset = 0;
-			stripe_unit = node->param.bv_stripe_unit << 9;
-			stripe_size = stripe_unit * node->bv_vol_n;
-			nstripes = node->bv_size * node->bv_vol_n / stripe_size;
-			/* Make sure total size is a multiple of stripe size */
-			size = node->bv_size;
-			if (size % stripe_size != 0) {
-				/* XXX Should this be an error? */
-				BL_LOG_WARNING(
-					"%s: %d units of %llu bytes is not a multiple of %lld stripe size\n",
-					__func__, node->bv_vol_n,
-					(long long unsigned) node->bv_size,
-					(long long unsigned) stripe_size);
-				size = nstripes * stripe_size;
-			}
-			table->size = size;
+			/* Truncate size to a stripe unit boundary */
+			stripe_unit = node->param.bv_stripe_unit;
+			table->size =
+			    node->bv_size - (node->bv_size % stripe_unit);
 			strcpy(table->target_type, "striped");
 			sprintf(table->params, "%d %llu %n", node->bv_vol_n,
 				(long long unsigned) stripe_unit, &pos);