* [PATCH 0/3] nbd reconnect on open
@ 2020-09-07 10:24 Vladimir Sementsov-Ogievskiy
2020-09-07 10:24 ` [PATCH 1/3] block/nbd: move initial connect to coroutine Vladimir Sementsov-Ogievskiy
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-09-07 10:24 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, mreitz, kwolf, eblake, vsementsov, den
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-vsementsov@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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/3] block/nbd: move initial connect to coroutine
2020-09-07 10:24 [PATCH 0/3] nbd reconnect on open Vladimir Sementsov-Ogievskiy
@ 2020-09-07 10:24 ` Vladimir Sementsov-Ogievskiy
2020-09-07 10:24 ` [PATCH 2/3] nbd: allow reconnect on open, with corresponding new options Vladimir Sementsov-Ogievskiy
2020-09-07 10:24 ` [PATCH 3/3] iotests: add 306 to test reconnect on nbd open Vladimir Sementsov-Ogievskiy
2 siblings, 0 replies; 4+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-09-07 10:24 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, mreitz, kwolf, eblake, vsementsov, den
We are going to implement reconnect-on-open. Let's reuse existing
reconnect loop. For this, do initial connect in connection coroutine.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/nbd.c | 94 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 53 insertions(+), 41 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 4548046cd7..16f039b7dc 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -57,6 +57,7 @@ typedef struct {
} NBDClientRequest;
typedef enum NBDClientState {
+ NBD_CLIENT_OPENING,
NBD_CLIENT_CONNECTING_WAIT,
NBD_CLIENT_CONNECTING_NOWAIT,
NBD_CLIENT_CONNECTED,
@@ -113,6 +114,7 @@ typedef struct BDRVNBDState {
CoQueue free_sema;
Coroutine *connection_co;
Coroutine *teardown_co;
+ Coroutine *open_co;
QemuCoSleepState *connection_co_sleep_ns_state;
bool drained;
bool wait_drained_end;
@@ -140,8 +142,6 @@ typedef struct BDRVNBDState {
NBDConnectThread *connect_thread;
} BDRVNBDState;
-static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
- Error **errp);
static QIOChannelSocket *nbd_co_establish_connection(BlockDriverState *bs,
Error **errp);
static void nbd_co_establish_connection_cancel(BlockDriverState *bs,
@@ -338,7 +338,8 @@ static void nbd_teardown_connection(BlockDriverState *bs)
static bool nbd_client_connecting(BDRVNBDState *s)
{
return s->state == NBD_CLIENT_CONNECTING_WAIT ||
- s->state == NBD_CLIENT_CONNECTING_NOWAIT;
+ s->state == NBD_CLIENT_CONNECTING_NOWAIT ||
+ s->state == NBD_CLIENT_OPENING;
}
static bool nbd_client_connecting_wait(BDRVNBDState *s)
@@ -638,6 +639,7 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
{
uint64_t timeout = 1 * NANOSECONDS_PER_SECOND;
uint64_t max_timeout = 16 * NANOSECONDS_PER_SECOND;
+ bool initial_connect = s->state == NBD_CLIENT_OPENING;
if (s->state == NBD_CLIENT_CONNECTING_WAIT) {
reconnect_delay_timer_init(s, qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
@@ -646,6 +648,25 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
nbd_reconnect_attempt(s);
+ if (initial_connect) {
+ if (s->state == NBD_CLIENT_CONNECTED) {
+ /* All good. Just kick nbd_open() to successfully return */
+ if (s->open_co) {
+ aio_co_wake(s->open_co);
+ s->open_co = NULL;
+ }
+ aio_wait_kick();
+ return;
+ } else {
+ /*
+ * Failed. Currently, reconnect on open is not allowed, so quit.
+ * nbd_open() will be kicked in the end of nbd_connection_entry()
+ */
+ s->state = NBD_CLIENT_QUIT;
+ return;
+ }
+ }
+
while (nbd_client_connecting(s)) {
if (s->drained) {
bdrv_dec_in_flight(s->bs);
@@ -758,6 +779,11 @@ static coroutine_fn void nbd_connection_entry(void *opaque)
s->ioc = NULL;
}
+ if (s->open_co) {
+ aio_co_wake(s->open_co);
+ s->open_co = NULL;
+ }
+
if (s->teardown_co) {
aio_co_wake(s->teardown_co);
}
@@ -1746,26 +1772,6 @@ static void nbd_client_close(BlockDriverState *bs)
nbd_teardown_connection(bs);
}
-static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
- Error **errp)
-{
- ERRP_GUARD();
- QIOChannelSocket *sioc;
-
- sioc = qio_channel_socket_new();
- qio_channel_set_name(QIO_CHANNEL(sioc), "nbd-client");
-
- qio_channel_socket_connect_sync(sioc, saddr, errp);
- if (*errp) {
- object_unref(OBJECT(sioc));
- return NULL;
- }
-
- qio_channel_set_delay(QIO_CHANNEL(sioc), false);
-
- return sioc;
-}
-
/* nbd_client_handshake takes ownership on sioc. On failure it is unref'ed. */
static int nbd_client_handshake(BlockDriverState *bs, QIOChannelSocket *sioc,
Error **errp)
@@ -2229,7 +2235,6 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
{
int ret;
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
- QIOChannelSocket *sioc;
ret = nbd_process_options(bs, options, errp);
if (ret < 0) {
@@ -2239,23 +2244,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
s->bs = bs;
qemu_co_mutex_init(&s->send_mutex);
qemu_co_queue_init(&s->free_sema);
-
- /*
- * establish TCP connection, return error if it fails
- * TODO: Configurable retry-until-timeout behaviour.
- */
- sioc = nbd_establish_connection(s->saddr, errp);
- if (!sioc) {
- return -ECONNREFUSED;
- }
-
- ret = nbd_client_handshake(bs, sioc, errp);
- if (ret < 0) {
- nbd_clear_bdrvstate(s);
- return ret;
- }
- /* successfully connected */
- s->state = NBD_CLIENT_CONNECTED;
+ s->state = NBD_CLIENT_OPENING;
nbd_init_connect_thread(s);
@@ -2263,6 +2252,29 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
bdrv_inc_in_flight(bs);
aio_co_schedule(bdrv_get_aio_context(bs), s->connection_co);
+ if (qemu_in_coroutine()) {
+ s->open_co = qemu_coroutine_self();
+ qemu_coroutine_yield();
+ } else {
+ BDRV_POLL_WHILE(bs, s->state == NBD_CLIENT_OPENING);
+ }
+
+ if (s->state != NBD_CLIENT_CONNECTED && s->connect_status < 0) {
+ /*
+ * It's possible that state != NBD_CLIENT_CONNECTED, but connect_status
+ * is 0. This means that initial connecting succeed, but failed later
+ * (during BDRV_POLL_WHILE). It's a rare case, but it happen in iotest
+ * 83. Let's don't care and just report success in this case: it not
+ * much differs from the case when connection failed immediately after
+ * succeeded open.
+ */
+ assert(s->connect_err);
+ error_propagate(errp, s->connect_err);
+ s->connect_err = NULL;
+ nbd_clear_bdrvstate(s);
+ return s->connect_status;
+ }
+
return 0;
}
--
2.21.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] nbd: allow reconnect on open, with corresponding new options
2020-09-07 10:24 [PATCH 0/3] nbd reconnect on open Vladimir Sementsov-Ogievskiy
2020-09-07 10:24 ` [PATCH 1/3] block/nbd: move initial connect to coroutine Vladimir Sementsov-Ogievskiy
@ 2020-09-07 10:24 ` Vladimir Sementsov-Ogievskiy
2020-09-07 10:24 ` [PATCH 3/3] iotests: add 306 to test reconnect on nbd open Vladimir Sementsov-Ogievskiy
2 siblings, 0 replies; 4+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-09-07 10:24 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, mreitz, kwolf, eblake, vsementsov, den
Note: currently, using new option with long timeout in qmp command
blockdev-add is not good idea, as qmp interface is blocking, so,
don't add it now, let's add it later after
"monitor: Optionally run handlers in coroutines" series merged.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/nbd.c | 115 +++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 92 insertions(+), 23 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 16f039b7dc..8fea5ededb 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -125,12 +125,14 @@ typedef struct BDRVNBDState {
bool wait_in_flight;
QEMUTimer *reconnect_delay_timer;
+ QEMUTimer *open_timer;
NBDClientRequest requests[MAX_NBD_REQUESTS];
NBDReply reply;
BlockDriverState *bs;
/* Connection parameters */
+ uint64_t open_timeout;
uint32_t reconnect_delay;
SocketAddress *saddr;
char *export, *tlscredsid;
@@ -304,7 +306,7 @@ static void coroutine_fn nbd_client_co_drain_end(BlockDriverState *bs)
}
-static void nbd_teardown_connection(BlockDriverState *bs)
+static void nbd_teardown_connection_async(BlockDriverState *bs)
{
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
@@ -324,6 +326,14 @@ static void nbd_teardown_connection(BlockDriverState *bs)
}
nbd_co_establish_connection_cancel(bs, true);
}
+}
+
+static void nbd_teardown_connection(BlockDriverState *bs)
+{
+ BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
+
+ nbd_teardown_connection_async(bs);
+
if (qemu_in_coroutine()) {
s->teardown_co = qemu_coroutine_self();
/* connection_co resumes us when it terminates */
@@ -473,6 +483,11 @@ nbd_co_establish_connection(BlockDriverState *bs, Error **errp)
s->wait_connect = true;
qemu_coroutine_yield();
+ if (!s->connect_thread) {
+ error_setg(errp, "Connection attempt cancelled by other operation");
+ return NULL;
+ }
+
qemu_mutex_lock(&thr->mutex);
switch (thr->state) {
@@ -528,6 +543,12 @@ static void nbd_co_establish_connection_cancel(BlockDriverState *bs,
bool wake = false;
bool do_free = false;
+ if (!thr) {
+ /* already detached or finished */
+ assert(!s->wait_connect);
+ return;
+ }
+
qemu_mutex_lock(&thr->mutex);
if (thr->state == CONNECT_THREAD_RUNNING) {
@@ -623,10 +644,15 @@ static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
bdrv_inc_in_flight(s->bs);
out:
- s->connect_status = ret;
- error_free(s->connect_err);
- s->connect_err = NULL;
- error_propagate(&s->connect_err, local_err);
+ if (s->connect_status == -ETIMEDOUT) {
+ /* Don't rewrite timeout error by following cancel-provoked error */
+ error_free(local_err);
+ } else {
+ s->connect_status = ret;
+ error_free(s->connect_err);
+ s->connect_err = NULL;
+ error_propagate(&s->connect_err, local_err);
+ }
if (ret >= 0) {
/* successfully connected */
@@ -635,11 +661,44 @@ out:
}
}
+static void open_timer_del(BDRVNBDState *s)
+{
+ if (s->open_timer) {
+ timer_del(s->open_timer);
+ timer_free(s->open_timer);
+ s->open_timer = NULL;
+ }
+}
+
+static void open_timer_cb(void *opaque)
+{
+ BDRVNBDState *s = opaque;
+
+ if (!s->connect_status) {
+ /* First attempt was not finished. We should set an error */
+ s->connect_status = -ETIMEDOUT;
+ error_setg(&s->connect_err, "First connection attempt is cancelled by "
+ "timeout");
+ }
+
+ nbd_teardown_connection_async(s->bs);
+ open_timer_del(s);
+}
+
+static void open_timer_init(BDRVNBDState *s, uint64_t expire_time_ns)
+{
+ assert(!s->open_timer && s->state == NBD_CLIENT_OPENING);
+ s->open_timer = aio_timer_new(bdrv_get_aio_context(s->bs),
+ QEMU_CLOCK_REALTIME,
+ SCALE_NS,
+ open_timer_cb, s);
+ timer_mod(s->open_timer, expire_time_ns);
+}
+
static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
{
uint64_t timeout = 1 * NANOSECONDS_PER_SECOND;
uint64_t max_timeout = 16 * NANOSECONDS_PER_SECOND;
- bool initial_connect = s->state == NBD_CLIENT_OPENING;
if (s->state == NBD_CLIENT_CONNECTING_WAIT) {
reconnect_delay_timer_init(s, qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
@@ -648,23 +707,9 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
nbd_reconnect_attempt(s);
- if (initial_connect) {
- if (s->state == NBD_CLIENT_CONNECTED) {
- /* All good. Just kick nbd_open() to successfully return */
- if (s->open_co) {
- aio_co_wake(s->open_co);
- s->open_co = NULL;
- }
- aio_wait_kick();
- return;
- } else {
- /*
- * Failed. Currently, reconnect on open is not allowed, so quit.
- * nbd_open() will be kicked in the end of nbd_connection_entry()
- */
- s->state = NBD_CLIENT_QUIT;
- return;
- }
+ if (s->state == NBD_CLIENT_OPENING && !s->open_timeout) {
+ s->state = NBD_CLIENT_QUIT;
+ return;
}
while (nbd_client_connecting(s)) {
@@ -694,6 +739,16 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
}
reconnect_delay_timer_del(s);
+ open_timer_del(s);
+
+ if (s->state == NBD_CLIENT_CONNECTED) {
+ /* All good. Just kick nbd_open() to successfully return */
+ if (s->open_co) {
+ aio_co_wake(s->open_co);
+ s->open_co = NULL;
+ }
+ aio_wait_kick();
+ }
}
static coroutine_fn void nbd_connection_entry(void *opaque)
@@ -2164,6 +2219,14 @@ static QemuOptsList nbd_runtime_opts = {
"future requests before a successful reconnect will "
"immediately fail. Default 0",
},
+ {
+ .name = "open-timeout",
+ .type = QEMU_OPT_NUMBER,
+ .help = "In seconds. If zero, nbd driver tries to establish "
+ "connection only once, on fail open fails. If non-zero, "
+ "nbd driver may do several attempts until success or "
+ "@open-timeout seconds passed. Default 0",
+ },
{ /* end of list */ }
},
};
@@ -2219,6 +2282,7 @@ static int nbd_process_options(BlockDriverState *bs, QDict *options,
}
s->reconnect_delay = qemu_opt_get_number(opts, "reconnect-delay", 0);
+ s->open_timeout = qemu_opt_get_number(opts, "open-timeout", 0);
ret = 0;
@@ -2252,6 +2316,11 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
bdrv_inc_in_flight(bs);
aio_co_schedule(bdrv_get_aio_context(bs), s->connection_co);
+ if (s->open_timeout) {
+ open_timer_init(s, qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
+ s->open_timeout * NANOSECONDS_PER_SECOND);
+ }
+
if (qemu_in_coroutine()) {
s->open_co = qemu_coroutine_self();
qemu_coroutine_yield();
--
2.21.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] iotests: add 306 to test reconnect on nbd open
2020-09-07 10:24 [PATCH 0/3] nbd reconnect on open Vladimir Sementsov-Ogievskiy
2020-09-07 10:24 ` [PATCH 1/3] block/nbd: move initial connect to coroutine Vladimir Sementsov-Ogievskiy
2020-09-07 10:24 ` [PATCH 2/3] nbd: allow reconnect on open, with corresponding new options Vladimir Sementsov-Ogievskiy
@ 2020-09-07 10:24 ` Vladimir Sementsov-Ogievskiy
2 siblings, 0 replies; 4+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-09-07 10:24 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, mreitz, kwolf, eblake, vsementsov, den
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
tests/qemu-iotests/306 | 46 +++++++++++++++++++++++++++++++++++
tests/qemu-iotests/306.out | 5 ++++
tests/qemu-iotests/group | 1 +
tests/qemu-iotests/iotests.py | 11 +++++++++
4 files changed, 63 insertions(+)
create mode 100755 tests/qemu-iotests/306
create mode 100644 tests/qemu-iotests/306.out
diff --git a/tests/qemu-iotests/306 b/tests/qemu-iotests/306
new file mode 100755
index 0000000000..4b8b4090be
--- /dev/null
+++ b/tests/qemu-iotests/306
@@ -0,0 +1,46 @@
+#!/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
+
+disk, nbd_sock = file_path('disk', 'nbd-sock')
+qemu_img_create('-f', iotests.imgfmt, disk, '1M')
+
+client = qemu_io_popen('--image-opts', '-c', 'read 0 1M',
+ 'driver=nbd,__virtuozzo_open-reconnect-attempts=4,'
+ 'server.type=unix,server.path=' + nbd_sock)
+
+time.sleep(2)
+
+qemu_nbd('-k', nbd_sock, '-f', iotests.imgfmt, disk)
+
+iotests.log(client.communicate()[0], filters=[iotests.filter_qemu_io])
+
+client = qemu_io_popen('--image-opts', '-c', 'read 0 1M',
+ 'driver=nbd,__virtuozzo_open-reconnect-attempts=1,'
+ 'server.type=unix,server.path=' + nbd_sock)
+
+time.sleep(1.5)
+
+iotests.log(client.communicate()[0], filters=[iotests.filter_qemu_io,
+ iotests.filter_testfiles])
diff --git a/tests/qemu-iotests/306.out b/tests/qemu-iotests/306.out
new file mode 100644
index 0000000000..82c74b4a2d
--- /dev/null
+++ b/tests/qemu-iotests/306.out
@@ -0,0 +1,5 @@
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+can't open: Failed to connect socket TEST_DIR/PID-nbd-sock: No such file or directory
+
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 5cad015231..e2b479ffb1 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -313,3 +313,4 @@
302 quick
303 rw quick
304 rw quick
+306 rw auto quick
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e197c73ca5..7d8f53fe19 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -195,6 +195,17 @@ def qemu_io_log(*args):
log(result, filters=[filter_testfiles, filter_qemu_io])
return result
+def qemu_io_popen(*args):
+ '''Run qemu-nbd in daemon mode and return the parent's exit code'''
+ default_args = qemu_io_args[:]
+
+ if ('-f' in args or '--image-opts' in args) and '-f' in default_args:
+ ind = default_args.index('-f')
+ del default_args[ind:ind+2]
+
+ return subprocess.Popen(default_args + list(args), stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT)
+
def qemu_io_silent(*args):
'''Run qemu-io and return the exit code, suppressing stdout'''
args = qemu_io_args + list(args)
--
2.21.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-07 10:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-07 10:24 [PATCH 0/3] nbd reconnect on open Vladimir Sementsov-Ogievskiy
2020-09-07 10:24 ` [PATCH 1/3] block/nbd: move initial connect to coroutine Vladimir Sementsov-Ogievskiy
2020-09-07 10:24 ` [PATCH 2/3] nbd: allow reconnect on open, with corresponding new options Vladimir Sementsov-Ogievskiy
2020-09-07 10:24 ` [PATCH 3/3] iotests: add 306 to test reconnect on nbd open Vladimir Sementsov-Ogievskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).