[v3,9/9] iotests: add nbd-reconnect-on-open test

Commit Message

Vladimir Sementsov-Ogievskiy Sept. 6, 2021, 7:06 p.m. UTC

Signed-off-by: Vladimir Sementsov-Ogievskiy <[e-mail]>
---
 .../qemu-iotests/tests/nbd-reconnect-on-open  | 71 +++++++++++++++++++
 .../tests/nbd-reconnect-on-open.out           | 11 +++
 2 files changed, 82 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/nbd-reconnect-on-open
 create mode 100644 tests/qemu-iotests/tests/nbd-reconnect-on-open.out

Comments

Eric Blake Sept. 7, 2021, 8:51 p.m. UTC | #1

On Mon, Sep 06, 2021 at 10:06:54PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[e-mail]>
> ---
>  .../qemu-iotests/tests/nbd-reconnect-on-open  | 71 +++++++++++++++++++
>  .../tests/nbd-reconnect-on-open.out           | 11 +++
>  2 files changed, 82 insertions(+)
>  create mode 100755 tests/qemu-iotests/tests/nbd-reconnect-on-open
>  create mode 100644 tests/qemu-iotests/tests/nbd-reconnect-on-open.out

I'm less confident in my review of the python code, but...

> 
> diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open b/tests/qemu-iotests/tests/nbd-reconnect-on-open
> new file mode 100755
> index 0000000000..7ee9bce947
> --- /dev/null
> +++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open
> @@ -0,0 +1,71 @@

> +
> +def create_args(open_timeout):
> +    return ['--image-opts', '-c', 'read 0 1M',
> +            f'driver=nbd,open-timeout={open_timeout},'
> +            f'server.type=unix,server.path={nbd_sock}']
> +
> +
> +def check_fail_to_connect(open_timeout):
> +    log(f'Check fail to connect with {open_timeout} seconds of timeout')
> +
> +    start_t = time.time()
> +    qemu_io_log(*create_args(open_timeout))
> +    delta_t = time.time() - start_t
> +
> +    max_delta = open_timeout + 0.2

Is this fractional delay going to bite us on heavily-loaded CI machines?

> +    if open_timeout <= delta_t <= max_delta:
> +        log(f'qemu_io finished in {open_timeout}..{max_delta} seconds, OK')
> +    else:
> +        note = 'too early' if delta_t < open_timeout else 'too long'
> +        log(f'qemu_io finished in {delta_t:.1f} seconds, {note}')
> +
> +
> +qemu_img_create('-f', iotests.imgfmt, disk, '1M')
> +
> +# Start NBD client when NBD server is not yet running. It should not fail, but
> +# wait for 5 seconds for the server to be available.
> +client = qemu_io_popen(*create_args(5))
> +
> +time.sleep(1)
> +qemu_nbd('-k', nbd_sock, '-f', iotests.imgfmt, disk)
> +
> +# client should succeed
> +log(client.communicate()[0], filters=[iotests.filter_qemu_io])
> +
> +# Server was started without --persistent flag, so it should be off now. Let's
> +# check it and it the same time check that with open-timeout=0 client fails

check it and at

> +# immediately.
> +check_fail_to_connect(0)
> +
> +# Check that we will fail after non-zero timeout if server is still unavailable
> +check_fail_to_connect(1)
> diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open.out b/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
> new file mode 100644
> index 0000000000..a35ae30ea4
> --- /dev/null
> +++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
> @@ -0,0 +1,11 @@
> +read 1048576/1048576 bytes at offset 0
> +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +Check fail to connect with 0 seconds of timeout
> +qemu-io: can't open: Failed to connect to 'TEST_DIR/PID-nbd-sock': No such file or directory
> +
> +qemu_io finished in 0..0.2 seconds, OK
> +Check fail to connect with 1 seconds of timeout
> +qemu-io: can't open: Failed to connect to 'TEST_DIR/PID-nbd-sock': No such file or directory
> +
> +qemu_io finished in 1..1.2 seconds, OK

