[Qemu-devel] [PATCH for 2.11 0/4] fix bitmaps migration through shared storage

[Qemu-devel] [PATCH for 2.11 0/4] fix bitmaps migration through shared storage

[Vladimir Sementsov-Ogievskiy]

Hi all.

This fix may be considered as a material for 2.11, or may be postponed
to 2.12, on your choice.

The core patch is 02.

Vladimir Sementsov-Ogievskiy (4):
  qcow2-bitmap: add qcow2_reopen_bitmaps_rw_hint()
  qcow2: handle reopening bitmaps on bdrv_invalidate_cache
  scripts/qemu.py: tiny enhancement for event_wiat
  iotests: add dirty bitmap migration test

 block/qcow2.h                 |  2 ++
 block/qcow2-bitmap.c          | 15 +++++++-
 block/qcow2.c                 |  8 ++++-
 scripts/qemu.py               | 10 ++++--
 tests/qemu-iotests/169        | 82 +++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/169.out    |  5 +++
 tests/qemu-iotests/group      |  1 +
 tests/qemu-iotests/iotests.py |  7 +++-
 8 files changed, 124 insertions(+), 6 deletions(-)
 create mode 100755 tests/qemu-iotests/169
 create mode 100644 tests/qemu-iotests/169.out

-- 
2.11.1

[Qemu-devel] [PATCH 1/4] qcow2-bitmap: add qcow2_reopen_bitmaps_rw_hint()

[Vladimir Sementsov-Ogievskiy]

Add version of qcow2_reopen_bitmaps_rw, which do the same work but
also return a hint about was header updated or not. This will be
used in the following fix for bitmaps reloading after migration.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2.h        |  2 ++
 block/qcow2-bitmap.c | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 6f0ff15dd0..40fa5b7cfe 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -667,6 +667,8 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
                                   void **refcount_table,
                                   int64_t *refcount_table_size);
 bool qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
+                                 Error **errp);
 int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp);
 void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp);
 int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp);
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index f45e46cfbd..6c80dcda69 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1002,7 +1002,8 @@ fail:
     return false;
 }
 
-int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
+int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
+                                 Error **errp)
 {
     BDRVQcow2State *s = bs->opaque;
     Qcow2BitmapList *bm_list;
@@ -1010,6 +1011,10 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
     GSList *ro_dirty_bitmaps = NULL;
     int ret = 0;
 
+    if (header_updated != NULL) {
+        *header_updated = false;
+    }
+
     if (s->nb_bitmaps == 0) {
         /* No bitmaps - nothing to do */
         return 0;
@@ -1053,6 +1058,9 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
             error_setg_errno(errp, -ret, "Can't update bitmap directory");
             goto out;
         }
+        if (header_updated != NULL) {
+            *header_updated = true;
+        }
         g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false);
     }
 
@@ -1063,6 +1071,11 @@ out:
     return ret;
 }
 
+int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
+{
+    return qcow2_reopen_bitmaps_rw_hint(bs, NULL, errp);
+}
+
 /* store_bitmap_data()
  * Store bitmap to image, filling bitmap table accordingly.
  */
-- 
2.11.1

[Qemu-devel] [PATCH 2/4] qcow2: handle reopening bitmaps on bdrv_invalidate_cache

[Vladimir Sementsov-Ogievskiy]

Consider migration with shared storage. Persistent bitmaps are stored
on bdrv_inactivate. Then, on destination
process_incoming_migration_bh() calls bdrv_invalidate_cache_all() which
leads to qcow2_load_autoloading_dirty_bitmaps() which fails if bitmaps
are already loaded on destination start. In this case we should call
qcow2_reopen_bitmaps_rw instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 1914a940e5..f5f205e1b7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1450,7 +1450,13 @@ static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
         s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
     }
 
