On 2015/02/02 18:18, Hitoshi Mitake wrote:
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) )
Thank you for your patch, but it seems to have some issues.
If the snapshot whose vid is 0x0 is deleted after the above sequence,
deleting other snapshots of 00471718 gets to fail.
(The situation can be reproduced with the below sequence:
$ dog vdi create 00471718 1G
$ dog vdi snapshot 00471718 (repeat 8 times)
$ dog vdi delete -s 8 00471718
$ dog vdi delete -s 2 00471718 )
Thanks,
Yoshifumi Fukumoto
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 <[email protected]>
---
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;
--
sheepdog mailing list
[email protected]
https://lists.wpkg.org/mailman/listinfo/sheepdog
