qemu-iotests: Make test 192 use QMP; convert it to Python
PATCH 1/1 qemu-iotests: Make test 192 use QMP; convert it to Python
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.
--- tests/qemu-iotests/192 | 78 ++++++++++++++++++---------------------------- tests/qemu-iotests/192.out | 14 ++++----- 2 files changed, 38 insertions(+), 54 deletions(-)
tests/qemu-iotests/192 +31 -47
@@ -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 [email protected] - -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 <[email protected]> +# [Converted to Python from the original Bash version by Fam +# Zheng (<[email protected])] +# +# 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'))
tests/qemu-iotests/192.out +7 -7
@@ -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': {}}
Signature · 1 line
2.9.5
Hi Kashyap,
Quoted 85 lines · 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 <[email protected]>--- tests/qemu-iotests/192 | 78 ++++++++++++++++++---------------------------- tests/qemu-iotests/192.out | 14 ++++----- 2 files changed, 38 insertions(+), 54 deletions(-)tests/qemu-iotests/192 +12
-47@@ -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 [email protected] - -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 <[email protected]> +# [Converted to Python from the original Bash version by Fam +# Zheng (<[email protected])] +# +# Purpose: Test NBD export with '-incoming' (non-shared storage +# migration use case from libvirt) + +import os +import atexit +import iotests +
tests/qemu-iotests/192 line 38
Quoted 10 lines
tests/qemu-iotests/192 +10
-0+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?
tests/qemu-iotests/192 line 40
Quoted 2 lines
tests/qemu-iotests/192 +2
-0+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
Quoted 29 lines
tests/qemu-iotests/192 +7
-0+ +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'))tests/qemu-iotests/192.out +7
-7@@ -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': {}}Signature · 1 line
2.9.5
Quoted 3 lines · Fam Zheng
Hi Kashyap,
On Mon, 09/04 13:16, Kashyap Chamarthy wrote:
[...]
Quoted 4 lines
tests/qemu-iotests/192 line 38
Quoted 2 lines
tests/qemu-iotests/192 +2
-0+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')
Quoted 6 lines
tests/qemu-iotests/192 line 40
Quoted 2 lines
tests/qemu-iotests/192 +2
-0+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
Quoted 1 line
Otherwise looks good to me.
Thanks for the quick review!
[...]
Signature · 1 line
/kashyap
Quoted 10 lines · Kashyap Chamarthy
Quoted 9 lines · Fam Zheng
Hi Kashyap,
Quoted 5 lines · 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".
Kevin actually argued that keeping some HMP coverage is a GOOD thing:
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg00798.html
Quoted 8 lines
Quoted 4 lines
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?
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Quoted 12 lines · Eric Blake
Quoted 10 lines · Kashyap Chamarthy
Quoted 9 lines · Fam Zheng
Hi Kashyap,
Quoted 5 lines · 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".
Quoted 3 lines
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.
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
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.
Quoted 5 lines
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.
Signature · 1 line
/kashyap
Quoted 7 lines · 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.
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
Signature · 3 lines
|: 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 :|
Quoted 7 lines · Daniel P. Berrange
Quoted 6 lines · 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".
Quoted 6 lines
Quoted 1 line
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.
Signature · 1 line
/kashyap