-    if (qcow2_load_autoloading_dirty_bitmaps(bs, &local_err)) {
+    if (bdrv_has_readonly_bitmaps(bs)) {
+        if (!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_autoloading_dirty_bitmaps(bs, &local_err)) {
         update_header = false;
     }
     if (local_err != NULL) {
-- 
2.11.1

[Qemu-devel] [PATCH 3/4] scripts/qemu.py: tiny enhancement for event_wiat

[Vladimir Sementsov-Ogievskiy]

- add comment
- allow int timeout and disallow bool (which has special
  meaning for pull_event())
- remove unreachable 'return None'

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 scripts/qemu.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 9bfdf6d37d..8763335254 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -291,7 +291,13 @@ class QEMUMachine(object):
         branch processing on match's value None
            {"foo": {"bar": 1}} matches {"foo": None}
            {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
+
+        @raise QMPTimeoutError: If timeout period elapses.
+        @raise QMPConnectError: If some other error occurred.
         '''
+
+        assert isinstance(timeout, float) or isinstance(timeout, int)
+
         def event_match(event, match=None):
             if match is None:
                 return True
@@ -316,13 +322,11 @@ class QEMUMachine(object):
 
         # Poll for new events
         while True:
-            event = self._qmp.pull_event(wait=timeout)
+            event = self._qmp.pull_event(wait=float(timeout))
             if (event['event'] == name) and event_match(event, match):
                 return event
             self._events.append(event)
 
-        return None
-
     def get_log(self):
         '''
         After self.shutdown or failed qemu execution, this returns the output
-- 
2.11.1

[Qemu-devel] [PATCH 4/4] iotests: add dirty bitmap migration test

[Vladimir Sementsov-Ogievskiy]

The test creates two vms (vm_a, vm_b), create dirty bitmap in
the first one, do several writes to corresponding device and
then migrate vm_a to vm_b with dirty bitmaps.

For now, only migration through shared storage for persistent
bitmaps is available, so it is tested here. Only offline variant
is tested for now (a kind of suspend-resume), as it is needed
to test that this case is successfully fixed by recent patch.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/169        | 82 +++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/169.out    |  5 +++
 tests/qemu-iotests/group      |  1 +
 tests/qemu-iotests/iotests.py |  7 +++-
 4 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/169
 create mode 100644 tests/qemu-iotests/169.out

diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
new file mode 100755
index 0000000000..a0f213b274
--- /dev/null
+++ b/tests/qemu-iotests/169
@@ -0,0 +1,82 @@
+#!/usr/bin/env python
+#
+# Tests for dirty bitmaps migration.
+#
+# Copyright (c) 2016-2017 Parallels International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import os
+import iotests
+import time
+from iotests import qemu_img
+
+disk = os.path.join(iotests.test_dir, 'disk')
+migfile = os.path.join(iotests.test_dir, 'migfile')
+
+class TestPersistentDirtyBitmapSuspendResume(iotests.QMPTestCase):
+
+    def tearDown(self):
+        self.vm_a.shutdown()
+        self.vm_b.shutdown()
+        os.remove(disk)
+        os.remove(migfile)
+
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, disk, '1M')
+
+        self.vm_a = iotests.VM(path_suffix='a').add_drive(disk)
+        self.vm_a.launch()
+
+        self.vm_b = iotests.VM(path_suffix='b').add_drive(disk)
+        self.vm_b.add_incoming("exec: cat '" + migfile + "'")
+
+    def test_migration_persistent_shared_offline(self):
+        """ A kind of suspend-resume """
+        granularity = 512
+        regions = [
+            { 'start': 0,           'count': 0x10000 },
+            { 'start': 0xf0000,     'count': 0x10000 },
+            { 'start': 0xa0201,     'count': 0x1000  }
+            ]
+
+        result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0',
+                               name='bitmap0', granularity=granularity,
+                               persistent=True, autoload=True)
+        self.assert_qmp(result, 'return', {});
+
+        for r in regions:
+            self.vm_a.hmp_qemu_io('drive0',
+                                  'write %d %d' % (r['start'], r['count']))
+
+        result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
+                               node='drive0', name='bitmap0')
+        sha256 = result['return']['sha256']
+
+        result = self.vm_a.qmp('migrate', uri='exec:cat>' + migfile)
+        self.assert_qmp(result, 'return', {});
+        self.assertNotEqual(self.vm_a.event_wait("STOP"), None)
+        self.vm_a.shutdown()
+
+        self.vm_b.launch()
+        self.vm_b.event_wait("RESUME", timeout=10)
+
+        result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256',
+                               node='drive0', name='bitmap0')
+        self.assert_qmp(result, 'return/sha256', sha256);
+
+
+if __name__ == '__main__':
+    iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/169.out
new file mode 100644
index 0000000000..ae1213e6f8
--- /dev/null
+++ b/tests/qemu-iotests/169.out
@@ -0,0 +1,5 @@
+.
+----------------------------------------------------------------------
+Ran 1 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 3e688678dd..e879c7ebc7 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -169,6 +169,7 @@
 162 auto quick
 163 rw auto quick
 165 rw auto quick
+169 rw auto quick
 170 rw auto quick
 171 rw auto quick
 172 auto
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6f057904a9..ce029b51e2 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -430,6 +430,10 @@ def verify_platform(supported_oses=['linux']):
     if True not in [sys.platform.startswith(x) for x in supported_oses]:
         notrun('not suitable for this OS: %s' % sys.platform)
 
+def verify_cache_mode(supported_cache_modes=[]):
+    if supported_cache_modes and (cachemode not in supported_cache_modes):
+        notrun('not suitable for this cache mode: %s' % cachemode)
+
 def supports_quorum():
     return 'quorum' in qemu_img_pipe('--help')
 
@@ -438,7 +442,7 @@ def verify_quorum():
     if not supports_quorum():
         notrun('quorum support missing')
 
