[Qemu-devel] [PULL 0/2] Block patches for 2.12.0-rc4

[Qemu-devel] [PULL 0/2] Block patches for 2.12.0-rc4

[Max Reitz]

The following changes since commit ae2b1b4e1bb89ea949446597c8776255da0a79d3:

  Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-2.12-pull-request' into staging (2018-04-16 10:11:17 +0100)

are available in the Git repository at:

  git://github.com/XanClic/qemu.git tags/pull-block-2018-04-16

for you to fetch changes up to 25bf2426f3b27857afa35194227040eab821a047:

  iotests: fix 169 (2018-04-16 13:35:32 +0200)

----------------------------------------------------------------
A fix for handling dirty bitmaps stored in qcow2 files.  This is not
absolutely necessary for 2.12, but if there is an rc4, it should go in.

----------------------------------------------------------------
Vladimir Sementsov-Ogievskiy (2):
  qcow2: try load bitmaps only once
  iotests: fix 169

 block/qcow2.h          |  1 +
 block/qcow2.c          | 16 ++++++++--------
 tests/qemu-iotests/169 | 48 +++++++++++++++++++++++++++---------------------
 3 files changed, 36 insertions(+), 29 deletions(-)

-- 
2.14.3

[Qemu-devel] [PULL 1/2] qcow2: try load bitmaps only once

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Checking reopen by existence of some bitmaps is wrong, as it may be
some other bitmaps, or on the other hand, user may remove bitmaps. This
criteria is bad. To simplify things and make behavior more predictable
let's just add a flag to remember, that we've already tried to load
bitmaps on open and do not want do it again.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20180411122606.367301-2-vsementsov@virtuozzo.com
[mreitz: Changed comment wording according to Eric Blake's suggestion]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.h |  1 +
 block/qcow2.c | 16 ++++++++--------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index d301f77cea..adf5c3950f 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -298,6 +298,7 @@ typedef struct BDRVQcow2State {
     uint32_t nb_bitmaps;
     uint64_t bitmap_directory_size;
     uint64_t bitmap_directory_offset;
+    bool dirty_bitmaps_loaded;
 
     int flags;
     int qcow_version;
diff --git a/block/qcow2.c b/block/qcow2.c
index 486f3e83b7..ef68772aca 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1142,6 +1142,7 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
     uint64_t ext_end;
     uint64_t l1_vm_state_index;
     bool update_header = false;
+    bool header_updated = false;
 
     ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
     if (ret < 0) {
@@ -1480,10 +1481,9 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
         s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
     }
 
-    if (bdrv_dirty_bitmap_next(bs, NULL)) {
-        /* It's some kind of reopen with already existing dirty bitmaps. There
-         * are no known cases where we need loading bitmaps in such situation,
-         * so it's safer don't load them.
+    if (s->dirty_bitmaps_loaded) {
+        /* It's some kind of reopen. There are no known cases where we need to
+         * reload bitmaps in such a situation, so it's safer to skip them.
          *
          * Moreover, if we have some readonly bitmaps and we are reopening for
          * rw we should reopen bitmaps correspondingly.
@@ -1491,13 +1491,13 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
         if (bdrv_has_readonly_bitmaps(bs) &&
             !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE))
         {
-            bool header_updated = false;
             qcow2_reopen_bitmaps_rw_hint(bs, &header_updated, &local_err);
-            update_header = update_header && !header_updated;
         }
-    } else if (qcow2_load_dirty_bitmaps(bs, &local_err)) {
-        update_header = false;
+    } else {
+        header_updated = qcow2_load_dirty_bitmaps(bs, &local_err);
+        s->dirty_bitmaps_loaded = true;
     }
+    update_header = update_header && !header_updated;
     if (local_err != NULL) {
         error_propagate(errp, local_err);
         ret = -EINVAL;
-- 
2.14.3

[Qemu-devel] [PULL 2/2] iotests: fix 169

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Improve and fix 169:
    - use MIGRATION events instead of RESUME
    - make a TODO: enable dirty-bitmaps capability for offline case
    - recreate vm_b without -incoming near test end

This (likely) fixes racy faults at least of the following types:

    - timeout on waiting for RESUME event
    - sha256 mismatch on line 136 (142 after this patch)
    - fail to self.vm_b.launch() on line 135 (141 now after this patch)

And surely fixes cat processes, left after test finish.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20180411122606.367301-3-vsementsov@virtuozzo.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/169 | 48 +++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
index 153b10b6e7..f243db9955 100755
--- a/tests/qemu-iotests/169
+++ b/tests/qemu-iotests/169
@@ -31,6 +31,8 @@ disk_a = os.path.join(iotests.test_dir, 'disk_a')
 disk_b = os.path.join(iotests.test_dir, 'disk_b')
 size = '1M'
 mig_file = os.path.join(iotests.test_dir, 'mig_file')
+mig_cmd = 'exec: cat > ' + mig_file
+incoming_cmd = 'exec: cat ' + mig_file
 
 
 class TestDirtyBitmapMigration(iotests.QMPTestCase):
@@ -49,7 +51,6 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
         self.vm_a.launch()
 
         self.vm_b = iotests.VM(path_suffix='b')
-        self.vm_b.add_incoming("exec: cat '" + mig_file + "'")
 
     def add_bitmap(self, vm, granularity, persistent):
         params = {'node': 'drive0',
@@ -86,36 +87,30 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
                    (0xa0201, 0x1000))
 
         should_migrate = migrate_bitmaps 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)
 
         if online:
             os.mkfifo(mig_file)
             self.vm_b.launch()
+            result = self.vm_b.qmp('migrate-set-capabilities',
+                                   capabilities=mig_caps)
+            self.assert_qmp(result, 'return', {})
 
         self.add_bitmap(self.vm_a, granularity, persistent)
         for r in regions:
             self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % r)
         sha256 = self.get_bitmap_hash(self.vm_a)
 
-        if migrate_bitmaps:
-            capabilities = [{'capability': 'dirty-bitmaps', 'state': True}]
-
-            result = self.vm_a.qmp('migrate-set-capabilities',
-                                   capabilities=capabilities)
-            self.assert_qmp(result, 'return', {})
-
-            if online:
-                result = self.vm_b.qmp('migrate-set-capabilities',
-                                       capabilities=capabilities)
-                self.assert_qmp(result, 'return', {})
-
-        result = self.vm_a.qmp('migrate-set-capabilities',
-                               capabilities=[{'capability': 'events',
-                                              'state': True}])
-        self.assert_qmp(result, 'return', {})
-
-        result = self.vm_a.qmp('migrate', uri='exec:cat>' + mig_file)
+        result = self.vm_a.qmp('migrate', uri=mig_cmd)
         while True:
             event = self.vm_a.event_wait('MIGRATION')
             if event['data']['status'] == 'completed':
@@ -124,14 +119,25 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
         if not online:
             self.vm_a.shutdown()
             self.vm_b.launch()
-            # TODO enable bitmap capability for vm_b in this case
+            result = self.vm_b.qmp('migrate-set-capabilities',
+                                   capabilities=mig_caps)
+            self.assert_qmp(result, 'return', {})
+            result = self.vm_b.qmp('migrate-incoming', uri=incoming_cmd)
+            self.assert_qmp(result, 'return', {})
 
-        self.vm_b.event_wait("RESUME", timeout=10.0)
+        while True:
+            event = self.vm_b.event_wait('MIGRATION')
+            if event['data']['status'] == 'completed':
+                break
 
         self.check_bitmap(self.vm_b, sha256 if should_migrate else False)
 
         if should_migrate:
             self.vm_b.shutdown()
+            # recreate vm_b, as we don't want -incoming option (this will lead
+            # to "cat" process left alive after test finish)
+            self.vm_b = iotests.VM(path_suffix='b')
+            self.vm_b.add_drive(disk_a if shared_storage else disk_b)
             self.vm_b.launch()
             self.check_bitmap(self.vm_b, sha256 if persistent else False)
 
-- 
2.14.3

Re: [Qemu-devel] [PULL 0/2] Block patches for 2.12.0-rc4

[Peter Maydell]

On 16 April 2018 at 13:38, Max Reitz <mreitz@redhat.com> wrote:
> The following changes since commit ae2b1b4e1bb89ea949446597c8776255da0a79d3:
>
>   Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-2.12-pull-request' into staging (2018-04-16 10:11:17 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/XanClic/qemu.git tags/pull-block-2018-04-16
>
> for you to fetch changes up to 25bf2426f3b27857afa35194227040eab821a047:
>
>   iotests: fix 169 (2018-04-16 13:35:32 +0200)
>
> ----------------------------------------------------------------
> A fix for handling dirty bitmaps stored in qcow2 files.  This is not
> absolutely necessary for 2.12, but if there is an rc4, it should go in.
>
> ----------------------------------------------------------------
> Vladimir Sementsov-Ogievskiy (2):
>   qcow2: try load bitmaps only once
>   iotests: fix 169
>
>  block/qcow2.h          |  1 +
>  block/qcow2.c          | 16 ++++++++--------
>  tests/qemu-iotests/169 | 48 +++++++++++++++++++++++++++---------------------
>  3 files changed, 36 insertions(+), 29 deletions(-)

Applied, thanks.

-- PMM