On 2015-02-09 at 20:35, John Snow wrote:
Signed-off-by: John Snow <js...@redhat.com>
---
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_image
Is this really necessary? For me, it works without, too; and if it is
necessary, shouldn't it be part of patch 13?
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.
+ 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 % num
I hope nobody has a % in the test path...
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?
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?
+ 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.
+
+ 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
+ # 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'''