-def main(supported_fmts=[], supported_oses=['linux']):
+def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[]):
     '''Run tests'''
 
     global debug
@@ -455,6 +459,7 @@ def main(supported_fmts=[], supported_oses=['linux']):
     verbosity = 1
     verify_image_format(supported_fmts)
     verify_platform(supported_oses)
+    verify_cache_mode(supported_cache_modes)
 
     # We need to filter out the time taken from the output so that qemu-iotest
     # can reliably diff the results against master output.
-- 
2.11.1

Re: [Qemu-devel] [PATCH 4/4] iotests: add dirty bitmap migration test

[Vladimir Sementsov-Ogievskiy]

28.11.2017 10:14, Vladimir Sementsov-Ogievskiy wrote:
> The test creates two vms (vm_a, vm_b), create dirty bitmap in
> the first one, do several writes to corresponding device and
> then migrate vm_a to vm_b with dirty bitmaps.
>
> For now, only migration through shared storage for persistent
> bitmaps is available, so it is tested here. Only offline variant
> is tested for now (a kind of suspend-resume), as it is needed
> to test that this case is successfully fixed by recent patch.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/169        | 82 +++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/169.out    |  5 +++
>   tests/qemu-iotests/group      |  1 +
>   tests/qemu-iotests/iotests.py |  7 +++-
>   4 files changed, 94 insertions(+), 1 deletion(-)
>   create mode 100755 tests/qemu-iotests/169
>   create mode 100644 tests/qemu-iotests/169.out
>
> diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
> new file mode 100755
> index 0000000000..a0f213b274
> --- /dev/null
> +++ b/tests/qemu-iotests/169
> @@ -0,0 +1,82 @@
> +#!/usr/bin/env python
> +#
> +# Tests for dirty bitmaps migration.
> +#
> +# Copyright (c) 2016-2017 Parallels International GmbH
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import os
> +import iotests
> +import time
> +from iotests import qemu_img
> +
> +disk = os.path.join(iotests.test_dir, 'disk')
> +migfile = os.path.join(iotests.test_dir, 'migfile')
> +
> +class TestPersistentDirtyBitmapSuspendResume(iotests.QMPTestCase):
> +
> +    def tearDown(self):
> +        self.vm_a.shutdown()
> +        self.vm_b.shutdown()
> +        os.remove(disk)
> +        os.remove(migfile)
> +
> +    def setUp(self):
> +        qemu_img('create', '-f', iotests.imgfmt, disk, '1M')
> +
> +        self.vm_a = iotests.VM(path_suffix='a').add_drive(disk)
> +        self.vm_a.launch()
> +
> +        self.vm_b = iotests.VM(path_suffix='b').add_drive(disk)
> +        self.vm_b.add_incoming("exec: cat '" + migfile + "'")
> +
> +    def test_migration_persistent_shared_offline(self):
> +        """ A kind of suspend-resume """
> +        granularity = 512
> +        regions = [
> +            { 'start': 0,           'count': 0x10000 },
> +            { 'start': 0xf0000,     'count': 0x10000 },
> +            { 'start': 0xa0201,     'count': 0x1000  }
> +            ]
> +
> +        result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0',
> +                               name='bitmap0', granularity=granularity,
> +                               persistent=True, autoload=True)
> +        self.assert_qmp(result, 'return', {});
> +
> +        for r in regions:
> +            self.vm_a.hmp_qemu_io('drive0',
> +                                  'write %d %d' % (r['start'], r['count']))
> +
> +        result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
> +                               node='drive0', name='bitmap0')
> +        sha256 = result['return']['sha256']
> +
> +        result = self.vm_a.qmp('migrate', uri='exec:cat>' + migfile)
> +        self.assert_qmp(result, 'return', {});
> +        self.assertNotEqual(self.vm_a.event_wait("STOP"), None)
> +        self.vm_a.shutdown()
> +
> +        self.vm_b.launch()
> +        self.vm_b.event_wait("RESUME", timeout=10)
> +
> +        result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256',
> +                               node='drive0', name='bitmap0')
> +        self.assert_qmp(result, 'return/sha256', sha256);
> +
> +
> +if __name__ == '__main__':
> +    iotests.main(supported_fmts=['qcow2'])
> diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/169.out
> new file mode 100644
> index 0000000000..ae1213e6f8
> --- /dev/null
> +++ b/tests/qemu-iotests/169.out
> @@ -0,0 +1,5 @@
> +.
> +----------------------------------------------------------------------
> +Ran 1 tests
> +
> +OK
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 3e688678dd..e879c7ebc7 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -169,6 +169,7 @@
>   162 auto quick
>   163 rw auto quick
>   165 rw auto quick
> +169 rw auto quick
>   170 rw auto quick
>   171 rw auto quick
>   172 auto
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 6f057904a9..ce029b51e2 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py

following chunks are extra ones, drop them, sorry for this garbage.

