diff mbox

[v2] qemu-iotests: Disable Quorum testing in 041 when Quorum is not builtin

Message ID 1403900236-3087-1-git-send-email-benoit.canet@irqsave.net
State New
Headers show

Commit Message

Benoît Canet June 27, 2014, 8:17 p.m. UTC 
This avoid breaking tests on RHEL6 where gnutls is too old for quorum to be
built by default.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 tests/qemu-iotests/041 | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

Comments

Stefan Hajnoczi June 30, 2014, 2:09 p.m. UTC | #1 
On Fri, Jun 27, 2014 at 10:17:16PM +0200, Benoît Canet wrote:
I think this patch can be boiled down to 2 lines:

 if __name__ == '__main__':
+    if 'quorum' not in iotests.qemu_img('--help'):
+        iotests.notrun('not built with quorum support')
     iotests.main(supported_fmts=['qcow2', 'qed'])

What do you think?
Benoît Canet June 30, 2014, 2:25 p.m. UTC | #2 
The Monday 30 Jun 2014 à 16:09:57 (+0200), Stefan Hajnoczi wrote :
> On Fri, Jun 27, 2014 at 10:17:16PM +0200, Benoît Canet wrote:
> I think this patch can be boiled down to 2 lines:
> 
>  if __name__ == '__main__':
> +    if 'quorum' not in iotests.qemu_img('--help'):
> +        iotests.notrun('not built with quorum support')
>      iotests.main(supported_fmts=['qcow2', 'qed'])
> 
> What do you think?

The detection part could work using qemu_img_pipe.

Are we sure that the test won't fail in strange ways like
when using skipTest ?

If so do you want me to respin this patch with your version ?

Best regards

Benoît
Stefan Hajnoczi July 1, 2014, 7:28 a.m. UTC | #3 
On Mon, Jun 30, 2014 at 04:25:47PM +0200, Benoît Canet wrote:
> The Monday 30 Jun 2014 à 16:09:57 (+0200), Stefan Hajnoczi wrote :
> > On Fri, Jun 27, 2014 at 10:17:16PM +0200, Benoît Canet wrote:
> > I think this patch can be boiled down to 2 lines:
> > 
> >  if __name__ == '__main__':
> > +    if 'quorum' not in iotests.qemu_img('--help'):
> > +        iotests.notrun('not built with quorum support')
> >      iotests.main(supported_fmts=['qcow2', 'qed'])
> > 
> > What do you think?
> 
> The detection part could work using qemu_img_pipe.

You're right, I used the wrong function.  We need the stdout.

> Are we sure that the test won't fail in strange ways like
> when using skipTest ?

Not sure what you mean.

> If so do you want me to respin this patch with your version ?

Yes, please.
Kevin Wolf July 1, 2014, 8:10 a.m. UTC | #4 
Am 30.06.2014 um 16:09 hat Stefan Hajnoczi geschrieben:
> On Fri, Jun 27, 2014 at 10:17:16PM +0200, Benoît Canet wrote:
> I think this patch can be boiled down to 2 lines:
> 
>  if __name__ == '__main__':
> +    if 'quorum' not in iotests.qemu_img('--help'):
> +        iotests.notrun('not built with quorum support')
>      iotests.main(supported_fmts=['qcow2', 'qed'])
> 
> What do you think?

That would disable the whole test case 041, whereas Benoît's patch
only disables the quorum-related part. (The test case is really about
migration, not only quorum.)

Kevin
Stefan Hajnoczi July 1, 2014, 8:45 a.m. UTC | #5 
On Tue, Jul 1, 2014 at 10:10 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 30.06.2014 um 16:09 hat Stefan Hajnoczi geschrieben:
>> On Fri, Jun 27, 2014 at 10:17:16PM +0200, Benoît Canet wrote:
>> I think this patch can be boiled down to 2 lines:
>>
>>  if __name__ == '__main__':
>> +    if 'quorum' not in iotests.qemu_img('--help'):
>> +        iotests.notrun('not built with quorum support')
>>      iotests.main(supported_fmts=['qcow2', 'qed'])
>>
>> What do you think?
>
> That would disable the whole test case 041, whereas Benoît's patch
> only disables the quorum-related part. (The test case is really about
> migration, not only quorum.)

Good point.  I thought about skipping TestRepairQuorum to avoid all
those if statements (which are also easy to forget when adding a new
test case) but it would be more hassle than just having the if
statements.

