[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH 10/11] iotests: 124 - backup_prepare refactoring

From: Max Reitz
Subject: Re: [Qemu-block] [PATCH 10/11] iotests: 124 - backup_prepare refactoring
Date: Tue, 17 Mar 2015 16:50:41 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

On 2015-03-04 at 23:15, John Snow wrote:
Allow tests to call just the backup preparation routine
without invoking a backup. Useful for transactions where
we want to prepare, but don't wish to issue the QMP command.

Signed-off-by: John Snow <address@hidden>
  tests/qemu-iotests/124 | 17 ++++++++++++-----
  1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 2eccc3e..4afdca1 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -100,7 +100,6 @@ class TestIncrementalBackup(iotests.QMPTestCase):
def img_create(self, img, fmt=iotests.imgfmt, size='64M',
                     parent=None, parentFormat=None):
-        plist = list()

Hm. This appears to never have been used. Would it make sense to drop it from "iotests: add invalid input incremental backup tests" in your transactionless series?

          if parent:
              if parentFormat is None:
                  parentFormat = fmt
@@ -140,17 +139,25 @@ class TestIncrementalBackup(iotests.QMPTestCase):
          return bitmap
- def create_incremental(self, bitmap=None, num=None,
-                           parent=None, parentFormat=None, validate=True):
+    def prepare_backup(self, bitmap=None, parent=None):
          if bitmap is None:
              bitmap = self.bitmaps[-1]

Removing this empty line looks like it should never have been added, too.

          if parent is None:
              parent = bitmap.last_target()
- target = bitmap.new_target(num)
+        target = bitmap.new_target()
          self.img_create(target, bitmap.drive['fmt'], parent=parent)
+        return target
+    def create_incremental(self, bitmap=None, parent=None,
+                           parentFormat=None, validate=True):
+        if bitmap is None:
+            bitmap = self.bitmaps[-1]
+        if parent is None:
+            parent = bitmap.last_target()
+        target = self.prepare_backup(bitmap, parent)
          result = self.vm.qmp('drive-backup', device=bitmap.drive['id'],
                               sync='dirty-bitmap', bitmap=bitmap.name,
                               format=bitmap.drive['fmt'], target=target,

Both "issues" are not caused by this patch, however, so whether you squash the hunks somewhere else or not:

Reviewed-by: Max Reitz <address@hidden>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]