> @@ -430,6 +430,10 @@ def verify_platform(supported_oses=['linux']):
>       if True not in [sys.platform.startswith(x) for x in supported_oses]:
>           notrun('not suitable for this OS: %s' % sys.platform)
>   
> +def verify_cache_mode(supported_cache_modes=[]):
> +    if supported_cache_modes and (cachemode not in supported_cache_modes):
> +        notrun('not suitable for this cache mode: %s' % cachemode)
> +
>   def supports_quorum():
>       return 'quorum' in qemu_img_pipe('--help')
>   
> @@ -438,7 +442,7 @@ def verify_quorum():
>       if not supports_quorum():
>           notrun('quorum support missing')
>   
> -def main(supported_fmts=[], supported_oses=['linux']):
> +def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[]):
>       '''Run tests'''
>   
>       global debug
> @@ -455,6 +459,7 @@ def main(supported_fmts=[], supported_oses=['linux']):
>       verbosity = 1
>       verify_image_format(supported_fmts)
>       verify_platform(supported_oses)
> +    verify_cache_mode(supported_cache_modes)
>   
>       # We need to filter out the time taken from the output so that qemu-iotest
>       # can reliably diff the results against master output.


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [Qemu-block] [PATCH 3/4] scripts/qemu.py: tiny enhancement for event_wiat

[John Snow]

On 11/28/2017 02:14 AM, Vladimir Sementsov-Ogievskiy wrote:
> - add comment
> - allow int timeout and disallow bool (which has special
>   meaning for pull_event())
> - remove unreachable 'return None'
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  scripts/qemu.py | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 9bfdf6d37d..8763335254 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -291,7 +291,13 @@ class QEMUMachine(object):
>          branch processing on match's value None
>             {"foo": {"bar": 1}} matches {"foo": None}
>             {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
> +
> +        @raise QMPTimeoutError: If timeout period elapses.
> +        @raise QMPConnectError: If some other error occurred.
>          '''
> +
> +        assert isinstance(timeout, float) or isinstance(timeout, int)
> +
>          def event_match(event, match=None):
>              if match is None:
>                  return True
> @@ -316,13 +322,11 @@ class QEMUMachine(object):
>  
>          # Poll for new events
>          while True:
> -            event = self._qmp.pull_event(wait=timeout)
> +            event = self._qmp.pull_event(wait=float(timeout))

Before this patch you *could* ask for no timeout, and this would block
waiting for an event. After, you can't.

Are you making the argument that it is *never* correct to ask for no
timeout?

>              if (event['event'] == name) and event_match(event, match):
>                  return event
>              self._events.append(event)
>  
> -        return None
> -
>      def get_log(self):
>          '''
>          After self.shutdown or failed qemu execution, this returns the output
> 

Re: [Qemu-devel] [Qemu-block] [PATCH 1/4] qcow2-bitmap: add qcow2_reopen_bitmaps_rw_hint()

[John Snow]

On 11/28/2017 02:14 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add version of qcow2_reopen_bitmaps_rw, which do the same work but
> also return a hint about was header updated or not. This will be
> used in the following fix for bitmaps reloading after migration.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/qcow2.h        |  2 ++
>  block/qcow2-bitmap.c | 15 ++++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 6f0ff15dd0..40fa5b7cfe 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -667,6 +667,8 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>                                    void **refcount_table,
>                                    int64_t *refcount_table_size);
>  bool qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp);
> +int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
> +                                 Error **errp);
>  int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp);
>  void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp);
>  int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp);
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index f45e46cfbd..6c80dcda69 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -1002,7 +1002,8 @@ fail:
>      return false;
>  }
>  
> -int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
> +int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
> +                                 Error **errp)
>  {
>      BDRVQcow2State *s = bs->opaque;
>      Qcow2BitmapList *bm_list;
> @@ -1010,6 +1011,10 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
>      GSList *ro_dirty_bitmaps = NULL;
>      int ret = 0;
>  
> +    if (header_updated != NULL) {
> +        *header_updated = false;
> +    }
> +
>      if (s->nb_bitmaps == 0) {
>          /* No bitmaps - nothing to do */
>          return 0;
> @@ -1053,6 +1058,9 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
>              error_setg_errno(errp, -ret, "Can't update bitmap directory");
>              goto out;
>          }
> +        if (header_updated != NULL) {
> +            *header_updated = true;
> +        }
>          g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false);
>      }
>  
> @@ -1063,6 +1071,11 @@ out:
>      return ret;
>  }
>  
> +int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
> +{
> +    return qcow2_reopen_bitmaps_rw_hint(bs, NULL, errp);
> +}
> +
>  /* store_bitmap_data()
>   * Store bitmap to image, filling bitmap table accordingly.
>   */
> 

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [Qemu-block] [PATCH 2/4] qcow2: handle reopening bitmaps on bdrv_invalidate_cache

[John Snow]

