[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 6/6] iotest 201: new test for qmp nbd-server-
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 6/6] iotest 201: new test for qmp nbd-server-remove |
Date: |
Tue, 9 Jan 2018 14:49:50 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 |
On 12/07/2017 09:51 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
> tests/qemu-iotests/201 | 130
> +++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/201.out | 5 ++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 136 insertions(+)
> create mode 100644 tests/qemu-iotests/201
> create mode 100644 tests/qemu-iotests/201.out
>
pep8 complains:
$ pep8 tests/qemu-iotests/201
tests/qemu-iotests/201:31:1: E302 expected 2 blank lines, found 1
tests/qemu-iotests/201:68:17: E128 continuation line under-indented for
visual indent
tests/qemu-iotests/201:69:17: E128 continuation line under-indented for
visual indent
I don't mind cleaning those up (if you don't have to respin because of
my comments on 5/6).
> +import os
> +import sys
> +import iotests
> +import time
> +from iotests import qemu_img, qemu_io, filter_qemu_io, QemuIoInteractive
> +
> +nbd_port = '10900'
> +nbd_uri = 'nbd+tcp://localhost:' + nbd_port + '/exp'
Is this still going to be safe when we improve the testsuite to run
multiple tests in parallel? Wouldn't it be safer to dynamically choose
the port, instead of hard-coding one?
> +disk = os.path.join(iotests.test_dir, 'disk')
> +
> +class TestNbdServerRemove(iotests.QMPTestCase):
> + def setUp(self):
> + qemu_img('create', '-f', iotests.imgfmt, disk, '1M')
> +
> + self.vm = iotests.VM().add_drive(disk)
> + self.vm.launch()
> +
> + address = {
> + 'type': 'inet',
> + 'data': {
> + 'host': 'localhost',
> + 'port': nbd_port
Again, for a one-shot test, this works; but it doesn't lend itself to
parallel testing. Maybe do a loop with incrementing port numbers until
one succeeds, if we can reliably detect when a port is already in use?
> + def assertConnectFailed(self, qemu_io_output):
> + self.assertEqual(filter_qemu_io(qemu_io_output).strip(),
> + "can't open device nbd+tcp://localhost:10900/exp: "
> +
Worth parameterizing or filtering this assertion to match the rest of
the parameterization of nbd_port?
> + "Requested export not available\nserver reported: "
> +
> + "export 'exp' not present")
> +
> + def do_test_connect_after_remove(self, force=None):
> + args = ('-r', '-f', 'raw', '-c', 'read 0 512', nbd_uri)
> + self.assertReadOk(qemu_io(*args))
> + self.remove_export('exp', force)
> + self.assertExportNotFound('exp')
> + self.assertConnectFailed(qemu_io(*args))
> +
> + def test_connect_after_remove_default(self):
> + self.do_test_connect_after_remove()
> +
> + def test_connect_after_remove_safe(self):
> + self.do_test_connect_after_remove(False)
> +
> + def test_connect_after_remove_force(self):
> + self.do_test_connect_after_remove(True)
May need updates if my comments on 3/6 result in us having three states
rather than just 2 (the difference being whether there is a knob for
choosing to let existing clients gracefully disconnect with all pending
I/O completed, and/or failing the nbd-server-remove if a client is still
connected).
> +++ b/tests/qemu-iotests/201.out
> @@ -0,0 +1,5 @@
> +.......
> +----------------------------------------------------------------------
> +Ran 7 tests
I'm not a fan of python tests that are difficult to debug. Your
additions to 147 in patch 4/6 are okay (hard to debug, but an
incremental addition); but is it possible to rewrite this test in a bit
more verbose manner? See test 194 and this message for more details:
https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg00234.html
> +
> +OK
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 3e688678dd..15df7678b3 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -197,3 +197,4 @@
> 197 rw auto quick
> 198 rw auto
> 200 rw auto
> +201 rw auto quick
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v2 6/6] iotest 201: new test for qmp nbd-server-remove,
Eric Blake <=