Overall, the test looks like a nice demonstration of the feature: you
are showing that the client can now start before the server, and that
the retry for N seconds is handled gracefully both by the creation of
the server and by the expiration of the retry timeout.

Vladimir Sementsov-Ogievskiy Sept. 8, 2021, 7:55 a.m. UTC | #2

07.09.2021 23:51, Eric Blake wrote:
>> +def check_fail_to_connect(open_timeout):
>> +    log(f'Check fail to connect with {open_timeout} seconds of timeout')
>> +
>> +    start_t = time.time()
>> +    qemu_io_log(*create_args(open_timeout))
>> +    delta_t = time.time() - start_t
>> +
>> +    max_delta = open_timeout + 0.2
> Is this fractional delay going to bite us on heavily-loaded CI machines?
> 

You mean that it's too small and may be racy? Hmm. But increasing it now means wasting extra time.. I'd adjust it when CI fail if it happens.

Patch

diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open b/tests/qemu-iotests/tests/nbd-reconnect-on-open
new file mode 100755
index 0000000000..7ee9bce947
--- /dev/null
+++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open
@@ -0,0 +1,71 @@ 
+#!/usr/bin/env python3
+#
+# Test nbd reconnect on open
+#
+# Copyright (c) 2020 Virtuozzo International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import time
+
+import iotests
+from iotests import qemu_img_create, file_path, qemu_io_popen, qemu_nbd, \
+    qemu_io_log, log
+
+iotests.script_initialize(supported_fmts=['qcow2'])
+
+disk, nbd_sock = file_path('disk', 'nbd-sock')
+
+
+def create_args(open_timeout):
+    return ['--image-opts', '-c', 'read 0 1M',
+            f'driver=nbd,open-timeout={open_timeout},'
+            f'server.type=unix,server.path={nbd_sock}']
+
+
+def check_fail_to_connect(open_timeout):
+    log(f'Check fail to connect with {open_timeout} seconds of timeout')
+
+    start_t = time.time()
+    qemu_io_log(*create_args(open_timeout))
+    delta_t = time.time() - start_t
+
+    max_delta = open_timeout + 0.2
+    if open_timeout <= delta_t <= max_delta:
+        log(f'qemu_io finished in {open_timeout}..{max_delta} seconds, OK')
+    else:
+        note = 'too early' if delta_t < open_timeout else 'too long'
+        log(f'qemu_io finished in {delta_t:.1f} seconds, {note}')
+
+
+qemu_img_create('-f', iotests.imgfmt, disk, '1M')
+
+# Start NBD client when NBD server is not yet running. It should not fail, but
+# wait for 5 seconds for the server to be available.
+client = qemu_io_popen(*create_args(5))
+
+time.sleep(1)
+qemu_nbd('-k', nbd_sock, '-f', iotests.imgfmt, disk)
+
+# client should succeed
+log(client.communicate()[0], filters=[iotests.filter_qemu_io])
+
+# Server was started without --persistent flag, so it should be off now. Let's
+# check it and it the same time check that with open-timeout=0 client fails
+# immediately.
+check_fail_to_connect(0)
+
+# Check that we will fail after non-zero timeout if server is still unavailable
+check_fail_to_connect(1)
diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open.out b/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
new file mode 100644
index 0000000000..a35ae30ea4
--- /dev/null
+++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
@@ -0,0 +1,11 @@ 
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Check fail to connect with 0 seconds of timeout
+qemu-io: can't open: Failed to connect to 'TEST_DIR/PID-nbd-sock': No such file or directory
+
+qemu_io finished in 0..0.2 seconds, OK
+Check fail to connect with 1 seconds of timeout
+qemu-io: can't open: Failed to connect to 'TEST_DIR/PID-nbd-sock': No such file or directory
+
+qemu_io finished in 1..1.2 seconds, OK