All of lore.kernel.org
 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 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.