Message ID | 1425528911-10300-11-git-send-email-jsnow@redhat.com |
---|---|
State | New |
Headers | show |
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 <jsnow@redhat.com> > --- > 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 <mreitz@redhat.com>
On 03/17/2015 04:50 PM, Max Reitz wrote: > 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 <jsnow@redhat.com> >> --- >> 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? > Yes. (Poor kitty.) >> 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. > Also yes. >> 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 <mreitz@redhat.com> > Thanks!
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() 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] - 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,
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 <jsnow@redhat.com> --- tests/qemu-iotests/124 | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)