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