On Wed, Jan 12, 2022 at 12:55 PM Peter Lieven <p...@kamp.de> wrote: > > Am 12.01.22 um 10:59 schrieb Ilya Dryomov: > > On Mon, Jan 10, 2022 at 12:43 PM Peter Lieven <p...@kamp.de> wrote: > >> librbd had a bug until early 2022 that affected all versions of ceph that > >> supported fast-diff. This bug results in reporting of incorrect offsets > >> if the offset parameter to rbd_diff_iterate2 is not object aligned. > >> Work around this bug by rounding down the offset to object boundaries. > >> > >> Fixes: https://tracker.ceph.com/issues/53784 > > I don't think the Fixes tag is appropriate here. Linking librbd > > ticket is fine but this patch doesn't really fix anything. > > > Okay, I will change that to See:
It's already linked in the source code, up to you if you also want to link it in the description. > > > > > >> Cc: qemu-sta...@nongnu.org > >> Signed-off-by: Peter Lieven <p...@kamp.de> > >> --- > >> block/rbd.c | 17 ++++++++++++++++- > >> 1 file changed, 16 insertions(+), 1 deletion(-) > >> > >> diff --git a/block/rbd.c b/block/rbd.c > >> index 5e9dc91d81..260cb9f4b4 100644 > >> --- a/block/rbd.c > >> +++ b/block/rbd.c > >> @@ -1333,6 +1333,7 @@ static int coroutine_fn > >> qemu_rbd_co_block_status(BlockDriverState *bs, > >> int status, r; > >> RBDDiffIterateReq req = { .offs = offset }; > >> uint64_t features, flags; > >> + int64_t head; > >> > >> assert(offset + bytes <= s->image_size); > >> > >> @@ -1360,6 +1361,19 @@ static int coroutine_fn > >> qemu_rbd_co_block_status(BlockDriverState *bs, > >> return status; > >> } > >> > >> + /* > >> + * librbd had a bug until early 2022 that affected all versions of > >> ceph that > >> + * supported fast-diff. This bug results in reporting of incorrect > >> offsets > >> + * if the offset parameter to rbd_diff_iterate2 is not object aligned. > >> + * Work around this bug by rounding down the offset to object > >> boundaries. > >> + * > >> + * See: https://tracker.ceph.com/issues/53784 > >> + */ > >> + head = offset & (s->object_size - 1); > >> + offset -= head; > >> + req.offs -= head; > >> + bytes += head; > > So it looks like the intention is to have more or less a permanent > > workaround since all librbd versions are affected, right? For that, > > I think we would need to also reject custom striping patterns and > > clones. For the above to be reliable, the image has to be standalone > > and have a default striping pattern (stripe_unit == object_size && > > stripe_count == 1). Otherwise, behave as if fast-diff is disabled or > > invalid. > > > Do you have a fealing how many users use a different striping pattern than > default? Very few. > > What about EC backed pools? In this context EC pools behave exactly the same as replicated pools. > > Do you have another idea how we can detect if the librbd version is broken? No. Initially I wanted to just fix these bugs in librbd, relying on the assumption that setups with a new QEMU should also have a fairly new librbd. But after looking at various distros and realizing the extent of rbd_diff_iterate2() issues, I think a long-term workaround in QEMU makes sense. A configure-time check for known good versions of librbd can be added later if someone feels like it. Thanks, Ilya