NBD reconnect on open
Hi Eric! There is a question to discuss. We need to make an option to allow nbd-reconnect loop on nbd-open. For example, add optional nbd blockdev option open-reconnect-delay, to make it possible to start qemu with specified nbd connection, when nbd server is down, and make several tries to connect before starting the guest. So, we need it for nbd opened from commandline arguments, and this case seems OK. But adding option to QAPI, we also allow it for qmp blockdev-add, and reconnecting in context of qmp command execution is a wrong thing.. I can add new option only to options in block/nbd.c, but this way -blockdev command line option will not work, it needs QAPI definition. What do you think about it? I can detect somehow in nbd_open that we are in qmp monitor context, and return error if open-reconnect-delay specified.. Is it OK? Is there a way to do it? -- Best regards, Vladimir
Re: NBD reconnect on open
Am 04.12.2019 um 13:18 hat Vladimir Sementsov-Ogievskiy geschrieben: > There is a question to discuss. > > We need to make an option to allow nbd-reconnect loop on nbd-open. > For example, add optional nbd blockdev option open-reconnect-delay, to > make it possible to start qemu with specified nbd connection, when nbd > server is down, and make several tries to connect before starting the > guest. > > So, we need it for nbd opened from commandline arguments, and this case > seems OK. > > But adding option to QAPI, we also allow it for qmp blockdev-add, and > reconnecting in context of qmp command execution is a wrong thing.. > > I can add new option only to options in block/nbd.c, but this way > -blockdev command line option will not work, it needs QAPI definition. > > What do you think about it? I think there is a more general problem here actually. bdrv_open() is a blocking operation and it shouldn't be. BlockDriver should probably have a .bdrv_co_open instead, and I think this wouldn't be too hard to do. However, that's only half of the solution: QMP still takes the BQL while it's executing a command, so even if we used a coroutine, it would be of no use if then the QMP command implementation would just call BDRV_POLL_WHILE() to wait for the completion of the coroutine while holding the BQL. This is going in direction of async commands that Marc-André was working on. I didn't follow this closely, so I'm not sure what the status there is, but he and Markus should be able to tell more. > I can detect somehow in nbd_open that we are in qmp monitor context, and > return error if open-reconnect-delay specified.. Is it OK? Is there a > way to do it? The whole point of -blockdev is that it's a direct mapping of blockdev-add to the command line, so making things behave differently between them sounds like a bad idea. Kevin
[PATCH 0/3] nbd reconnect on open
Hi all! There is a new feature: reconnect on open. It is useful when start of vm and start of nbd server are not simple to sync. This is based on "[PATCH 0/4] nbd reconnect new fixes" Based-on: <20200903190301.367620-1-vsement...@virtuozzo.com> Vladimir Sementsov-Ogievskiy (3): block/nbd: move initial connect to coroutine nbd: allow reconnect on open, with corresponding new options iotests: add 306 to test reconnect on nbd open block/nbd.c | 173 +- tests/qemu-iotests/306| 46 + tests/qemu-iotests/306.out| 5 + tests/qemu-iotests/group | 1 + tests/qemu-iotests/iotests.py | 11 +++ 5 files changed, 190 insertions(+), 46 deletions(-) create mode 100755 tests/qemu-iotests/306 create mode 100644 tests/qemu-iotests/306.out -- 2.21.3
[PATCH v4 0/7] nbd reconnect on open
Hi all!
The functionality is reviewed, python testing part is not.
I've dropped the patch "qapi: make blockdev-add a coroutine command":
it's optional, I don't want to slow down the whole series because of it.
v4:
01-03: wording, add Eric's r-b
others: small changes, never had an r-b
Vladimir Sementsov-Ogievskiy (7):
nbd: allow reconnect on open, with corresponding new options
nbd/client-connection: nbd_co_establish_connection(): return real
error
nbd/client-connection: improve error message of cancelled attempt
iotests.py: add qemu_tool_popen()
For qemu_io* functions support --image-opts argument, which conflicts
with -f argument from qemu_io_args.
Add qemu-io Popen constructor wrapper. To be used in the following new
test commit.
iotests: add nbd-reconnect-on-open test
qapi/block-core.json | 9 ++-
block/nbd.c | 45 +++-
nbd/client-connection.c | 59 ++-
tests/qemu-iotests/iotests.py | 36 ++----
.../qemu-iotests/tests/nbd-reconnect-on-open | 71 +++
.../tests/nbd-reconnect-on-open.out | 11 +++
6 files changed, 199 insertions(+), 32 deletions(-)
create mode 100755 tests/qemu-iotests/tests/nbd-reconnect-on-open
create mode 100644 tests/qemu-iotests/tests/nbd-reconnect-on-open.out
--
2.31.1
[PATCH v3 0/9] nbd reconnect on open
Hi all! After a long delay here is v3. v3 is rebased on top of big refactoring of nbd connection code, and on top of last portion of it, not yet merged: Based-on: <20210902103805.25686-1-vsement...@virtuozzo.com> "[PATCH v6 0/5] block/nbd: drop connection_co" So, the core patch (02) is changed a lot. QAPI interface added. Vladimir Sementsov-Ogievskiy (9): nbd/client-connection: nbd_co_establish_connection(): fix non set errp qapi: make blockdev-add a coroutine command nbd: allow reconnect on open, with corresponding new options nbd/client-connection: nbd_co_establish_connection(): return real error nbd/client-connection: improve error message of cancelled attempt iotests.py: add qemu_tool_popen() iotests.py: add and use qemu_io_wrap_args() iotests.py: add qemu_io_popen() iotests: add nbd-reconnect-on-open test qapi/block-core.json | 12 +++- block/nbd.c | 45 +++- nbd/client-connection.c | 56 +++ tests/qemu-iotests/iotests.py | 39 ++ .../qemu-iotests/tests/nbd-reconnect-on-open | 71 +++ .../tests/nbd-reconnect-on-open.out | 11 +++ 6 files changed, 203 insertions(+), 31 deletions(-) create mode 100755 tests/qemu-iotests/tests/nbd-reconnect-on-open create mode 100644 tests/qemu-iotests/tests/nbd-reconnect-on-open.out -- 2.29.2
Re: [PATCH v4 0/7] nbd reconnect on open
13.12.2021 18:32, Vladimir Sementsov-Ogievskiy wrote: Hi all! The functionality is reviewed, python testing part is not. I've dropped the patch "qapi: make blockdev-add a coroutine command": it's optional, I don't want to slow down the whole series because of it. v4: 01-03: wording, add Eric's r-b others: small changes, never had an r-b Vladimir Sementsov-Ogievskiy (7): nbd: allow reconnect on open, with corresponding new options nbd/client-connection: nbd_co_establish_connection(): return real error nbd/client-connection: improve error message of cancelled attempt iotests.py: add qemu_tool_popen() For qemu_io* functions support --image-opts argument, which conflicts with -f argument from qemu_io_args. Add qemu-io Popen constructor wrapper. To be used in the following new test commit. iotests: add nbd-reconnect-on-open test qapi/block-core.json | 9 ++- block/nbd.c | 45 +++- nbd/client-connection.c | 59 ++- tests/qemu-iotests/iotests.py | 36 ++---- .../qemu-iotests/tests/nbd-reconnect-on-open | 71 +++ .../tests/nbd-reconnect-on-open.out | 11 +++ 6 files changed, 199 insertions(+), 32 deletions(-) create mode 100755 tests/qemu-iotests/tests/nbd-reconnect-on-open create mode 100644 tests/qemu-iotests/tests/nbd-reconnect-on-open.out Thanks for reviewing! I do s/6.2/7.0/ fix to patch 1, restore subjects of patches 5,6 (which were somehow lost in transition v3->v4) and apply the series to my nbd branch. -- Best regards, Vladimir
[PATCH v2 for-6.0 0/8] nbd reconnect on open
Hi all! There is a new feature: reconnect on open. It is useful when start of vm and start of nbd server are not simple to sync. v2: rebase on master. Also, I've discovered that I've sent downstream version of test which doesn't work here. So, the test is rewritten (with appropriate improvements of iotests.py) Vladimir Sementsov-Ogievskiy (8): block/nbd: move initial connect to coroutine nbd: allow reconnect on open, with corresponding new options iotests.py: fix qemu_tool_pipe_and_status() iotests.py: qemu_io(): reuse qemu_tool_pipe_and_status() iotests.py: add qemu_tool_popen() iotests.py: add and use qemu_io_wrap_args() iotests.py: add qemu_io_popen() iotests: add 306 to test reconnect on nbd open block/nbd.c | 173 +- tests/qemu-iotests/306| 71 ++ tests/qemu-iotests/306.out| 11 +++ tests/qemu-iotests/group | 1 + tests/qemu-iotests/iotests.py | 56 +++ 5 files changed, 246 insertions(+), 66 deletions(-) create mode 100755 tests/qemu-iotests/306 create mode 100644 tests/qemu-iotests/306.out -- 2.21.3
[PATCH] iotests/nbd-reconnect-on-open: Fix NBD socket path
Socket paths need to be short to avoid failures. This is why there is a
iotests.sock_dir (defaulting to /tmp) separate from the disk image base
directory.
Make use of it to fix failures in too deeply nested test directories.
Fixes: ab7f7e67a7e7b49964109501dfcde4ec29bae60e
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/tests/nbd-reconnect-on-open | 3 ++-
tests/qemu-iotests/tests/nbd-reconnect-on-open.out | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open
b/tests/qemu-iotests/tests/nbd-reconnect-on-open
index d0b401b060..3ce52021c3 100755
--- a/tests/qemu-iotests/tests/nbd-reconnect-on-open
+++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open
@@ -26,7 +26,8 @@ from iotests import qemu_img_create, file_path,
qemu_io_popen, qemu_nbd, \
iotests.script_initialize(supported_fmts=['qcow2'])
-disk, nbd_sock = file_path('disk', 'nbd-sock')
+disk = file_path('disk')
+nbd_sock = file_path('nbd-sock', base_dir=iotests.sock_dir)
def create_args(open_timeout):
diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
b/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
index a35ae30ea4..b3dd90f2a3 100644
--- a/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
+++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
@@ -2,10 +2,10 @@ 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: can't open: Failed to connect to 'SOCK_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: can't open: Failed to connect to 'SOCK_DIR/PID-nbd-sock': No such
file or directory
qemu_io finished in 1..1.2 seconds, OK
--
2.40.1
[PULL 7/7] iotests: add nbd-reconnect-on-open test
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Nikita Lapshin
---
.../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
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 00..8be721a24f
--- /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 at 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 00..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
--
2.31.1
[PATCH v4 7/7] iotests: add nbd-reconnect-on-open test
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
.../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
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 00..8be721a24f
--- /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 at 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 00..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
--
2.31.1
[PATCH v3 9/9] iotests: add nbd-reconnect-on-open test
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
.../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
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 00..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 00..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
--
2.29.2
Re: [PATCH] iotests/nbd-reconnect-on-open: Fix NBD socket path
On Wed, May 03, 2023 at 06:50:19PM +0200, Kevin Wolf wrote: > Socket paths need to be short to avoid failures. This is why there is a > iotests.sock_dir (defaulting to /tmp) separate from the disk image base > directory. > > Make use of it to fix failures in too deeply nested test directories. > > Fixes: ab7f7e67a7e7b49964109501dfcde4ec29bae60e > Signed-off-by: Kevin Wolf > --- > tests/qemu-iotests/tests/nbd-reconnect-on-open | 3 ++- > tests/qemu-iotests/tests/nbd-reconnect-on-open.out | 4 ++-- > 2 files changed, 4 insertions(+), 3 deletions(-) Reviewed-by: Eric Blake Just barely missed today's NBD pull request; I'll queue it for my next one, or it can go in through your block tree if you beat me to it. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH] iotests/nbd-reconnect-on-open: Fix NBD socket path
On 03.05.23 19:50, Kevin Wolf wrote: Socket paths need to be short to avoid failures. This is why there is a iotests.sock_dir (defaulting to /tmp) separate from the disk image base directory. Make use of it to fix failures in too deeply nested test directories. Fixes: ab7f7e67a7e7b49964109501dfcde4ec29bae60e Signed-off-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [PATCH v2 for-6.0 0/8] nbd reconnect on open
ping :) 30.11.2020 16:40, Vladimir Sementsov-Ogievskiy wrote: Hi all! There is a new feature: reconnect on open. It is useful when start of vm and start of nbd server are not simple to sync. v2: rebase on master. Also, I've discovered that I've sent downstream version of test which doesn't work here. So, the test is rewritten (with appropriate improvements of iotests.py) Vladimir Sementsov-Ogievskiy (8): block/nbd: move initial connect to coroutine nbd: allow reconnect on open, with corresponding new options iotests.py: fix qemu_tool_pipe_and_status() iotests.py: qemu_io(): reuse qemu_tool_pipe_and_status() iotests.py: add qemu_tool_popen() iotests.py: add and use qemu_io_wrap_args() iotests.py: add qemu_io_popen() iotests: add 306 to test reconnect on nbd open block/nbd.c | 173 +- tests/qemu-iotests/306| 71 ++ tests/qemu-iotests/306.out| 11 +++ tests/qemu-iotests/group | 1 + tests/qemu-iotests/iotests.py | 56 +++ 5 files changed, 246 insertions(+), 66 deletions(-) create mode 100755 tests/qemu-iotests/306 create mode 100644 tests/qemu-iotests/306.out -- Best regards, Vladimir
Re: [PATCH v2 for-6.0 0/8] nbd reconnect on open
On 11/30/20 7:40 AM, Vladimir Sementsov-Ogievskiy wrote: > Hi all! There is a new feature: reconnect on open. It is useful when > start of vm and start of nbd server are not simple to sync. > > v2: rebase on master. > Also, I've discovered that I've sent downstream version of test which > doesn't work here. So, the test is rewritten (with appropriate > improvements of iotests.py) Because of my questions on 2, I haven't queued the series yet; but 3 and 4 were independent enough to take now. > > Vladimir Sementsov-Ogievskiy (8): > block/nbd: move initial connect to coroutine > nbd: allow reconnect on open, with corresponding new options > iotests.py: fix qemu_tool_pipe_and_status() > iotests.py: qemu_io(): reuse qemu_tool_pipe_and_status() > iotests.py: add qemu_tool_popen() > iotests.py: add and use qemu_io_wrap_args() > iotests.py: add qemu_io_popen() > iotests: add 306 to test reconnect on nbd open > > block/nbd.c | 173 +- > tests/qemu-iotests/306| 71 ++ > tests/qemu-iotests/306.out| 11 +++ > tests/qemu-iotests/group | 1 + > tests/qemu-iotests/iotests.py | 56 +++ > 5 files changed, 246 insertions(+), 66 deletions(-) > create mode 100755 tests/qemu-iotests/306 > create mode 100644 tests/qemu-iotests/306.out > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 for-6.0 0/8] nbd reconnect on open
ping ping 18.12.2020 13:57, Vladimir Sementsov-Ogievskiy wrote: ping :) 30.11.2020 16:40, Vladimir Sementsov-Ogievskiy wrote: Hi all! There is a new feature: reconnect on open. It is useful when start of vm and start of nbd server are not simple to sync. v2: rebase on master. Also, I've discovered that I've sent downstream version of test which doesn't work here. So, the test is rewritten (with appropriate improvements of iotests.py) Vladimir Sementsov-Ogievskiy (8): block/nbd: move initial connect to coroutine nbd: allow reconnect on open, with corresponding new options iotests.py: fix qemu_tool_pipe_and_status() iotests.py: qemu_io(): reuse qemu_tool_pipe_and_status() iotests.py: add qemu_tool_popen() iotests.py: add and use qemu_io_wrap_args() iotests.py: add qemu_io_popen() iotests: add 306 to test reconnect on nbd open block/nbd.c | 173 +- tests/qemu-iotests/306 | 71 ++ tests/qemu-iotests/306.out | 11 +++ tests/qemu-iotests/group | 1 + tests/qemu-iotests/iotests.py | 56 +++ 5 files changed, 246 insertions(+), 66 deletions(-) create mode 100755 tests/qemu-iotests/306 create mode 100644 tests/qemu-iotests/306.out -- Best regards, Vladimir
Re: [PATCH v3 9/9] iotests: add nbd-reconnect-on-open test
On Mon, Sep 06, 2021 at 10:06:54PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> .../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 00..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 00..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.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 9/9] iotests: add nbd-reconnect-on-open test
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.
--
Best regards,
Vladimir
[PULL 05/28] iotests/nbd-reconnect-on-open: Fix NBD socket path
Socket paths need to be short to avoid failures. This is why there is a
iotests.sock_dir (defaulting to /tmp) separate from the disk image base
directory.
Make use of it to fix failures in too deeply nested test directories.
Fixes: ab7f7e67a7e7b49964109501dfcde4ec29bae60e
Signed-off-by: Kevin Wolf
Message-Id: <20230503165019.8867-1-kw...@redhat.com>
Reviewed-by: Eric Blake
Reviewed-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/tests/nbd-reconnect-on-open | 3 ++-
tests/qemu-iotests/tests/nbd-reconnect-on-open.out | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open
b/tests/qemu-iotests/tests/nbd-reconnect-on-open
index d0b401b060..3ce52021c3 100755
--- a/tests/qemu-iotests/tests/nbd-reconnect-on-open
+++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open
@@ -26,7 +26,8 @@ from iotests import qemu_img_create, file_path,
qemu_io_popen, qemu_nbd, \
iotests.script_initialize(supported_fmts=['qcow2'])
-disk, nbd_sock = file_path('disk', 'nbd-sock')
+disk = file_path('disk')
+nbd_sock = file_path('nbd-sock', base_dir=iotests.sock_dir)
def create_args(open_timeout):
diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
b/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
index a35ae30ea4..b3dd90f2a3 100644
--- a/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
+++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
@@ -2,10 +2,10 @@ 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: can't open: Failed to connect to 'SOCK_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: can't open: Failed to connect to 'SOCK_DIR/PID-nbd-sock': No such
file or directory
qemu_io finished in 1..1.2 seconds, OK
--
2.40.1
Re: [PATCH v4 7/7] iotests: add nbd-reconnect-on-open test
On 12/13/21 18:32, Vladimir Sementsov-Ogievskiy wrote: Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Nikita Lapshin