On 11/28/2017 02:14 AM, Vladimir Sementsov-Ogievskiy wrote:
> Consider migration with shared storage. Persistent bitmaps are stored
> on bdrv_inactivate. Then, on destination
> process_incoming_migration_bh() calls bdrv_invalidate_cache_all() which
> leads to qcow2_load_autoloading_dirty_bitmaps() which fails if bitmaps
> are already loaded on destination start. In this case we should call
> qcow2_reopen_bitmaps_rw instead.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/qcow2.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 1914a940e5..f5f205e1b7 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1450,7 +1450,13 @@ static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
>          s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
>      }
>  
> -    if (qcow2_load_autoloading_dirty_bitmaps(bs, &local_err)) {
> +    if (bdrv_has_readonly_bitmaps(bs)) {
> +        if (!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_autoloading_dirty_bitmaps(bs, &local_err)) {
>          update_header = false;
>      }
>      if (local_err != NULL) {
> 

It sounds right to me, but I'm going to ask for eyes from Kevin or Max.

Acked-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] iotests: add dirty bitmap migration test

[John Snow]

On 11/28/2017 02:14 AM, Vladimir Sementsov-Ogievskiy wrote:
> The test creates two vms (vm_a, vm_b), create dirty bitmap in
> the first one, do several writes to corresponding device and
> then migrate vm_a to vm_b with dirty bitmaps.
> 
> For now, only migration through shared storage for persistent
> bitmaps is available, so it is tested here. Only offline variant
> is tested for now (a kind of suspend-resume), as it is needed
> to test that this case is successfully fixed by recent patch.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  tests/qemu-iotests/169        | 82 +++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/169.out    |  5 +++
>  tests/qemu-iotests/group      |  1 +
>  tests/qemu-iotests/iotests.py |  7 +++-
>  4 files changed, 94 insertions(+), 1 deletion(-)
>  create mode 100755 tests/qemu-iotests/169
>  create mode 100644 tests/qemu-iotests/169.out
> 
> diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
> new file mode 100755
> index 0000000000..a0f213b274
> --- /dev/null
> +++ b/tests/qemu-iotests/169
> @@ -0,0 +1,82 @@
> +#!/usr/bin/env python
> +#
> +# Tests for dirty bitmaps migration.
> +#
> +# Copyright (c) 2016-2017 Parallels International GmbH
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import os
> +import iotests
> +import time
> +from iotests import qemu_img
> +
> +disk = os.path.join(iotests.test_dir, 'disk')
> +migfile = os.path.join(iotests.test_dir, 'migfile')
> +
> +class TestPersistentDirtyBitmapSuspendResume(iotests.QMPTestCase):
> +
> +    def tearDown(self):
> +        self.vm_a.shutdown()
> +        self.vm_b.shutdown()
> +        os.remove(disk)
> +        os.remove(migfile)
> +
> +    def setUp(self):
> +        qemu_img('create', '-f', iotests.imgfmt, disk, '1M')
> +
> +        self.vm_a = iotests.VM(path_suffix='a').add_drive(disk)
> +        self.vm_a.launch()
> +
> +        self.vm_b = iotests.VM(path_suffix='b').add_drive(disk)
> +        self.vm_b.add_incoming("exec: cat '" + migfile + "'")
> +
> +    def test_migration_persistent_shared_offline(self):
> +        """ A kind of suspend-resume """
> +        granularity = 512
> +        regions = [
> +            { 'start': 0,           'count': 0x10000 },
> +            { 'start': 0xf0000,     'count': 0x10000 },
> +            { 'start': 0xa0201,     'count': 0x1000  }
> +            ]
> +
> +        result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0',
> +                               name='bitmap0', granularity=granularity,
> +                               persistent=True, autoload=True)
> +        self.assert_qmp(result, 'return', {});
> +
> +        for r in regions:
> +            self.vm_a.hmp_qemu_io('drive0',
> +                                  'write %d %d' % (r['start'], r['count']))
> +
> +        result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
> +                               node='drive0', name='bitmap0')
> +        sha256 = result['return']['sha256']
> +
> +        result = self.vm_a.qmp('migrate', uri='exec:cat>' + migfile)
> +        self.assert_qmp(result, 'return', {});
> +        self.assertNotEqual(self.vm_a.event_wait("STOP"), None)
> +        self.vm_a.shutdown()
> +
> +        self.vm_b.launch()
> +        self.vm_b.event_wait("RESUME", timeout=10)
> +
> +        result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256',
> +                               node='drive0', name='bitmap0')
> +        self.assert_qmp(result, 'return/sha256', sha256);
> +
> +
> +if __name__ == '__main__':
> +    iotests.main(supported_fmts=['qcow2'])
> diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/169.out
> new file mode 100644
> index 0000000000..ae1213e6f8
> --- /dev/null
> +++ b/tests/qemu-iotests/169.out
> @@ -0,0 +1,5 @@
> +.
> +----------------------------------------------------------------------
> +Ran 1 tests
> +
> +OK
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 3e688678dd..e879c7ebc7 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -169,6 +169,7 @@
>  162 auto quick
>  163 rw auto quick
>  165 rw auto quick
> +169 rw auto quick
>  170 rw auto quick
>  171 rw auto quick
>  172 auto

