qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: Kashyap Chamarthy <kchamart@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org,
	stefanha@redhat.com, eblake@redhat.com, kwolf@redhat.com,
	dgilbert@redhat.com
Subject: Re: [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python
Date: Mon, 4 Sep 2017 20:55:23 +0800	[thread overview]
Message-ID: <20170904125523.GC19748@lemon.lan> (raw)
In-Reply-To: <20170904111616.21333-1-kchamart@redhat.com>

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
> 

  reply	other threads:[~2017-09-04 12:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170904125523.GC19748@lemon.lan \
    --to=famz@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=kchamart@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).