[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2 3/3] qemu-iotests: Add 088 new test for drive
From: |
Benoît Canet |
Subject: |
Re: [Qemu-devel] [PATCH V2 3/3] qemu-iotests: Add 088 new test for drive-mirror-replace. |
Date: |
Fri, 14 Mar 2014 15:57:01 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
The Thursday 13 Mar 2014 à 22:31:01 (+0100), Max Reitz wrote :
> On 11.03.2014 22:53, Benoît Canet wrote:
> >Tests for drive-mirror-replace whose purpose is to enable quorum file
> >mirroring
> >and replacement after failure.
> >
> >Signed-off-by: Benoit Canet <address@hidden>
> >---
> > tests/qemu-iotests/041 | 34 +------
> > tests/qemu-iotests/088 | 221
> > ++++++++++++++++++++++++++++++++++++++++++
> > tests/qemu-iotests/088.out | 5 +
> > tests/qemu-iotests/group | 1 +
> > tests/qemu-iotests/iotests.py | 33 +++++++
> > 5 files changed, 261 insertions(+), 33 deletions(-)
> > create mode 100755 tests/qemu-iotests/088
> > create mode 100644 tests/qemu-iotests/088.out
> >
> >diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
> >index ec470b2..10535a6 100755
> >--- a/tests/qemu-iotests/041
> >+++ b/tests/qemu-iotests/041
> >@@ -21,45 +21,13 @@
> > import time
> > import os
> > import iotests
> >-from iotests import qemu_img, qemu_io
> >+from iotests import qemu_img, qemu_io, ImageMirroringTestCase
> > backing_img = os.path.join(iotests.test_dir, 'backing.img')
> > target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img')
> > test_img = os.path.join(iotests.test_dir, 'test.img')
> > target_img = os.path.join(iotests.test_dir, 'target.img')
> >-class ImageMirroringTestCase(iotests.QMPTestCase):
> >- '''Abstract base class for image mirroring test cases'''
> >-
> >- def wait_ready(self, drive='drive0'):
> >- '''Wait until a block job BLOCK_JOB_READY event'''
> >- ready = False
> >- while not ready:
> >- for event in self.vm.get_qmp_events(wait=True):
> >- if event['event'] == 'BLOCK_JOB_READY':
> >- self.assert_qmp(event, 'data/type', 'mirror')
> >- self.assert_qmp(event, 'data/device', drive)
> >- ready = True
> >-
> >- def wait_ready_and_cancel(self, drive='drive0'):
> >- self.wait_ready(drive)
> >- event = self.cancel_and_wait()
> >- self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED')
> >- self.assert_qmp(event, 'data/type', 'mirror')
> >- self.assert_qmp(event, 'data/offset', self.image_len)
> >- self.assert_qmp(event, 'data/len', self.image_len)
> >-
> >- def complete_and_wait(self, drive='drive0', wait_ready=True):
> >- '''Complete a block job and wait for it to finish'''
> >- if wait_ready:
> >- self.wait_ready()
> >-
> >- result = self.vm.qmp('block-job-complete', device=drive)
> >- self.assert_qmp(result, 'return', {})
> >-
> >- event = self.wait_until_completed()
> >- self.assert_qmp(event, 'data/type', 'mirror')
> >-
> > class TestSingleDrive(ImageMirroringTestCase):
> > image_len = 1 * 1024 * 1024 # MB
> >diff --git a/tests/qemu-iotests/088 b/tests/qemu-iotests/088
> >new file mode 100755
> >index 0000000..326d307
> >--- /dev/null
> >+++ b/tests/qemu-iotests/088
> >@@ -0,0 +1,221 @@
> >+#!/usr/bin/env python
> >+#
> >+# Tests for Quorum image replacement
> >+#
> >+# Copyright (C) 2014 Nodalink, EURL.
> >+#
> >+# based on 041
> >+#
> >+# 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 subprocess
> >+import time
> >+import os
> >+import iotests
> >+from iotests import qemu_img, qemu_img_args
> >+from iotests import ImageMirroringTestCase
> >+
> >+quorum_img1 = os.path.join(iotests.test_dir, 'quorum1.img')
> >+quorum_img2 = os.path.join(iotests.test_dir, 'quorum2.img')
> >+quorum_img3 = os.path.join(iotests.test_dir, 'quorum3.img')
> >+quorum_backup_img = os.path.join(iotests.test_dir, 'quorum_backup.img')
> >+quorum_repair_img = os.path.join(iotests.test_dir, 'quorum_repair.img')
> >+quorum_snapshot_file = os.path.join(iotests.test_dir, 'quorum_snapshot.img')
> >+
> >+class TestRepairQuorum(ImageMirroringTestCase):
> >+ """ This class test quorum file repair using drive-mirror.
> >+ It's mostly a fork of TestSingleDrive. """
> >+ image_len = 1 * 1024 * 1024 # MB
> >+ IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
> >+
> >+ def setUp(self):
> >+ self.vm = iotests.VM()
> >+
> >+ # Add each individual quorum images
> >+ for i in self.IMAGES:
> >+ qemu_img('create', '-f', iotests.imgfmt, i,
> >+ str(self.image_len))
> >+ # Assign a node name to each quorum image in order to manipulate
> >+ # them
> >+ opts = "node-name=img%i" % self.IMAGES.index(i)
> >+ self.vm = self.vm.add_drive(i, opts)
> >+
> >+ self.vm.launch()
> >+
> >+ #assemble the quorum block device from the individual files
> >+ args = { "options" : { "driver": "quorum", "id": "quorum0",
> >+ "vote-threshold": 2, "children": [ "img0", "img1", "img2"
> >] } }
> >+ result = self.vm.qmp("blockdev-add", **args)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+
> >+ def tearDown(self):
> >+ self.vm.shutdown()
> >+ for i in self.IMAGES + [ quorum_repair_img ]:
> >+ # Do a try/except because the test may have deleted some images
> >+ try:
> >+ os.remove(i)
> >+ except OSError:
> >+ pass
> >+
> >+ def test_complete(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> >+ target=quorum_repair_img,
> >format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.wait_ready(drive='quorum0')
> >+
> >+ result = self.vm.qmp('drive-mirror-replace', device='quorum0',
> >+ target_reference='img1', new_node_name='img11')
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ event = self.wait_until_completed(drive='quorum0')
> >+ self.assert_qmp(event, 'data/type', 'mirror')
> >+
> >+ result = self.vm.qmp('query-named-block-nodes')
> >+ self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
> >+ # TODO: a better test requiring some QEMU infrastructure will be
> >added
> >+ # to check that this file is really driven by quorum
> >+ self.vm.shutdown()
> >+ self.assertTrue(iotests.compare_images(quorum_img2,
> >quorum_repair_img),
> >+ 'target image does not match source after
> >mirroring')
>
> Considering you didn't write anything to the images, this assertion
> failing would be rather surprising. Perhaps you could write some
> test data, so the drive-mirror blockjob actually does something? (or
> maybe I'm mistaken and there is indeed some data)
I use sync=full so there is some data.
>
> >+
> >+ def test_device_not_active(self):
> >+ result = self.vm.qmp('drive-mirror-replace', device='quorum12',
> >+ target_reference='img1', new_node_name='img11')
> >+ self.assert_qmp(result, 'error/class', 'DeviceNotActive')
>
> Hm, this tests what happens if the block device given as the
> mirroring source does not exist - I could imagine another test for
> what happens if the block device does exist, but there is no
> (mirroring) blockjob running. Without any blockjob running, the
> error path in the newly added code is exactly the same, however
> (find_block_job() fails); and to test what happens if a different
> blockjob type is running ("Can only be used on a drive-mirror block
> job" should be returned) would probably be more difficult.
>
> Thus, this test alone probably suffices.
>
> >+ self.vm.shutdown()
> >+
> >+ def test_sync_no_full(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='none',
> >+ target=quorum_repair_img,
> >format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.wait_ready(drive='quorum0')
> >+
> >+ result = self.vm.qmp('drive-mirror-replace', device='quorum0',
> >+ target_reference='img1', new_node_name='img11')
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ event = self.cancel_and_wait(drive='quorum0', resume=True)
> >+ self.assert_qmp(event, 'data/type', 'mirror')
> >+
> >+ self.vm.shutdown()
> >+
> >+ def test_sync_empty_reference(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='none',
>
> Since this test is not about the sync mode, this should most likely
> be sync='full' again, I guess. The same applies to all the following
> test functions.
true, thanks
>
> >+ target=quorum_repair_img,
> >format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.wait_ready(drive='quorum0')
> >+
> >+ result = self.vm.qmp('drive-mirror-replace', device='quorum0',
> >+ target_reference='', new_node_name='img11')
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ event = self.cancel_and_wait(drive='quorum0', resume=True)
> >+ self.assert_qmp(event, 'data/type', 'mirror')
> >+
> >+ self.vm.shutdown()
> >+
> >+ def test_sync_no_reference(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='none',
> >+ target=quorum_repair_img,
> >format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.wait_ready(drive='quorum0')
> >+
> >+ result = self.vm.qmp('drive-mirror-replace', device='quorum0',
> >+ new_node_name='img11')
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ event = self.cancel_and_wait(drive='quorum0', resume=True)
> >+ self.assert_qmp(event, 'data/type', 'mirror')
> >+
> >+ self.vm.shutdown()
> >+
> >+ def test_sync_no_node_name(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='none',
> >+ target=quorum_repair_img,
> >format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.wait_ready(drive='quorum0')
> >+
> >+ result = self.vm.qmp('drive-mirror-replace', device='quorum0',
> >+ target_reference='img1')
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ event = self.cancel_and_wait(drive='quorum0', resume=True)
> >+ self.assert_qmp(event, 'data/type', 'mirror')
> >+
> >+ self.vm.shutdown()
> >+
> >+ def test_sync_empty_node_name(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='none',
> >+ target=quorum_repair_img,
> >format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.wait_ready(drive='quorum0')
> >+
> >+ result = self.vm.qmp('drive-mirror-replace', device='quorum0',
> >+ target_reference='img1', new_node_name='')
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ event = self.cancel_and_wait(drive='quorum0', resume=True)
> >+ self.assert_qmp(event, 'data/type', 'mirror')
> >+
> >+ self.vm.shutdown()
> >+
> >+ def test_source_and_dest_equal(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='none',
> >+ target=quorum_repair_img,
> >format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.wait_ready(drive='quorum0')
> >+
> >+ result = self.vm.qmp('drive-mirror-replace', device='quorum0',
> >+ target_reference='quorum0',
> >new_node_name='img11')
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ event = self.cancel_and_wait(drive='quorum0', resume=True)
> >+ self.assert_qmp(event, 'data/type', 'mirror')
> >+
> >+ self.vm.shutdown()
>
> In these above test functions, you maybe could even compare the
> error message in order to make sure it's always appropriate. But the
> most important thing is that these are reported as errors, hence I'm
> fine with the GenericError assertions.
>
> >+
> >+if __name__ == '__main__':
> >+ p1 = subprocess.Popen(qemu_img_args, stdout=subprocess.PIPE)
> >+ p2 = subprocess.Popen(["grep", "quorum"], stdin=p1.stdout,
> >stdout=subprocess.PIPE)
> >+ p1.stdout.close() # Allow p1 to receive a SIGsubprocess.PIPE if p2
> >exits.
> >+ has_quorum = len(p2.communicate()[0]) != 0
> >+ if has_quorum:
> >+ iotests.main(supported_fmts=['qcow2', 'qed'])
> >+ else:
> >+ f = open("088.out", "r")
> >+ print(f.read().strip())
> >+ f.close()
>
> I think I'd prefer calling iotests.notrun() here, if possible,
> rather than giving the impression this test succeeded when it was in
> fact not run at all.
>
> I can live with leaving the other issues unchanged, but for this I
> do request a change or at least an explanation before giving a
> reviewed-by. ;-)
Ok
>
> Max
>
> >diff --git a/tests/qemu-iotests/088.out b/tests/qemu-iotests/088.out
> >new file mode 100644
> >index 0000000..594c16f
> >--- /dev/null
> >+++ b/tests/qemu-iotests/088.out
> >@@ -0,0 +1,5 @@
> >+........
> >+----------------------------------------------------------------------
> >+Ran 8 tests
> >+
> >+OK
> >diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> >index b96c6bc..e590d09 100644
> >--- a/tests/qemu-iotests/group
> >+++ b/tests/qemu-iotests/group
> >@@ -89,3 +89,4 @@
> > 085 rw auto quick
> > 086 rw auto quick
> > 087 rw auto quick
> >+088 rw auto
> >diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> >index e4fa9af..a803943 100644
> >--- a/tests/qemu-iotests/iotests.py
> >+++ b/tests/qemu-iotests/iotests.py
> >@@ -272,6 +272,39 @@ class QMPTestCase(unittest.TestCase):
> > self.assert_no_active_block_jobs()
> > return event
> >+# made common here for 041 and 088
> >+class ImageMirroringTestCase(QMPTestCase):
> >+ '''Abstract base class for image mirroring test cases'''
> >+
> >+ def wait_ready(self, drive='drive0'):
> >+ '''Wait until a block job BLOCK_JOB_READY event'''
> >+ ready = False
> >+ while not ready:
> >+ for event in self.vm.get_qmp_events(wait=True):
> >+ if event['event'] == 'BLOCK_JOB_READY':
> >+ self.assert_qmp(event, 'data/type', 'mirror')
> >+ self.assert_qmp(event, 'data/device', drive)
> >+ ready = True
> >+
> >+ def wait_ready_and_cancel(self, drive='drive0'):
> >+ self.wait_ready(drive=drive)
> >+ event = self.cancel_and_wait(drive=drive)
> >+ self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED')
> >+ self.assert_qmp(event, 'data/type', 'mirror')
> >+ self.assert_qmp(event, 'data/offset', self.image_len)
> >+ self.assert_qmp(event, 'data/len', self.image_len)
> >+
> >+ def complete_and_wait(self, drive='drive0', wait_ready=True):
> >+ '''Complete a block job and wait for it to finish'''
> >+ if wait_ready:
> >+ self.wait_ready(drive=drive)
> >+
> >+ result = self.vm.qmp('block-job-complete', device=drive)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ event = self.wait_until_completed(drive=drive)
> >+ self.assert_qmp(event, 'data/type', 'mirror')
> >+
> > def notrun(reason):
> > '''Skip this test suite'''
> > # Each test in qemu-iotests has a number ("seq")
>