Looks good without the extraneous bits, thank you.

We're obviously late for 2.11, so I recommend CC qemu-stable after
review and we'll include it in 2.11.1.

Reviewed-by: John Snow <jsnow@redhat.com>

> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 6f057904a9..ce029b51e2 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -430,6 +430,10 @@ def verify_platform(supported_oses=['linux']):
>      if True not in [sys.platform.startswith(x) for x in supported_oses]:
>          notrun('not suitable for this OS: %s' % sys.platform)
>  
> +def verify_cache_mode(supported_cache_modes=[]):
> +    if supported_cache_modes and (cachemode not in supported_cache_modes):
> +        notrun('not suitable for this cache mode: %s' % cachemode)
> +
>  def supports_quorum():
>      return 'quorum' in qemu_img_pipe('--help')
>  
> @@ -438,7 +442,7 @@ def verify_quorum():
>      if not supports_quorum():
>          notrun('quorum support missing')
>  
> -def main(supported_fmts=[], supported_oses=['linux']):
> +def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[]):
>      '''Run tests'''
>  
>      global debug
> @@ -455,6 +459,7 @@ def main(supported_fmts=[], supported_oses=['linux']):
>      verbosity = 1
>      verify_image_format(supported_fmts)
>      verify_platform(supported_oses)
> +    verify_cache_mode(supported_cache_modes)
>  
>      # We need to filter out the time taken from the output so that qemu-iotest
>      # can reliably diff the results against master output.
> 

Re: [Qemu-devel] [Qemu-block] [PATCH 3/4] scripts/qemu.py: tiny enhancement for event_wiat

[Vladimir Sementsov-Ogievskiy]

06.12.2017 01:59, John Snow wrote:
>
> On 11/28/2017 02:14 AM, Vladimir Sementsov-Ogievskiy wrote:
>> - add comment
>> - allow int timeout and disallow bool (which has special
>>    meaning for pull_event())
>> - remove unreachable 'return None'
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   scripts/qemu.py | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>> index 9bfdf6d37d..8763335254 100644
>> --- a/scripts/qemu.py
>> +++ b/scripts/qemu.py
>> @@ -291,7 +291,13 @@ class QEMUMachine(object):
>>           branch processing on match's value None
>>              {"foo": {"bar": 1}} matches {"foo": None}
>>              {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
>> +
>> +        @raise QMPTimeoutError: If timeout period elapses.
>> +        @raise QMPConnectError: If some other error occurred.
>>           '''
>> +
>> +        assert isinstance(timeout, float) or isinstance(timeout, int)
>> +
>>           def event_match(event, match=None):
>>               if match is None:
>>                   return True
>> @@ -316,13 +322,11 @@ class QEMUMachine(object):
>>   
>>           # Poll for new events
>>           while True:
>> -            event = self._qmp.pull_event(wait=timeout)
>> +            event = self._qmp.pull_event(wait=float(timeout))
> Before this patch you *could* ask for no timeout, and this would block
> waiting for an event. After, you can't.
>
> Are you making the argument that it is *never* correct to ask for no
> timeout?

hm, you are right, I'll fix.

>
>>               if (event['event'] == name) and event_match(event, match):
>>                   return event
>>               self._events.append(event)
>>   
>> -        return None
>> -
>>       def get_log(self):
>>           '''
>>           After self.shutdown or failed qemu execution, this returns the output
>>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 3/4] scripts/qemu.py: tiny enhancement for event_wiat

[Vladimir Sementsov-Ogievskiy]

let's just drop this patch.

28.11.2017 10:14, Vladimir Sementsov-Ogievskiy wrote:
> - add comment
> - allow int timeout and disallow bool (which has special
>    meaning for pull_event())
> - remove unreachable 'return None'
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   scripts/qemu.py | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 9bfdf6d37d..8763335254 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -291,7 +291,13 @@ class QEMUMachine(object):
>           branch processing on match's value None
>              {"foo": {"bar": 1}} matches {"foo": None}
>              {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
> +
> +        @raise QMPTimeoutError: If timeout period elapses.
> +        @raise QMPConnectError: If some other error occurred.
>           '''
> +
> +        assert isinstance(timeout, float) or isinstance(timeout, int)
> +
>           def event_match(event, match=None):
>               if match is None:
>                   return True
> @@ -316,13 +322,11 @@ class QEMUMachine(object):
>   
>           # Poll for new events
>           while True:
> -            event = self._qmp.pull_event(wait=timeout)
> +            event = self._qmp.pull_event(wait=float(timeout))
>               if (event['event'] == name) and event_match(event, match):
>                   return event
>               self._events.append(event)
>   
> -        return None
> -
>       def get_log(self):
>           '''
>           After self.shutdown or failed qemu execution, this returns the output


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 4/4] iotests: add dirty bitmap migration test

