fix bitmaps migration through shared storage

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 <[email protected]>

---
 block/qcow2.h        |  2 ++
 block/qcow2-bitmap.c | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

block/qcow2.h +2 -0

index 6f0ff15dd0..40fa5b7cfe 100644
@@ -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);

block/qcow2-bitmap.c +9 -1

index f45e46cfbd..6c80dcda69 100644
@@ -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;

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 <[email protected]>

---
 block/qcow2.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

block/qcow2.c +0 -1

index 1914a940e5..f5f205e1b7 100644
@@ -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)) {

- 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 <[email protected]>

---
 scripts/qemu.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

scripts/qemu.py +6 -0

index 9bfdf6d37d..8763335254 100644
@@ -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

scripts/qemu.py +0 -2

             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

Quoted 29 lines · 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 <[email protected]>

---
  scripts/qemu.py | 10 +++++++---
  1 file changed, 7 insertions(+), 3 deletions(-)

scripts/qemu.py +6 -0

index 9bfdf6d37d..8763335254 100644
@@ -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

scripts/qemu.py class QEMUMachine(object): · line 325

Quoted 5 lines

scripts/qemu.py +1 -1

@@ -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?

Quoted 11 lines

scripts/qemu.py +0 -2

              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

- 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 <[email protected]>

---
  scripts/qemu.py | 10 +++++++---
  1 file changed, 7 insertions(+), 3 deletions(-)

scripts/qemu.py +7 -3

index 9bfdf6d37d..8763335254 100644
@@ -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

tests/qemu-iotests/iotests.py +6 -1

@@ -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.

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 <[email protected]>

---
 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

tests/qemu-iotests/169 new file +82 -0

new file mode 100755
index 0000000000..a0f213b274
@@ -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'])

tests/qemu-iotests/169.out new file +5 -0

new file mode 100644
index 0000000000..ae1213e6f8
@@ -0,0 +1,5 @@
+.
+----------------------------------------------------------------------
+Ran 1 tests
+
+OK

tests/qemu-iotests/group +0 -0

index 3e688678dd..e879c7ebc7 100644

tests/qemu-iotests/iotests.py +6 -1

index 6f057904a9..ce029b51e2 100644
@@ -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.

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 <[email protected]>

---
  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

tests/qemu-iotests/169 new file +64 -0

new file mode 100755
index 0000000000..a0f213b274
@@ -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',

tests/qemu-iotests/169 new file +8 -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'])

tests/qemu-iotests/169.out new file +5 -0

new file mode 100644
index 0000000000..ae1213e6f8
@@ -0,0 +1,5 @@
+.
+----------------------------------------------------------------------
+Ran 1 tests
+
+OK

tests/qemu-iotests/group +1 -0

index 3e688678dd..e879c7ebc7 100644
@@ -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

tests/qemu-iotests/iotests.py +6 -1

index 6f057904a9..ce029b51e2 100644
@@ -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.

Quoted 95 lines · 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 <[email protected]>

---
  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

tests/qemu-iotests/169 new file +62 -0

new file mode 100755
index 0000000000..a0f213b274
@@ -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']))

tests/qemu-iotests/169 new file +6 -0

+
+        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)

Quoted 6 lines

+        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