[sheepdog] [PATCH] sheep: handle VID overflow correctly

Hitoshi Mitake mitake.hitoshi at lab.ntt.co.jp
Mon Feb 2 10:18:52 CET 2015


Current sheep cannot handle a case like this:
1. iterate snapshot creation and let latest working VDI have VID 0xffffff
2. create one more snapshot

(The situation can be reproduced with the below sequence:
 $ dog vdi create 00471718 1G
 $ dog vdi snapshot 00471718   (repeat 7 times) )

In this case, new VID becomes 0x000000. Current fill_vdi_info() and
fill_vdi_info_range() cannot handle this case. It comes from the below
two reasons:
1. Recent change 00ecfb24ee46f2 introduced a bug which breaks
   fill_vdi_info_range() in a case of underflow of its variable i.
2. fill_vdi_info_range() assumes that its parameters, left and right,
   are obtained from get_vdi_bitmap_range(). get_vdi_bitmap_range()
   obtains left and right which mean the range of existing VIDs is
   [left, right), in other words, [left, right - 1]. So
   fill_vdi_info_range() starts checking from right - 1 to left. But
   it means fill_vdi_info_range() cannot check VID 0xffffff even VID
   overflow happens. So this patch lets fill_vdi_info_range() check
   from right to left, and also change callers' manner (it passes left
   and right - 1 in ordinal cases).

Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
---
 sheep/vdi.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/sheep/vdi.c b/sheep/vdi.c
index 2889df6..3d14ccf 100644
--- a/sheep/vdi.c
+++ b/sheep/vdi.c
@@ -1404,9 +1404,11 @@ static int fill_vdi_info_range(uint32_t left, uint32_t right,
 		ret = SD_RES_NO_MEM;
 		goto out;
 	}
-	for (i = right - 1; i && i >= left; i--) {
+
+	i = right;
+	while (i >= left) {
 		if (!test_bit(i, sys->vdi_inuse))
-			continue;
+			goto next;
 
 		ret = sd_read_object(vid_to_vdi_oid(i), (char *)inode,
 				     SD_INODE_HEADER_SIZE, 0);
@@ -1420,10 +1422,10 @@ static int fill_vdi_info_range(uint32_t left, uint32_t right,
 				/* Read, delete, clone on snapshots */
 				if (!vdi_is_snapshot(inode)) {
 					vdi_found = true;
-					continue;
+					goto next;
 				}
 				if (!vdi_tag_match(iocb, inode))
-					continue;
+					goto next;
 			} else {
 				/*
 				 * Rollback & snap create, read, delete on
@@ -1438,6 +1440,10 @@ static int fill_vdi_info_range(uint32_t left, uint32_t right,
 			info->vid = inode->vdi_id;
 			goto out;
 		}
+next:
+		if (!i)
+			break;
+		i--;
 	}
 	ret = vdi_found ? SD_RES_NO_TAG : SD_RES_NO_VDI;
 out:
@@ -1452,13 +1458,23 @@ static int fill_vdi_info(unsigned long left, unsigned long right,
 	int ret;
 
 	if (left < right)
-		return fill_vdi_info_range(left, right, iocb, info);
+		return fill_vdi_info_range(left, right - 1, iocb, info);
+
+	if (!right)
+		/*
+		 * a special case right == 0
+		 * because the variables left and right have values obtained by
+		 * get_vdi_bitmap_range(), they mean used bitmap range is
+		 * [left, right). If right == 0, it means used bitmap range is
+		 * [left, SD_NR_VDIS].
+		 */
+		return fill_vdi_info_range(left, SD_NR_VDIS, iocb, info);
 
-	ret = fill_vdi_info_range(0, right, iocb, info);
+	ret = fill_vdi_info_range(0, right - 1, iocb, info);
 	switch (ret) {
 	case SD_RES_NO_VDI:
 	case SD_RES_NO_TAG:
-		ret = fill_vdi_info_range(left, SD_NR_VDIS - 1, iocb, info);
+		ret = fill_vdi_info_range(left, SD_NR_VDIS, iocb, info);
 		break;
 	default:
 		break;
-- 
1.9.1




More information about the sheepdog mailing list