I now think the original approach was right, just simplify
has_quorum() using qemu_img_pipe() instead of using subprocess.

Stefan
diff mbox

Patch

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 0815e19..2b1e8a0 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -18,10 +18,12 @@ 
 # 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_io
+import unittest
 
 backing_img = os.path.join(iotests.test_dir, 'backing.img')
 target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img')
@@ -735,6 +737,15 @@  class TestRepairQuorum(ImageMirroringTestCase):
     image_len = 1 * 1024 * 1024 # MB
     IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
 
+    def has_quorum(self):
+        qemu_img_args = os.environ.get('QEMU_IMG', 'qemu-img').strip().split(' ')
+        qemu_img_args.append("--help")
+        p1 = subprocess.Popen(qemu_img_args, stdout=subprocess.PIPE)
+        p2 = subprocess.Popen(["grep", "quorum"], stdin=p1.stdout,
+                              stdout=subprocess.PIPE)
+        p1.stdout.close()
+        return len(p2.communicate()[0])
+
     def setUp(self):
         self.vm = iotests.VM()
 
@@ -752,8 +763,9 @@  class TestRepairQuorum(ImageMirroringTestCase):
         #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', {})
+        if self.has_quorum():
+            result = self.vm.qmp("blockdev-add", **args)
+            self.assert_qmp(result, 'return', {})
 
 
     def tearDown(self):
@@ -766,6 +778,9 @@  class TestRepairQuorum(ImageMirroringTestCase):
                 pass
 
     def test_complete(self):
+        if not self.has_quorum():
+            return
+
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
@@ -784,6 +799,9 @@  class TestRepairQuorum(ImageMirroringTestCase):
                         'target image does not match source after mirroring')
 
     def test_cancel(self):
+        if not self.has_quorum():
+            return
+
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
@@ -800,6 +818,9 @@  class TestRepairQuorum(ImageMirroringTestCase):
         self.vm.shutdown()
 
     def test_cancel_after_ready(self):
+        if not self.has_quorum():
+            return
+
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
@@ -818,6 +839,9 @@  class TestRepairQuorum(ImageMirroringTestCase):
                         'target image does not match source after mirroring')
 
     def test_pause(self):
+        if not self.has_quorum():
+            return
+
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
@@ -846,6 +870,9 @@  class TestRepairQuorum(ImageMirroringTestCase):
                         'target image does not match source after mirroring')
 
     def test_medium_not_found(self):
+        if not self.has_quorum():
+            return
+
         result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full',
                              node_name='repair0',
                              replaces='img1',
@@ -853,6 +880,9 @@  class TestRepairQuorum(ImageMirroringTestCase):
         self.assert_qmp(result, 'error/class', 'GenericError')
 
     def test_image_not_found(self):
+        if not self.has_quorum():
+            return
+
         result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
                              node_name='repair0',
                              replaces='img1',
@@ -861,6 +891,9 @@  class TestRepairQuorum(ImageMirroringTestCase):
         self.assert_qmp(result, 'error/class', 'GenericError')
 
     def test_device_not_found(self):
+        if not self.has_quorum():
+            return
+
         result = self.vm.qmp('drive-mirror', device='nonexistent', sync='full',
                              node_name='repair0',
                              replaces='img1',
@@ -868,6 +901,9 @@  class TestRepairQuorum(ImageMirroringTestCase):
         self.assert_qmp(result, 'error/class', 'DeviceNotFound')
 
     def test_wrong_sync_mode(self):
+        if not self.has_quorum():
+            return
+
         result = self.vm.qmp('drive-mirror', device='quorum0',
                              node_name='repair0',
                              replaces='img1',
@@ -875,12 +911,18 @@  class TestRepairQuorum(ImageMirroringTestCase):
         self.assert_qmp(result, 'error/class', 'GenericError')
 
     def test_no_node_name(self):
+        if not self.has_quorum():
+            return
+
         result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
                              replaces='img1',
                              target=quorum_repair_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
     def test_unexistant_replaces(self):
+        if not self.has_quorum():
+            return
+
         result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
                              node_name='repair0',
                              replaces='img77',
@@ -888,6 +930,9 @@  class TestRepairQuorum(ImageMirroringTestCase):
         self.assert_qmp(result, 'error/class', 'GenericError')
 
     def test_after_a_quorum_snapshot(self):
+        if not self.has_quorum():
+            return
+
         result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1',
                              snapshot_file=quorum_snapshot_file,
                              snapshot_node_name="snap1");