[Vladimir Sementsov-Ogievskiy]

28.11.2017 10:14, Vladimir Sementsov-Ogievskiy wrote:
> The test creates two vms (vm_a, vm_b), create dirty bitmap in
> the first one, do several writes to corresponding device and
> then migrate vm_a to vm_b with dirty bitmaps.
>
> For now, only migration through shared storage for persistent
> bitmaps is available, so it is tested here. Only offline variant
> is tested for now (a kind of suspend-resume), as it is needed
> to test that this case is successfully fixed by recent patch.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/169        | 82 +++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/169.out    |  5 +++
>   tests/qemu-iotests/group      |  1 +
>   tests/qemu-iotests/iotests.py |  7 +++-
>   4 files changed, 94 insertions(+), 1 deletion(-)
>   create mode 100755 tests/qemu-iotests/169
>   create mode 100644 tests/qemu-iotests/169.out
>
> diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
> new file mode 100755
> index 0000000000..a0f213b274
> --- /dev/null
> +++ b/tests/qemu-iotests/169
> @@ -0,0 +1,82 @@
> +#!/usr/bin/env python
> +#
> +# Tests for dirty bitmaps migration.
> +#
> +# Copyright (c) 2016-2017 Parallels International GmbH
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import os
> +import iotests
> +import time
> +from iotests import qemu_img
> +
> +disk = os.path.join(iotests.test_dir, 'disk')
> +migfile = os.path.join(iotests.test_dir, 'migfile')
> +
> +class TestPersistentDirtyBitmapSuspendResume(iotests.QMPTestCase):
> +
> +    def tearDown(self):
> +        self.vm_a.shutdown()
> +        self.vm_b.shutdown()
> +        os.remove(disk)
> +        os.remove(migfile)
> +
> +    def setUp(self):
> +        qemu_img('create', '-f', iotests.imgfmt, disk, '1M')
> +
> +        self.vm_a = iotests.VM(path_suffix='a').add_drive(disk)
> +        self.vm_a.launch()
> +
> +        self.vm_b = iotests.VM(path_suffix='b').add_drive(disk)
> +        self.vm_b.add_incoming("exec: cat '" + migfile + "'")
> +
> +    def test_migration_persistent_shared_offline(self):
> +        """ A kind of suspend-resume """
> +        granularity = 512
> +        regions = [
> +            { 'start': 0,           'count': 0x10000 },
> +            { 'start': 0xf0000,     'count': 0x10000 },
> +            { 'start': 0xa0201,     'count': 0x1000  }
> +            ]
> +
> +        result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0',
> +                               name='bitmap0', granularity=granularity,
> +                               persistent=True, autoload=True)
> +        self.assert_qmp(result, 'return', {});
> +
> +        for r in regions:
> +            self.vm_a.hmp_qemu_io('drive0',
> +                                  'write %d %d' % (r['start'], r['count']))
> +
> +        result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
> +                               node='drive0', name='bitmap0')
> +        sha256 = result['return']['sha256']
> +
> +        result = self.vm_a.qmp('migrate', uri='exec:cat>' + migfile)
> +        self.assert_qmp(result, 'return', {});
> +        self.assertNotEqual(self.vm_a.event_wait("STOP"), None)
> +        self.vm_a.shutdown()
> +
> +        self.vm_b.launch()
> +        self.vm_b.event_wait("RESUME", timeout=10)

with previous patch dropped, please fix it to be 10.0

> +
> +        result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256',
> +                               node='drive0', name='bitmap0')
> +        self.assert_qmp(result, 'return/sha256', sha256);
> +
> +
> +if __name__ == '__main__':
> +    iotests.main(supported_fmts=['qcow2'])
> diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/169.out
> new file mode 100644
> index 0000000000..ae1213e6f8
> --- /dev/null
> +++ b/tests/qemu-iotests/169.out
> @@ -0,0 +1,5 @@
> +.
> +----------------------------------------------------------------------
> +Ran 1 tests
> +
> +OK
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 3e688678dd..e879c7ebc7 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -169,6 +169,7 @@
>   162 auto quick
>   163 rw auto quick
>   165 rw auto quick
> +169 rw auto quick
>   170 rw auto quick
>   171 rw auto quick
>   172 auto
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 6f057904a9..ce029b51e2 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -430,6 +430,10 @@ def verify_platform(supported_oses=['linux']):
>       if True not in [sys.platform.startswith(x) for x in supported_oses]:
>           notrun('not suitable for this OS: %s' % sys.platform)
>   
> +def verify_cache_mode(supported_cache_modes=[]):
> +    if supported_cache_modes and (cachemode not in supported_cache_modes):
> +        notrun('not suitable for this cache mode: %s' % cachemode)
> +
>   def supports_quorum():
>       return 'quorum' in qemu_img_pipe('--help')
>   
> @@ -438,7 +442,7 @@ def verify_quorum():
>       if not supports_quorum():
>           notrun('quorum support missing')
>   
> -def main(supported_fmts=[], supported_oses=['linux']):
> +def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[]):
>       '''Run tests'''
>   
>       global debug
> @@ -455,6 +459,7 @@ def main(supported_fmts=[], supported_oses=['linux']):
>       verbosity = 1
>       verify_image_format(supported_fmts)
>       verify_platform(supported_oses)
> +    verify_cache_mode(supported_cache_modes)
>   
>       # We need to filter out the time taken from the output so that qemu-iotest
>       # can reliably diff the results against master output.


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] iotests: add dirty bitmap migration test

