* [PATCH for-4.2 0/2] Fix bitmap migration
@ 2019-11-25 12:52 Vladimir Sementsov-Ogievskiy
2019-11-25 12:52 ` [PATCH 1/2] block/qcow2-bitmap: fix " Vladimir Sementsov-Ogievskiy
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-11-25 12:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-stable, qemu-devel, mreitz, jsnow
Hi all!
We've faced a bug in rhev-2.12.0-33.el7-based Qemu.
In upstream, bug introduced in 4.0 by 74da6b943565c45
"block/dirty-bitmaps: implement inconsistent bit" commit.
At this commit we started to load inconsistent bitmap instead of
silently ignoring them, and it now I see that it breaks migration.
The fix is very simple, so I think it's OK for 4.2.. Still, it's not a
degradation, so we may postpone it to 5.0.
Vladimir Sementsov-Ogievskiy (2):
block/qcow2-bitmap: fix bitmap migration
iotests: add new test cases to bitmap migration
block/qcow2-bitmap.c | 21 ++++++++++++++++++++-
tests/qemu-iotests/169 | 22 +++++++++++++++-------
tests/qemu-iotests/169.out | 4 ++--
3 files changed, 37 insertions(+), 10 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] block/qcow2-bitmap: fix bitmap migration
2019-11-25 12:52 [PATCH for-4.2 0/2] Fix bitmap migration Vladimir Sementsov-Ogievskiy
@ 2019-11-25 12:52 ` Vladimir Sementsov-Ogievskiy
2019-11-25 12:52 ` [PATCH 2/2] iotests: add new test cases to " Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-11-25 12:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-stable, qemu-devel, mreitz, jsnow
Fix bitmap migration with dirty-bitmaps capability enabled and shared
storage. We should ignore IN_USE bitmaps in the image on target, when
migrating bitmaps through migration channel, see new comment below.
Fixes: 74da6b943565c451
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2-bitmap.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 809bbc5d20..8abaf632fc 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -988,7 +988,26 @@ bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp)
}
QSIMPLEQ_FOREACH(bm, bm_list, entry) {
- BdrvDirtyBitmap *bitmap = load_bitmap(bs, bm, errp);
+ BdrvDirtyBitmap *bitmap;
+
+ if ((bm->flags & BME_FLAG_IN_USE) &&
+ bdrv_find_dirty_bitmap(bs, bm->name))
+ {
+ /*
+ * We already have corresponding BdrvDirtyBitmap, and bitmap in the
+ * image is marked IN_USE. Firstly, this state is valid, no reason
+ * to consider existing BdrvDirtyBitmap to be bad. Secondly it's
+ * absolutely possible, when we do migration with shared storage
+ * with dirty-bitmaps capability enabled: if the bitmap was loaded
+ * from this storage before migration start, the storage will
+ * of-course contain IN_USE outdated version of the bitmap, and we
+ * should not load it on migration target, as we already have this
+ * bitmap, being migrated.
+ */
+ continue;
+ }
+
+ bitmap = load_bitmap(bs, bm, errp);
if (bitmap == NULL) {
goto fail;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] iotests: add new test cases to bitmap migration
2019-11-25 12:52 [PATCH for-4.2 0/2] Fix bitmap migration Vladimir Sementsov-Ogievskiy
2019-11-25 12:52 ` [PATCH 1/2] block/qcow2-bitmap: fix " Vladimir Sementsov-Ogievskiy
@ 2019-11-25 12:52 ` Vladimir Sementsov-Ogievskiy
2019-11-25 13:50 ` [PATCH for-4.2 0/2] Fix " Max Reitz
2019-11-26 10:31 ` Max Reitz
3 siblings, 0 replies; 6+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-11-25 12:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-stable, qemu-devel, mreitz, jsnow
Add optional pre-shutdown: shutdown/launch vm before migration. This
leads to storing persistent bitmap to the storage, which breaks
migration with dirty-bitmaps capability enabled and shared storage
until fixed by previous commit.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/169 | 22 +++++++++++++++-------
tests/qemu-iotests/169.out | 4 ++--
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
index 8c204caf20..9656a7f620 100755
--- a/tests/qemu-iotests/169
+++ b/tests/qemu-iotests/169
@@ -134,7 +134,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
self.check_bitmap(self.vm_a, sha256 if persistent else False)
def do_test_migration(self, persistent, migrate_bitmaps, online,
- shared_storage):
+ shared_storage, pre_shutdown):
granularity = 512
# regions = ((start, count), ...)
@@ -142,15 +142,13 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
(0xf0000, 0x10000),
(0xa0201, 0x1000))
- should_migrate = migrate_bitmaps or persistent and shared_storage
+ should_migrate = \
+ (migrate_bitmaps and (persistent or not pre_shutdown)) or \
+ (persistent and shared_storage)
mig_caps = [{'capability': 'events', 'state': True}]
if migrate_bitmaps:
mig_caps.append({'capability': 'dirty-bitmaps', 'state': True})
- result = self.vm_a.qmp('migrate-set-capabilities',
- capabilities=mig_caps)
- self.assert_qmp(result, 'return', {})
-
self.vm_b.add_incoming(incoming_cmd if online else "defer")
self.vm_b.add_drive(disk_a if shared_storage else disk_b)
@@ -166,6 +164,14 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % r)
sha256 = self.get_bitmap_hash(self.vm_a)
+ if pre_shutdown:
+ self.vm_a.shutdown()
+ self.vm_a.launch()
+
+ result = self.vm_a.qmp('migrate-set-capabilities',
+ capabilities=mig_caps)
+ self.assert_qmp(result, 'return', {})
+
result = self.vm_a.qmp('migrate', uri=mig_cmd)
while True:
event = self.vm_a.event_wait('MIGRATION')
@@ -210,11 +216,13 @@ def inject_test_case(klass, name, method, *args, **kwargs):
mc = operator.methodcaller(method, *args, **kwargs)
setattr(klass, 'test_' + method + name, lambda self: mc(self))
-for cmb in list(itertools.product((True, False), repeat=4)):
+for cmb in list(itertools.product((True, False), repeat=5)):
name = ('_' if cmb[0] else '_not_') + 'persistent_'
name += ('_' if cmb[1] else '_not_') + 'migbitmap_'
name += '_online' if cmb[2] else '_offline'
name += '_shared' if cmb[3] else '_nonshared'
+ if (cmb[4]):
+ name += '__pre_shutdown'
inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration',
*list(cmb))
diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/169.out
index 3a89159833..5c26d15c0d 100644
--- a/tests/qemu-iotests/169.out
+++ b/tests/qemu-iotests/169.out
@@ -1,5 +1,5 @@
-....................
+....................................
----------------------------------------------------------------------
-Ran 20 tests
+Ran 36 tests
OK
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.2 0/2] Fix bitmap migration
2019-11-25 12:52 [PATCH for-4.2 0/2] Fix bitmap migration Vladimir Sementsov-Ogievskiy
2019-11-25 12:52 ` [PATCH 1/2] block/qcow2-bitmap: fix " Vladimir Sementsov-Ogievskiy
2019-11-25 12:52 ` [PATCH 2/2] iotests: add new test cases to " Vladimir Sementsov-Ogievskiy
@ 2019-11-25 13:50 ` Max Reitz
2019-11-25 14:04 ` Vladimir Sementsov-Ogievskiy
2019-11-26 10:31 ` Max Reitz
3 siblings, 1 reply; 6+ messages in thread
From: Max Reitz @ 2019-11-25 13:50 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, jsnow, qemu-devel, qemu-stable
[-- Attachment #1.1: Type: text/plain, Size: 1096 bytes --]
On 25.11.19 13:52, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> We've faced a bug in rhev-2.12.0-33.el7-based Qemu.
> In upstream, bug introduced in 4.0 by 74da6b943565c45
> "block/dirty-bitmaps: implement inconsistent bit" commit.
> At this commit we started to load inconsistent bitmap instead of
> silently ignoring them, and it now I see that it breaks migration.
>
> The fix is very simple, so I think it's OK for 4.2.. Still, it's not a
> degradation, so we may postpone it to 5.0.
>
> Vladimir Sementsov-Ogievskiy (2):
> block/qcow2-bitmap: fix bitmap migration
> iotests: add new test cases to bitmap migration
>
> block/qcow2-bitmap.c | 21 ++++++++++++++++++++-
> tests/qemu-iotests/169 | 22 +++++++++++++++-------
> tests/qemu-iotests/169.out | 4 ++--
> 3 files changed, 37 insertions(+), 10 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
Makes sense to me to put this into 4.2, but I don’t think it would
survive Peter’s check list. :?
(https://lists.nongnu.org/archive/html/qemu-block/2019-11/msg00807.html)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.2 0/2] Fix bitmap migration
2019-11-25 13:50 ` [PATCH for-4.2 0/2] Fix " Max Reitz
@ 2019-11-25 14:04 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 6+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-11-25 14:04 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: kwolf, jsnow, qemu-devel, qemu-stable
25.11.2019 16:50, Max Reitz wrote:
> On 25.11.19 13:52, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> We've faced a bug in rhev-2.12.0-33.el7-based Qemu.
>> In upstream, bug introduced in 4.0 by 74da6b943565c45
>> "block/dirty-bitmaps: implement inconsistent bit" commit.
>> At this commit we started to load inconsistent bitmap instead of
>> silently ignoring them, and it now I see that it breaks migration.
>>
>> The fix is very simple, so I think it's OK for 4.2.. Still, it's not a
>> degradation, so we may postpone it to 5.0.
>>
>> Vladimir Sementsov-Ogievskiy (2):
>> block/qcow2-bitmap: fix bitmap migration
>> iotests: add new test cases to bitmap migration
>>
>> block/qcow2-bitmap.c | 21 ++++++++++++++++++++-
>> tests/qemu-iotests/169 | 22 +++++++++++++++-------
>> tests/qemu-iotests/169.out | 4 ++--
>> 3 files changed, 37 insertions(+), 10 deletions(-)
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
>
> Makes sense to me to put this into 4.2, but I don’t think it would
> survive Peter’s check list. :?
It depends on how much the fixed case is popular..
Actually, anyone who tries to migrate bitmaps with dirty-bitmaps capability
enabled and with shared storage will very possibly run into this bug.
But who is it, except for Virtuozzo, I don't know:) Still, I see that a lot
of that staff merged into Rhel Qemu we based on, so at least this should go
to qemu-stable and to Rhel update.
>
> (https://lists.nongnu.org/archive/html/qemu-block/2019-11/msg00807.html)
>
> Max
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.2 0/2] Fix bitmap migration
2019-11-25 12:52 [PATCH for-4.2 0/2] Fix bitmap migration Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2019-11-25 13:50 ` [PATCH for-4.2 0/2] Fix " Max Reitz
@ 2019-11-26 10:31 ` Max Reitz
3 siblings, 0 replies; 6+ messages in thread
From: Max Reitz @ 2019-11-26 10:31 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, jsnow, qemu-devel, qemu-stable
[-- Attachment #1.1: Type: text/plain, Size: 972 bytes --]
On 25.11.19 13:52, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> We've faced a bug in rhev-2.12.0-33.el7-based Qemu.
> In upstream, bug introduced in 4.0 by 74da6b943565c45
> "block/dirty-bitmaps: implement inconsistent bit" commit.
> At this commit we started to load inconsistent bitmap instead of
> silently ignoring them, and it now I see that it breaks migration.
>
> The fix is very simple, so I think it's OK for 4.2.. Still, it's not a
> degradation, so we may postpone it to 5.0.
>
> Vladimir Sementsov-Ogievskiy (2):
> block/qcow2-bitmap: fix bitmap migration
> iotests: add new test cases to bitmap migration
>
> block/qcow2-bitmap.c | 21 ++++++++++++++++++++-
> tests/qemu-iotests/169 | 22 +++++++++++++++-------
> tests/qemu-iotests/169.out | 4 ++--
> 3 files changed, 37 insertions(+), 10 deletions(-)
Thanks, applied to my block branch:
https://git.xanclic.moe/XanClic/qemu/commits/branch/block
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-26 10:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-25 12:52 [PATCH for-4.2 0/2] Fix bitmap migration Vladimir Sementsov-Ogievskiy
2019-11-25 12:52 ` [PATCH 1/2] block/qcow2-bitmap: fix " Vladimir Sementsov-Ogievskiy
2019-11-25 12:52 ` [PATCH 2/2] iotests: add new test cases to " Vladimir Sementsov-Ogievskiy
2019-11-25 13:50 ` [PATCH for-4.2 0/2] Fix " Max Reitz
2019-11-25 14:04 ` Vladimir Sementsov-Ogievskiy
2019-11-26 10:31 ` Max Reitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).