On 07/24/2014 03:42 AM, Ilya Dryomov wrote:
> If we are mapping a snapshot, we must read in the parent_overlap value
> of that snapshot instead of that of the base image. Not doing so may
> in particular result in us returning zeros instead of user data:
>
> # cat overlap-snap.sh
> #!/bin/bash
> rbd create --size 10 --image-format 2 foo
> FOO_DEV=$(rbd map foo)
> dd if=/dev/urandom of=/dev/rbd0 bs=1M &>/dev/null
> echo "Base image"
> dd if=$FOO_DEV bs=1 count=16 skip=$(((4 << 20) - 8)) 2>/dev/null | xxd
> rbd snap create foo@snap
> rbd snap protect foo@snap
> rbd clone foo@snap bar
> rbd snap create bar@snap
> BAR_DEV=$(rbd map bar@snap)
> echo "Snapshot"
> dd if=$BAR_DEV bs=1 count=16 skip=$(((4 << 20) - 8)) 2>/dev/null | xxd
> rbd resize --allow-shrink --size 4 bar
> echo "Snapshot after base image resize"
> dd if=$BAR_DEV bs=1 count=16 skip=$(((4 << 20) - 8)) 2>/dev/null | xxd
>
> # ./overlap-snap.sh
> Base image
> 0000000: e781 e33b d34b 2225 6034 2845 a2e3 36ed ...;.K"%`4(E..6.
> Snapshot
> 0000000: e781 e33b d34b 2225 6034 2845 a2e3 36ed ...;.K"%`4(E..6.
> Resizing image: 100% complete...done.
> Snapshot after base image resize
> 0000000: e781 e33b d34b 2225 0000 0000 0000 0000 ...;.K"%........
>
> Even though bar@snap was taken with the old bar parent_overlap (8M),
> reads from bar@snap beyond the new bar parent_overlap (4M) return
> zeroes. Fix it.
>
> Signed-off-by: Ilya Dryomov <ilya.dryomov@xxxxxxxxxxx>
> ---
> drivers/block/rbd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index c4606987e9d1..cbc89fa9a677 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -4020,7 +4020,7 @@ static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev)
> goto out_err;
> }
>
> - snapid = cpu_to_le64(CEPH_NOSNAP);
> + snapid = cpu_to_le64(rbd_dev->spec->snap_id);
Well that's just an outright bug. It's been there since the
original commit that added parent support:
86b00e0 rbd: get parent spec for version 2 images
Parent images *must* be snapshots, so this was never
right.
I bet that was hard to figure out...
Looks good.
Reviewed-by: Alex Elder <elder@xxxxxxxxxx>
> ret = rbd_obj_method_sync(rbd_dev, rbd_dev->header_name,
> "rbd", "get_parent",
> &snapid, sizeof (snapid),
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
