qemu-devel
[Top][All Lists]
Advanced

[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: Max Reitz
Subject: Re: [Qemu-devel] [PATCH V2 3/3] qemu-iotests: Add 088 new test for drive-mirror-replace.
Date: Thu, 13 Mar 2014 22:31:01 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0

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)

+
+    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.

+                             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. ;-)

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")




reply via email to

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