|
From: | John Snow |
Subject: | Re: [Qemu-devel] [PATCH v12 14/17] iotests: add simple incremental backup case |
Date: | Wed, 11 Feb 2015 17:02:54 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 02/11/2015 04:40 PM, Max Reitz wrote:
On 2015-02-09 at 20:35, John Snow wrote:Signed-off-by: John Snow <address@hidden> --- tests/qemu-iotests/112 | 120 +++++++++++++++++++++++++++++++++++++++++- tests/qemu-iotests/112.out | 4 +- tests/qemu-iotests/iotests.py | 18 ++++--- 3 files changed, 133 insertions(+), 9 deletions(-) diff --git a/tests/qemu-iotests/112 b/tests/qemu-iotests/112 index 7985cd1..31431ad 100644 --- a/tests/qemu-iotests/112 +++ b/tests/qemu-iotests/112 @@ -22,18 +22,49 @@ import os import iotests +from iotests import qemu_img, qemu_io, create_imageIs this really necessary? For me, it works without, too; and if it is necessary, shouldn't it be part of patch 13?
Maybe not? It's baggage from copying 056 as a template.
def io_write_patterns(img, patterns): for pattern in patterns: iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img) +class Bitmap: + def __init__(self, name, node): + self.name = name + self.node = node + self.pattern = os.path.join(iotests.test_dir, + '%s.backup.%%s.img' % name)Shouldn't this be %%i? %%s works for me, but I think %%i might look better.
It's probably more semantically appropriate, yes.
+ self.num = 0 + self.backups = list() + + def new_target(self, num=None): + if num is None: + num = self.num + self.num = num + 1 + target = self.pattern % numI hope nobody has a % in the test path...
Oh, good point.
Although it does look nice, maybe just self.base = os.path.join(iotests.test_dir, '%s.backup.img.' % name) and target = self.base + str(num) is more robust (putting the '.img' at the end is a bonus).+ self.backups.append(target) + return target + + def last_target(self): + return self.backups[-1] + + def del_target(self): + os.remove(self.backups.pop()) + self.num -= 1 + + def __del__(self): + for backup in self.backups: + try: + os.remove(backup) + except OSError: + pass class TestIncrementalBackup(iotests.QMPTestCase): def setUp(self): self.bitmaps = list() self.files = list() - self.vm = iotests.VM() + self.vm = iotests.VM().add_args(['-S'])Isn't -machine accel=qtest (which is specified by default) enough?
Yes, this is actually somewhat vestigial from a standalone version of this that would tolerate an arbitrary image. I left it in because I thought it was harmless and it served as an illustration about the inferred timing of the commands being tested.
That said, it /can/ be removed. Its only purpose is illustrative.
self.test_img = os.path.join(iotests.test_dir, 'base.img') self.full_bak = os.path.join(iotests.test_dir, 'backup.img') self.foo_img = os.path.join(iotests.test_dir, 'foo.bar') @@ -58,6 +89,93 @@ class TestIncrementalBackup(iotests.QMPTestCase): iotests.qemu_img('create', '-f', fmt, img, size) self.files.append(img) + + def create_full_backup(self, drive='drive0'): + res = self.vm.qmp('drive-backup', device=drive, + sync='full', format=iotests.imgfmt, + target=self.full_bak) + self.assert_qmp(res, 'return', {}) + self.wait_until_completed(drive) + self.check_full_backup() + self.files.append(self.full_bak) + + + def check_full_backup(self): + self.assertTrue(iotests.compare_images(self.test_img, self.full_bak)) + + + def add_bitmap(self, name, node='drive0'): + bitmap = Bitmap(name, node) + self.bitmaps.append(bitmap) + result = self.vm.qmp('block-dirty-bitmap-add', node=bitmap.node, + name=bitmap.name) + self.assert_qmp(result, 'return', {}) + return bitmap + + + def create_incremental(self, bitmap=None, num=None, + parent=None, parentFormat=None, validate=True): + if bitmap is None: + bitmap = self.bitmaps[-1] + + # If this is the first incremental backup for a bitmap, + # use the full backup as a backing image. Otherwise, use + # the last incremental backup. + if parent is None: + if bitmap.num is 0:Why not == 0?
Just having a really good time in python.I think "is" tests instances; it's like a strict equals. == is equivalence as you and I are used to it. In this case, it winds up being the same because python has some numerical objects cached, and any equation that evaluates to 0 evaluates to the /same/ 0.
== is more correct, though, apparently.
+ parent = self.full_bak + else: + parent = self.bitmaps[-1].last_target()Is this intentional or should this be bitmap.last_target()? In this patch, no caller of create_incremental() gives any parameter, so there's no difference.
Vestigial from the standalone edition. 'bitmap' should be preferred over bitmaps[-1], yes.
+ + target = bitmap.new_target(num) + self.img_create(target, iotests.imgfmt, parent=parent) + + result = self.vm.qmp('drive-backup', device=bitmap.node, + sync='dirty-bitmap', bitmap=bitmap.name, + format=iotests.imgfmt, target=target, + mode='existing') + self.assert_qmp(result, 'return', {}) + + event = self.wait_until_completed(bitmap.node, check_offset=validate) + if validate: + return self.check_incremental(target) + + + def check_incremental(self, target=None): + if target is None: + target = self.bitmaps[-1].last_target() + self.assertTrue(iotests.compare_images(self.test_img, target)) + return True + + + def hmp_io_writes(self, drive, patterns): + for pattern in patterns: + self.vm.hmp_qemu_io(drive, 'write -P%s %s %s' % pattern) + self.vm.hmp_qemu_io(drive, 'flush') + + + # Create a bitmap and full backup before VM execution begins, + # then create a series of incremental backups during execution. + def test_incremental_simple(self): + self.create_full_backup() + self.add_bitmap('bitmap0', 'drive0') + self.vm.hmp('c')self.vm.qmp('continue') works, too, but as I said above, I think accel=qtest should be enough so you may not need this at all. (I'm saying this because we all know that QMP > HMP (obviously)) So I had some small questions which I think I want to have answers before giving an R-b, but the test logic looks good to me. Max
Explanation above covers this; a more human-focused demi-interactive version preceded this iotest.
I'll excise it and just use comments to create the same documentation effect.
+ # Sanity: Create a "hollow" incremental backup + self.create_incremental() + # Three writes: One complete overwrite, one new segment, + # and one partial overlap. + self.hmp_io_writes('drive0', (('0xab', 0, 512), + ('0xfe', '16M', '256k'), + ('0x64', '32736k', '64k'))) + self.create_incremental() + # Three more writes, one of each kind, like above + self.hmp_io_writes('drive0', (('0x9a', 0, 512), + ('0x55', '8M', '352k'), + ('0x78', '15872k', '1M'))) + self.create_incremental() + return True + + def test_sync_dirty_bitmap_missing(self): self.assert_no_active_block_jobs() self.files.append(self.foo_img) diff --git a/tests/qemu-iotests/112.out b/tests/qemu-iotests/112.out index fbc63e6..8d7e996 100644 --- a/tests/qemu-iotests/112.out +++ b/tests/qemu-iotests/112.out @@ -1,5 +1,5 @@ -.. +... ---------------------------------------------------------------------- -Ran 2 tests +Ran 3 tests OK diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 241b5ee..6bff935 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -88,6 +88,11 @@ class VM(object): '-display', 'none', '-vga', 'none'] self._num_drives = 0 + # Add arbitrary arguments to the command-line. + def add_args(self, arglist): + self._args = self._args + arglist + return self + # This can be used to add an unused monitor instance. def add_monitor_telnet(self, ip, port): args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port) @@ -109,23 +114,24 @@ class VM(object): self._num_drives += 1 return self + def hmp(self, args): + return self.qmp('human-monitor-command', + command_line=args) + def pause_drive(self, drive, event=None): '''Pause drive r/w operations''' if not event: self.pause_drive(drive, "read_aio") self.pause_drive(drive, "write_aio") return - self.qmp('human-monitor-command', - command_line='qemu-io %s "break %s bp_%s"' % (drive, event, drive)) + self.hmp('qemu-io %s "break %s bp_%s"' % (drive, event, drive)) def resume_drive(self, drive): - self.qmp('human-monitor-command', - command_line='qemu-io %s "remove_break bp_%s"' % (drive, drive)) + self.hmp('qemu-io %s "remove_break bp_%s"' % (drive, drive)) def hmp_qemu_io(self, drive, cmd): '''Write to a given drive using an HMP command''' - return self.qmp('human-monitor-command', - command_line='qemu-io %s "%s"' % (drive, cmd)) + return self.hmp('qemu-io %s "%s"' % (drive, cmd)) def add_fd(self, fd, fdset, opaque, opts=''): '''Pass a file descriptor to the VM'''
[Prev in Thread] | Current Thread | [Next in Thread] |