* [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python
@ 2017-09-04 11:16 Kashyap Chamarthy
2017-09-04 12:55 ` Fam Zheng
2017-09-04 14:48 ` Daniel P. Berrange
0 siblings, 2 replies; 7+ messages in thread
From: Kashyap Chamarthy @ 2017-09-04 11:16 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, stefanha, famz, eblake, kwolf, dgilbert, Kashyap Chamarthy
The test 192 ("Test NBD export with '-incoming' (non-shared
storage migration use case from libvirt")) is currently using HMP.
Replace the HMP usage with QMP, as the upstream preference seems to be:
"Use QMP where possible, unless you're explicitly testing something
related to HMP".
While at it, convert the test from Bash to Python.
Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
---
tests/qemu-iotests/192 | 78 ++++++++++++++++++----------------------------
tests/qemu-iotests/192.out | 14 ++++-----
2 files changed, 38 insertions(+), 54 deletions(-)
diff --git a/tests/qemu-iotests/192 b/tests/qemu-iotests/192
index b50a2c0c8e2fccdddfae4ac58ca35937c5f784c6..692475f0a8c80c81396587d3289d4b32c0ee0d21 100755
--- a/tests/qemu-iotests/192
+++ b/tests/qemu-iotests/192
@@ -1,7 +1,4 @@
-#!/bin/bash
-#
-# Test NBD export with -incoming (non-shared storage migration use case from
-# libvirt)
+#!/usr/bin/env python
#
# Copyright (C) 2017 Red Hat, Inc.
#
@@ -18,46 +15,33 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
-
-# creator
-owner=famz@redhat.com
-
-seq=`basename $0`
-echo "QA output created by $seq"
-
-here=`pwd`
-status=1 # failure is the default!
-
-_cleanup()
-{
- _cleanup_test_img
-}
-trap "_cleanup; exit \$status" 0 1 2 3 15
-
-# get standard environment, filters and checks
-. ./common.rc
-. ./common.filter
-
-_supported_fmt generic
-_supported_proto file
-_supported_os Linux
-
-if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
- _notrun "Requires a PC machine"
-fi
-
-size=64M
-_make_test_img $size
-
-{
-echo "nbd_server_start unix:$TEST_DIR/nbd"
-echo "nbd_server_add -w drive0"
-echo "q"
-} | $QEMU -nodefaults -display none -monitor stdio \
- -drive format=$IMGFMT,file=$TEST_IMG,if=ide,id=drive0 \
- -incoming defer 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp
-
-# success, all done
-echo "*** done"
-rm -f $seq.full
-status=0
+# Author: Kashyap Chamarthy <kchamart@redhat.com>
+# [Converted to Python from the original Bash version by Fam
+# Zheng (<famz@redhat.com)]
+#
+# Purpose: Test NBD export with '-incoming' (non-shared storage
+# migration use case from libvirt)
+
+import os
+import atexit
+import iotests
+
+iotests.verify_platform(['linux'])
+
+img_size = '1G'
+test_img_path = os.path.join(iotests.test_dir, 'dest.img')
+iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, test_img_path, img_size)
+
+iotests.log('Launching VM...')
+nbd_sock_path = os.path.join(iotests.test_dir, 'nbd.sock')
+dest_vm = (iotests.VM('dest').add_drive(test_img_path)
+ .add_incoming('defer'))
+dest_vm.launch()
+atexit.register(dest_vm.shutdown)
+
+iotests.log('Launching NBD server on destination...')
+iotests.log(dest_vm.qmp('nbd-server-start', addr={'type': 'unix', 'data': {'path': nbd_sock_path}}))
+iotests.log('Exporting the block device to NBD server...')
+iotests.log(dest_vm.qmp('nbd-server-add', device='drive0', writable=True))
+iotests.log('Stopping the NBD server on destination...')
+iotests.log(dest_vm.qmp('nbd-server-stop'))
diff --git a/tests/qemu-iotests/192.out b/tests/qemu-iotests/192.out
index 1e0be4c4d7a891f81c2536b87c5a29a21d39138c..f62b94c5641f1a1e60f17090aeb20ed0bc037afd 100644
--- a/tests/qemu-iotests/192.out
+++ b/tests/qemu-iotests/192.out
@@ -1,7 +1,7 @@
-QA output created by 192
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) nbd_server_start unix:TEST_DIR/nbd
-(qemu) nbd_server_add -w drive0
-(qemu) q
-*** done
+Launching VM...
+Launching NBD server on destination...
+{u'return': {}}
+Exporting the block device to NBD server...
+{u'return': {}}
+Stopping the NBD server on destination...
+{u'return': {}}
--
2.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python
2017-09-04 11:16 [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python Kashyap Chamarthy
@ 2017-09-04 12:55 ` Fam Zheng
2017-09-04 13:28 ` Kashyap Chamarthy
2017-09-04 14:48 ` Daniel P. Berrange
1 sibling, 1 reply; 7+ messages in thread
From: Fam Zheng @ 2017-09-04 12:55 UTC (permalink / raw)
To: Kashyap Chamarthy
Cc: qemu-devel, qemu-block, stefanha, eblake, kwolf, dgilbert
Hi Kashyap,
On Mon, 09/04 13:16, Kashyap Chamarthy wrote:
> The test 192 ("Test NBD export with '-incoming' (non-shared
> storage migration use case from libvirt")) is currently using HMP.
> Replace the HMP usage with QMP, as the upstream preference seems to be:
> "Use QMP where possible, unless you're explicitly testing something
> related to HMP".
>
> While at it, convert the test from Bash to Python.
>
> Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> ---
> tests/qemu-iotests/192 | 78 ++++++++++++++++++----------------------------
> tests/qemu-iotests/192.out | 14 ++++-----
> 2 files changed, 38 insertions(+), 54 deletions(-)
>
> diff --git a/tests/qemu-iotests/192 b/tests/qemu-iotests/192
> index b50a2c0c8e2fccdddfae4ac58ca35937c5f784c6..692475f0a8c80c81396587d3289d4b32c0ee0d21 100755
> --- a/tests/qemu-iotests/192
> +++ b/tests/qemu-iotests/192
> @@ -1,7 +1,4 @@
> -#!/bin/bash
> -#
> -# Test NBD export with -incoming (non-shared storage migration use case from
> -# libvirt)
> +#!/usr/bin/env python
> #
> # Copyright (C) 2017 Red Hat, Inc.
> #
> @@ -18,46 +15,33 @@
> # You should have received a copy of the GNU General Public License
> # along with this program. If not, see <http://www.gnu.org/licenses/>.
> #
> -
> -# creator
> -owner=famz@redhat.com
> -
> -seq=`basename $0`
> -echo "QA output created by $seq"
> -
> -here=`pwd`
> -status=1 # failure is the default!
> -
> -_cleanup()
> -{
> - _cleanup_test_img
> -}
> -trap "_cleanup; exit \$status" 0 1 2 3 15
> -
> -# get standard environment, filters and checks
> -. ./common.rc
> -. ./common.filter
> -
> -_supported_fmt generic
> -_supported_proto file
> -_supported_os Linux
> -
> -if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
> - _notrun "Requires a PC machine"
> -fi
> -
> -size=64M
> -_make_test_img $size
> -
> -{
> -echo "nbd_server_start unix:$TEST_DIR/nbd"
> -echo "nbd_server_add -w drive0"
> -echo "q"
> -} | $QEMU -nodefaults -display none -monitor stdio \
> - -drive format=$IMGFMT,file=$TEST_IMG,if=ide,id=drive0 \
> - -incoming defer 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp
> -
> -# success, all done
> -echo "*** done"
> -rm -f $seq.full
> -status=0
> +# Author: Kashyap Chamarthy <kchamart@redhat.com>
> +# [Converted to Python from the original Bash version by Fam
> +# Zheng (<famz@redhat.com)]
> +#
> +# Purpose: Test NBD export with '-incoming' (non-shared storage
> +# migration use case from libvirt)
> +
> +import os
> +import atexit
> +import iotests
> +
> +iotests.verify_platform(['linux'])
> +
> +img_size = '1G'
> +test_img_path = os.path.join(iotests.test_dir, 'dest.img')
> +iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, test_img_path, img_size)
> +
> +iotests.log('Launching VM...')
> +nbd_sock_path = os.path.join(iotests.test_dir, 'nbd.sock')
> +dest_vm = (iotests.VM('dest').add_drive(test_img_path)
> + .add_incoming('defer'))
Superfluous parenthesis?
> +dest_vm.launch()
> +atexit.register(dest_vm.shutdown)
As an improvement maybe you could rebase to Stefan's "iotests: clean up
resources using context managers" series and switch to "with" for the temp file
and VM.
Otherwise looks good to me.
Fam
> +
> +iotests.log('Launching NBD server on destination...')
> +iotests.log(dest_vm.qmp('nbd-server-start', addr={'type': 'unix', 'data': {'path': nbd_sock_path}}))
> +iotests.log('Exporting the block device to NBD server...')
> +iotests.log(dest_vm.qmp('nbd-server-add', device='drive0', writable=True))
> +iotests.log('Stopping the NBD server on destination...')
> +iotests.log(dest_vm.qmp('nbd-server-stop'))
> diff --git a/tests/qemu-iotests/192.out b/tests/qemu-iotests/192.out
> index 1e0be4c4d7a891f81c2536b87c5a29a21d39138c..f62b94c5641f1a1e60f17090aeb20ed0bc037afd 100644
> --- a/tests/qemu-iotests/192.out
> +++ b/tests/qemu-iotests/192.out
> @@ -1,7 +1,7 @@
> -QA output created by 192
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
> -QEMU X.Y.Z monitor - type 'help' for more information
> -(qemu) nbd_server_start unix:TEST_DIR/nbd
> -(qemu) nbd_server_add -w drive0
> -(qemu) q
> -*** done
> +Launching VM...
> +Launching NBD server on destination...
> +{u'return': {}}
> +Exporting the block device to NBD server...
> +{u'return': {}}
> +Stopping the NBD server on destination...
> +{u'return': {}}
> --
> 2.9.5
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python
2017-09-04 12:55 ` Fam Zheng
@ 2017-09-04 13:28 ` Kashyap Chamarthy
2017-09-05 19:37 ` Eric Blake
0 siblings, 1 reply; 7+ messages in thread
From: Kashyap Chamarthy @ 2017-09-04 13:28 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, qemu-block, stefanha, eblake, kwolf, dgilbert
On Mon, Sep 04, 2017 at 08:55:23PM +0800, Fam Zheng wrote:
> Hi Kashyap,
>
> On Mon, 09/04 13:16, Kashyap Chamarthy wrote:
[...]
> > +dest_vm = (iotests.VM('dest').add_drive(test_img_path)
> > + .add_incoming('defer'))
>
> Superfluous parenthesis?
Yes, the paranthesis can be removed and turn it into a single line:
dest_vm = iotests.VM('dest').add_drive(test_img_path).add_incoming('defer')
> > +dest_vm.launch()
> > +atexit.register(dest_vm.shutdown)
>
> As an improvement maybe you could rebase to Stefan's "iotests: clean up
> resources using context managers" series and switch to "with" for the temp file
> and VM.
Good point. I did notice that thread[*] about resource clean up. And I
see it's already applied to 'block-next'. Will rebase and change to the
'with' statement.
[*] https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg04639.html
> Otherwise looks good to me.
Thanks for the quick review!
[...]
--
/kashyap
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python
2017-09-04 11:16 [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python Kashyap Chamarthy
2017-09-04 12:55 ` Fam Zheng
@ 2017-09-04 14:48 ` Daniel P. Berrange
2017-09-04 15:05 ` Kashyap Chamarthy
1 sibling, 1 reply; 7+ messages in thread
From: Daniel P. Berrange @ 2017-09-04 14:48 UTC (permalink / raw)
To: Kashyap Chamarthy; +Cc: qemu-devel, kwolf, famz, qemu-block, dgilbert, stefanha
On Mon, Sep 04, 2017 at 01:16:16PM +0200, Kashyap Chamarthy wrote:
> The test 192 ("Test NBD export with '-incoming' (non-shared
> storage migration use case from libvirt")) is currently using HMP.
> Replace the HMP usage with QMP, as the upstream preference seems to be:
> "Use QMP where possible, unless you're explicitly testing something
> related to HMP".
>
> While at it, convert the test from Bash to Python.
FYI This looks like it'll clash with the patch I sent to fix this
test case with LUKS
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg00041.html
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python
2017-09-04 14:48 ` Daniel P. Berrange
@ 2017-09-04 15:05 ` Kashyap Chamarthy
0 siblings, 0 replies; 7+ messages in thread
From: Kashyap Chamarthy @ 2017-09-04 15:05 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, kwolf, famz, qemu-block, dgilbert, stefanha
On Mon, Sep 04, 2017 at 03:48:12PM +0100, Daniel P. Berrange wrote:
> On Mon, Sep 04, 2017 at 01:16:16PM +0200, Kashyap Chamarthy wrote:
> > The test 192 ("Test NBD export with '-incoming' (non-shared
> > storage migration use case from libvirt")) is currently using HMP.
> > Replace the HMP usage with QMP, as the upstream preference seems to be:
> > "Use QMP where possible, unless you're explicitly testing something
> > related to HMP".
> >
> > While at it, convert the test from Bash to Python.
>
> FYI This looks like it'll clash with the patch I sent to fix this
> test case with LUKS
>
> https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg00041.html
Oh, thanks for the pointer. Then, I think I should wait until yours is
merged, and then rework my changes on top of it.
--
/kashyap
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python
2017-09-04 13:28 ` Kashyap Chamarthy
@ 2017-09-05 19:37 ` Eric Blake
2017-09-08 10:06 ` Kashyap Chamarthy
0 siblings, 1 reply; 7+ messages in thread
From: Eric Blake @ 2017-09-05 19:37 UTC (permalink / raw)
To: Kashyap Chamarthy, Fam Zheng
Cc: qemu-devel, qemu-block, stefanha, kwolf, dgilbert
[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]
On 09/04/2017 08:28 AM, Kashyap Chamarthy wrote:
> On Mon, Sep 04, 2017 at 08:55:23PM +0800, Fam Zheng wrote:
>> Hi Kashyap,
>>
>> On Mon, 09/04 13:16, Kashyap Chamarthy wrote:
>>> The test 192 ("Test NBD export with '-incoming' (non-shared
>>> storage migration use case from libvirt")) is currently using HMP.
>>> Replace the HMP usage with QMP, as the upstream preference seems to be:
>>> "Use QMP where possible, unless you're explicitly testing something
>>> related to HMP".
Kevin actually argued that keeping some HMP coverage is a GOOD thing:
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg00798.html
>>
>> As an improvement maybe you could rebase to Stefan's "iotests: clean up
>> resources using context managers" series and switch to "with" for the temp file
>> and VM.
>
> Good point. I did notice that thread[*] about resource clean up. And I
> see it's already applied to 'block-next'. Will rebase and change to the
> 'with' statement.
I'm not sure if this patch helps us any; I personally find the unit-test
style python iotests harder to debug than the ones that produce
diff-able nnn.out files (debugging an nnn.out file that has only a list
of ..... doesn't make it easy to reproduce). If the test already runs
well in shell, what does the conversion to Python actually buy us?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python
2017-09-05 19:37 ` Eric Blake
@ 2017-09-08 10:06 ` Kashyap Chamarthy
0 siblings, 0 replies; 7+ messages in thread
From: Kashyap Chamarthy @ 2017-09-08 10:06 UTC (permalink / raw)
To: Eric Blake; +Cc: Fam Zheng, qemu-devel, qemu-block, stefanha, kwolf, dgilbert
On Tue, Sep 05, 2017 at 02:37:21PM -0500, Eric Blake wrote:
> On 09/04/2017 08:28 AM, Kashyap Chamarthy wrote:
> > On Mon, Sep 04, 2017 at 08:55:23PM +0800, Fam Zheng wrote:
> >> Hi Kashyap,
> >>
> >> On Mon, 09/04 13:16, Kashyap Chamarthy wrote:
>
> >>> The test 192 ("Test NBD export with '-incoming' (non-shared
> >>> storage migration use case from libvirt")) is currently using HMP.
> >>> Replace the HMP usage with QMP, as the upstream preference seems to be:
> >>> "Use QMP where possible, unless you're explicitly testing something
> >>> related to HMP".
>
> Kevin actually argued that keeping some HMP coverage is a GOOD thing:
>
> https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg00798.html
Yeah, noticed it after I sent my patch. I agree with Kevin's argument,
so I'll just simply drop my patch.
> >> As an improvement maybe you could rebase to Stefan's "iotests: clean up
> >> resources using context managers" series and switch to "with" for the temp file
> >> and VM.
> >
> > Good point. I did notice that thread[*] about resource clean up. And I
> > see it's already applied to 'block-next'. Will rebase and change to the
> > 'with' statement.
>
> I'm not sure if this patch helps us any; I personally find the unit-test
> style python iotests harder to debug than the ones that produce
> diff-able nnn.out files (debugging an nnn.out file that has only a list
> of ..... doesn't make it easy to reproduce). If the test already runs
> well in shell, what does the conversion to Python actually buy us?
While turning it to QMP, thought I'd convert it to Python as the test is
_very_ similar to 194 (also in Python).
Given Kevin's argument in the above thread you pointed out, just
disregard my patch.
--
/kashyap
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-09-08 10:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-04 11:16 [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python Kashyap Chamarthy
2017-09-04 12:55 ` Fam Zheng
2017-09-04 13:28 ` Kashyap Chamarthy
2017-09-05 19:37 ` Eric Blake
2017-09-08 10:06 ` Kashyap Chamarthy
2017-09-04 14:48 ` Daniel P. Berrange
2017-09-04 15:05 ` Kashyap Chamarthy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.