Merge ~paelzer/ubuntu/+source/qemu:lp-1246924-enable-glusterfs into ubuntu/+source/qemu:ubuntu/jammy-devel

Proposed by Christian Ehrhardt 
Status: Merged
Merge reported by: Christian Ehrhardt 
Merged at revision: 684fd902d2456fa3d683ba8a72bb7f35fccce6ca
Proposed branch: ~paelzer/ubuntu/+source/qemu:lp-1246924-enable-glusterfs
Merge into: ubuntu/+source/qemu:ubuntu/jammy-devel
Diff against target: 223 lines (+167/-4)
6 files modified
debian/changelog (+9/-0)
debian/control (+2/-1)
debian/control-in (+2/-3)
debian/patches/series (+2/-0)
debian/patches/ubuntu/lp-1968258-block-rbd-fix-handling-of-holes-in-.bdrv_co.patch (+54/-0)
debian/patches/ubuntu/lp-1968258-block-rbd-workaround-for-ceph-issue-53784.patch (+98/-0)
Reviewer Review Type Date Requested Status
Utkarsh Gupta (community) Needs Information
Canonical Server packageset reviewers Pending
git-ubuntu import Pending
Review via email: mp+418926@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI: I also added bug 1968258 as data corruption is kind of bad and we'd SRU it anyway we can as well fix it before release as part of the upload we need for glusterfs anyway.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI - glusterfs change merged in Debian (thanks MJT)

Revision history for this message
Utkarsh Gupta (utkarsh) wrote :

Okay, looks good, I think! +1.

review: Approve
Revision history for this message
Utkarsh Gupta (utkarsh) wrote :

Hey, hold on a sec. GlusterFS is not built on i386, so should it not be [!i386] instead of [linux-any], instead?

review: Needs Information
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Clarified via IRC, due to not being built there
https://people.canonical.com/~ubuntu-archive/germinate-output/i386.jammy/i386+build-depends
this way it allows us to have less delta.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Uploaded to get the MIR to the next stage:

Uploading qemu_6.2+dfsg-2ubuntu6.dsc
Uploading qemu_6.2+dfsg-2ubuntu6.debian.tar.xz
Uploading qemu_6.2+dfsg-2ubuntu6_source.buildinfo
Uploading qemu_6.2+dfsg-2ubuntu6_source.changes

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index c78e675..3ec2625 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,12 @@
6+qemu (1:6.2+dfsg-2ubuntu6) jammy; urgency=medium
7+
8+ * debian/control[-in]: no more disable glusterfs in Ubuntu (LP: #1246924)
9+ * Fix diff handling on ceph that can cause data corruption (LP: #1968258)
10+ - d/p/u/lp-1968258-block-rbd-fix-handling-of-holes-in-.bdrv_co.patch
11+ - d/p/u/lp-1968258-block-rbd-workaround-for-ceph-issue-53784.patch
12+
13+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Fri, 08 Apr 2022 09:36:34 +0200
14+
15 qemu (1:6.2+dfsg-2ubuntu5) jammy; urgency=medium
16
17 * d/p/u/tcg-Remove-dh_alias-indirection-for-dh_typecode.patch: fix 32bit
18diff --git a/debian/control b/debian/control
19index f485609..00627d8 100644
20--- a/debian/control
21+++ b/debian/control
22@@ -59,8 +59,9 @@ Build-Depends: debhelper-compat (= 12),
23 libpixman-1-dev,
24 # --enable-rbd amd64|arm64|armel|armhf|i386|mips64el|mipsel|ppc64el|s390x|ppc64|sparc64
25 librbd-dev [amd64 arm64 armel armhf i386 mips64el mipsel ppc64el s390x ppc64 sparc64],
26-# glusterfs is debian-only since ubuntu/glusterfs is in universe (MIR LP: #1274247)
27 # before buster it was glusterfs-common so keep it for now for bpo
28+# --enable-glusterfs linux-any
29+ libglusterfs-dev [linux-any] | glusterfs-common [linux-any],
30 # --enable-vnc-sasl
31 libsasl2-dev,
32 # --enable-sdl
33diff --git a/debian/control-in b/debian/control-in
34index 80e2b77..6754533 100644
35--- a/debian/control-in
36+++ b/debian/control-in
37@@ -63,10 +63,9 @@ Build-Depends: debhelper-compat (= 12),
38 libpixman-1-dev,
39 # --enable-rbd amd64|arm64|armel|armhf|i386|mips64el|mipsel|ppc64el|s390x|ppc64|sparc64
40 librbd-dev [amd64 arm64 armel armhf i386 mips64el mipsel ppc64el s390x ppc64 sparc64],
41-# glusterfs is debian-only since ubuntu/glusterfs is in universe (MIR LP: #1274247)
42 # before buster it was glusterfs-common so keep it for now for bpo
43-:debian:# --enable-glusterfs linux-any
44-:debian: libglusterfs-dev [linux-any] | glusterfs-common [linux-any],
45+# --enable-glusterfs linux-any
46+ libglusterfs-dev [linux-any] | glusterfs-common [linux-any],
47 # --enable-vnc-sasl
48 libsasl2-dev,
49 # --enable-sdl
50diff --git a/debian/patches/series b/debian/patches/series
51index d136bfb..ab3291d 100644
52--- a/debian/patches/series
53+++ b/debian/patches/series
54@@ -27,3 +27,5 @@ ubuntu/lp-1952448-relax-skiboot-gcc-deprecation-errors.patch
55 ubuntu/lp-1959984-s390x-ipl-support-extended-kernel-command-line-size.patch
56 ubuntu/fix-virtiofsd-for-glibc2.35.patch
57 ubuntu/tcg-Remove-dh_alias-indirection-for-dh_typecode.patch
58+ubuntu/lp-1968258-block-rbd-fix-handling-of-holes-in-.bdrv_co.patch
59+ubuntu/lp-1968258-block-rbd-workaround-for-ceph-issue-53784.patch
60diff --git a/debian/patches/ubuntu/lp-1968258-block-rbd-fix-handling-of-holes-in-.bdrv_co.patch b/debian/patches/ubuntu/lp-1968258-block-rbd-fix-handling-of-holes-in-.bdrv_co.patch
61new file mode 100644
62index 0000000..26d0f60
63--- /dev/null
64+++ b/debian/patches/ubuntu/lp-1968258-block-rbd-fix-handling-of-holes-in-.bdrv_co.patch
65@@ -0,0 +1,54 @@
66+From 9e302f64bb407a9bb097b626da97228c2654cfee Mon Sep 17 00:00:00 2001
67+From: Peter Lieven <pl@kamp.de>
68+Date: Thu, 13 Jan 2022 15:44:25 +0100
69+Subject: [PATCH] block/rbd: fix handling of holes in .bdrv_co_block_status
70+
71+the assumption that we can't hit a hole if we do not diff against a snapshot was wrong.
72+
73+We can see a hole in an image if we diff against base if there exists an older snapshot
74+of the image and we have discarded blocks in the image where the snapshot has data.
75+
76+Fix this by simply handling a hole like an unallocated area. There are no callbacks
77+for unallocated areas so just bail out if we hit a hole.
78+
79+Fixes: 0347a8fd4c3faaedf119be04c197804be40a384b
80+Suggested-by: Ilya Dryomov <idryomov@gmail.com>
81+Cc: qemu-stable@nongnu.org
82+Signed-off-by: Peter Lieven <pl@kamp.de>
83+Message-Id: <20220113144426.4036493-2-pl@kamp.de>
84+Reviewed-by: Ilya Dryomov <idryomov@gmail.com>
85+Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
86+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
87+
88+Origin: upstream, https://git.qemu.org/?p=qemu.git;a=commit;h=9e302f64bb407a9bb097b626da97
89+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1968258
90+Last-Update: 2022-04-08
91+
92+---
93+ block/rbd.c | 10 +++++-----
94+ 1 file changed, 5 insertions(+), 5 deletions(-)
95+
96+diff --git a/block/rbd.c b/block/rbd.c
97+index def96292e0..20bb896c4a 100644
98+--- a/block/rbd.c
99++++ b/block/rbd.c
100+@@ -1279,11 +1279,11 @@ static int qemu_rbd_diff_iterate_cb(uint64_t offs, size_t len,
101+ RBDDiffIterateReq *req = opaque;
102+
103+ assert(req->offs + req->bytes <= offs);
104+- /*
105+- * we do not diff against a snapshot so we should never receive a callback
106+- * for a hole.
107+- */
108+- assert(exists);
109++
110++ /* treat a hole like an unallocated area and bail out */
111++ if (!exists) {
112++ return 0;
113++ }
114+
115+ if (!req->exists && offs > req->offs) {
116+ /*
117+--
118+2.35.1
119+
120diff --git a/debian/patches/ubuntu/lp-1968258-block-rbd-workaround-for-ceph-issue-53784.patch b/debian/patches/ubuntu/lp-1968258-block-rbd-workaround-for-ceph-issue-53784.patch
121new file mode 100644
122index 0000000..5ee662e
123--- /dev/null
124+++ b/debian/patches/ubuntu/lp-1968258-block-rbd-workaround-for-ceph-issue-53784.patch
125@@ -0,0 +1,98 @@
126+From fc176116cdea816ceb8dd969080b2b95f58edbc0 Mon Sep 17 00:00:00 2001
127+From: Peter Lieven <pl@kamp.de>
128+Date: Thu, 13 Jan 2022 15:44:26 +0100
129+Subject: [PATCH] block/rbd: workaround for ceph issue #53784
130+
131+librbd had a bug until early 2022 that affected all versions of ceph that
132+supported fast-diff. This bug results in reporting of incorrect offsets
133+if the offset parameter to rbd_diff_iterate2 is not object aligned.
134+
135+This patch works around this bug for pre Quincy versions of librbd.
136+
137+Fixes: 0347a8fd4c3faaedf119be04c197804be40a384b
138+Cc: qemu-stable@nongnu.org
139+Signed-off-by: Peter Lieven <pl@kamp.de>
140+Message-Id: <20220113144426.4036493-3-pl@kamp.de>
141+Reviewed-by: Ilya Dryomov <idryomov@gmail.com>
142+Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
143+Tested-by: Stefano Garzarella <sgarzare@redhat.com>
144+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
145+
146+Origin: upstream, https://git.qemu.org/?p=qemu.git;a=commit;h=fc176116cdea816ceb8dd969080b
147+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1968258
148+Last-Update: 2022-04-08
149+
150+---
151+ block/rbd.c | 42 ++++++++++++++++++++++++++++++++++++++++--
152+ 1 file changed, 40 insertions(+), 2 deletions(-)
153+
154+diff --git a/block/rbd.c b/block/rbd.c
155+index 20bb896c4a..8f183eba2a 100644
156+--- a/block/rbd.c
157++++ b/block/rbd.c
158+@@ -1320,6 +1320,7 @@ static int coroutine_fn qemu_rbd_co_block_status(BlockDriverState *bs,
159+ int status, r;
160+ RBDDiffIterateReq req = { .offs = offset };
161+ uint64_t features, flags;
162++ uint64_t head = 0;
163+
164+ assert(offset + bytes <= s->image_size);
165+
166+@@ -1347,7 +1348,43 @@ static int coroutine_fn qemu_rbd_co_block_status(BlockDriverState *bs,
167+ return status;
168+ }
169+
170+- r = rbd_diff_iterate2(s->image, NULL, offset, bytes, true, true,
171++#if LIBRBD_VERSION_CODE < LIBRBD_VERSION(1, 17, 0)
172++ /*
173++ * librbd had a bug until early 2022 that affected all versions of ceph that
174++ * supported fast-diff. This bug results in reporting of incorrect offsets
175++ * if the offset parameter to rbd_diff_iterate2 is not object aligned.
176++ * Work around this bug by rounding down the offset to object boundaries.
177++ * This is OK because we call rbd_diff_iterate2 with whole_object = true.
178++ * However, this workaround only works for non cloned images with default
179++ * striping.
180++ *
181++ * See: https://tracker.ceph.com/issues/53784
182++ */
183++
184++ /* check if RBD image has non-default striping enabled */
185++ if (features & RBD_FEATURE_STRIPINGV2) {
186++ return status;
187++ }
188++
189++#pragma GCC diagnostic push
190++#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
191++ /*
192++ * check if RBD image is a clone (= has a parent).
193++ *
194++ * rbd_get_parent_info is deprecated from Nautilus onwards, but the
195++ * replacement rbd_get_parent is not present in Luminous and Mimic.
196++ */
197++ if (rbd_get_parent_info(s->image, NULL, 0, NULL, 0, NULL, 0) != -ENOENT) {
198++ return status;
199++ }
200++#pragma GCC diagnostic pop
201++
202++ head = req.offs & (s->object_size - 1);
203++ req.offs -= head;
204++ bytes += head;
205++#endif
206++
207++ r = rbd_diff_iterate2(s->image, NULL, req.offs, bytes, true, true,
208+ qemu_rbd_diff_iterate_cb, &req);
209+ if (r < 0 && r != QEMU_RBD_EXIT_DIFF_ITERATE2) {
210+ return status;
211+@@ -1366,7 +1403,8 @@ static int coroutine_fn qemu_rbd_co_block_status(BlockDriverState *bs,
212+ status = BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID;
213+ }
214+
215+- *pnum = req.bytes;
216++ assert(req.bytes > head);
217++ *pnum = req.bytes - head;
218+ return status;
219+ }
220+
221+--
222+2.35.1
223+

Subscribers

People subscribed via source and target branches