[John Snow]

On 12/06/2017 04:51 AM, Vladimir Sementsov-Ogievskiy wrote:
> 28.11.2017 10:14, Vladimir Sementsov-Ogievskiy wrote:
>> The test creates two vms (vm_a, vm_b), create dirty bitmap in
>> the first one, do several writes to corresponding device and
>> then migrate vm_a to vm_b with dirty bitmaps.
>>
>> For now, only migration through shared storage for persistent
>> bitmaps is available, so it is tested here. Only offline variant
>> is tested for now (a kind of suspend-resume), as it is needed
>> to test that this case is successfully fixed by recent patch.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/169        | 82
>> +++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/169.out    |  5 +++
>>   tests/qemu-iotests/group      |  1 +
>>   tests/qemu-iotests/iotests.py |  7 +++-
>>   4 files changed, 94 insertions(+), 1 deletion(-)
>>   create mode 100755 tests/qemu-iotests/169
>>   create mode 100644 tests/qemu-iotests/169.out
>>
>> diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
>> new file mode 100755
>> index 0000000000..a0f213b274
>> --- /dev/null
>> +++ b/tests/qemu-iotests/169
>> @@ -0,0 +1,82 @@
>> +#!/usr/bin/env python
>> +#
>> +# Tests for dirty bitmaps migration.
>> +#
>> +# Copyright (c) 2016-2017 Parallels International GmbH
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +#
>> +
>> +import os
>> +import iotests
>> +import time
>> +from iotests import qemu_img
>> +
>> +disk = os.path.join(iotests.test_dir, 'disk')
>> +migfile = os.path.join(iotests.test_dir, 'migfile')
>> +
>> +class TestPersistentDirtyBitmapSuspendResume(iotests.QMPTestCase):
>> +
>> +    def tearDown(self):
>> +        self.vm_a.shutdown()
>> +        self.vm_b.shutdown()
>> +        os.remove(disk)
>> +        os.remove(migfile)
>> +
>> +    def setUp(self):
>> +        qemu_img('create', '-f', iotests.imgfmt, disk, '1M')
>> +
>> +        self.vm_a = iotests.VM(path_suffix='a').add_drive(disk)
>> +        self.vm_a.launch()
>> +
>> +        self.vm_b = iotests.VM(path_suffix='b').add_drive(disk)
>> +        self.vm_b.add_incoming("exec: cat '" + migfile + "'")
>> +
>> +    def test_migration_persistent_shared_offline(self):
>> +        """ A kind of suspend-resume """
>> +        granularity = 512
>> +        regions = [
>> +            { 'start': 0,           'count': 0x10000 },
>> +            { 'start': 0xf0000,     'count': 0x10000 },
>> +            { 'start': 0xa0201,     'count': 0x1000  }
>> +            ]
>> +
>> +        result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0',
>> +                               name='bitmap0', granularity=granularity,
>> +                               persistent=True, autoload=True)
>> +        self.assert_qmp(result, 'return', {});
>> +
>> +        for r in regions:
>> +            self.vm_a.hmp_qemu_io('drive0',
>> +                                  'write %d %d' % (r['start'],
>> r['count']))
>> +
>> +        result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
>> +                               node='drive0', name='bitmap0')
>> +        sha256 = result['return']['sha256']
>> +
>> +        result = self.vm_a.qmp('migrate', uri='exec:cat>' + migfile)
>> +        self.assert_qmp(result, 'return', {});
>> +        self.assertNotEqual(self.vm_a.event_wait("STOP"), None)
>> +        self.vm_a.shutdown()
>> +
>> +        self.vm_b.launch()
>> +        self.vm_b.event_wait("RESUME", timeout=10)
> 
> with previous patch dropped, please fix it to be 10.0

Oh, I see, it gets confused over integral values? We should probably fix
that but it can be separate for now.

Everything looks good to me in that case, thanks