From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
"open list:Network Block Dev..." <qemu-block@nongnu.org>,
Max Reitz <mreitz@redhat.com>
Subject: [PULL 19/34] nbd: move connection code from block/nbd to nbd/client-connection
Date: Tue, 15 Jun 2021 15:47:41 -0500 [thread overview]
Message-ID: <20210615204756.281505-20-eblake@redhat.com> (raw)
In-Reply-To: <20210615204756.281505-1-eblake@redhat.com>
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
We now have bs-independent connection API, which consists of four
functions:
nbd_client_connection_new()
nbd_client_connection_release()
nbd_co_establish_connection()
nbd_co_establish_connection_cancel()
Move them to a separate file together with NBDClientConnection
structure which becomes private to the new API.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-18-vsementsov@virtuozzo.com>
[eblake: comment tweaks]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/nbd.h | 11 ++
block/nbd.c | 207 -----------------------------------
nbd/client-connection.c | 232 ++++++++++++++++++++++++++++++++++++++++
nbd/meson.build | 1 +
4 files changed, 244 insertions(+), 207 deletions(-)
create mode 100644 nbd/client-connection.c
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 5f34d23bb037..57381be76fc5 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -406,4 +406,15 @@ const char *nbd_info_lookup(uint16_t info);
const char *nbd_cmd_lookup(uint16_t info);
const char *nbd_err_lookup(int err);
+/* nbd/client-connection.c */
+typedef struct NBDClientConnection NBDClientConnection;
+
+NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr);
+void nbd_client_connection_release(NBDClientConnection *conn);
+
+QIOChannelSocket *coroutine_fn
+nbd_co_establish_connection(NBDClientConnection *conn, Error **errp);
+
+void coroutine_fn nbd_co_establish_connection_cancel(NBDClientConnection *conn);
+
#endif
diff --git a/block/nbd.c b/block/nbd.c
index fa6e5e85bd6f..26914509f153 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -66,33 +66,6 @@ typedef enum NBDClientState {
NBD_CLIENT_QUIT
} NBDClientState;
-typedef struct NBDClientConnection {
- /* Initialization constants */
- SocketAddress *saddr; /* address to connect to */
-
- QemuMutex mutex;
-
- /*
- * @sioc and @err represent a connection attempt. While running
- * is true, they are only used by the connection thread, and mutex
- * locking is not needed. Once the thread finishes,
- * nbd_co_establish_connection then steals these pointers while
- * under the mutex.
- */
- QIOChannelSocket *sioc;
- Error *err;
-
- /* All further fields are accessed only under mutex */
- bool running; /* thread is running now */
- bool detached; /* thread is detached and should cleanup the state */
-
- /*
- * wait_co: if non-NULL, which coroutine to wake in
- * nbd_co_establish_connection() after yield()
- */
- Coroutine *wait_co;
-} NBDClientConnection;
-
typedef struct BDRVNBDState {
QIOChannelSocket *sioc; /* The master data channel */
QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
@@ -127,12 +100,8 @@ typedef struct BDRVNBDState {
NBDClientConnection *conn;
} BDRVNBDState;
-static void nbd_client_connection_release(NBDClientConnection *conn);
static int nbd_establish_connection(BlockDriverState *bs, SocketAddress *saddr,
Error **errp);
-static coroutine_fn QIOChannelSocket *
-nbd_co_establish_connection(NBDClientConnection *conn, Error **errp);
-static void nbd_co_establish_connection_cancel(NBDClientConnection *conn);
static int nbd_client_handshake(BlockDriverState *bs, Error **errp);
static void nbd_yank(void *opaque);
@@ -345,182 +314,6 @@ static bool nbd_client_connecting_wait(BDRVNBDState *s)
return qatomic_load_acquire(&s->state) == NBD_CLIENT_CONNECTING_WAIT;
}
-static NBDClientConnection *
-nbd_client_connection_new(const SocketAddress *saddr)
-{
- NBDClientConnection *conn = g_new(NBDClientConnection, 1);
-
- *conn = (NBDClientConnection) {
- .saddr = QAPI_CLONE(SocketAddress, saddr),
- };
-
- qemu_mutex_init(&conn->mutex);
-
- return conn;
-}
-
-static void nbd_client_connection_do_free(NBDClientConnection *conn)
-{
- if (conn->sioc) {
- qio_channel_close(QIO_CHANNEL(conn->sioc), NULL);
- object_unref(OBJECT(conn->sioc));
- }
- error_free(conn->err);
- qapi_free_SocketAddress(conn->saddr);
- g_free(conn);
-}
-
-static void *connect_thread_func(void *opaque)
-{
- NBDClientConnection *conn = opaque;
- int ret;
- bool do_free;
-
- conn->sioc = qio_channel_socket_new();
-
- error_free(conn->err);
- conn->err = NULL;
- ret = qio_channel_socket_connect_sync(conn->sioc, conn->saddr, &conn->err);
- if (ret < 0) {
- object_unref(OBJECT(conn->sioc));
- conn->sioc = NULL;
- }
-
- qio_channel_set_delay(QIO_CHANNEL(conn->sioc), false);
-
- qemu_mutex_lock(&conn->mutex);
-
- assert(conn->running);
- conn->running = false;
- if (conn->wait_co) {
- aio_co_wake(conn->wait_co);
- conn->wait_co = NULL;
- }
- do_free = conn->detached;
-
- qemu_mutex_unlock(&conn->mutex);
-
- if (do_free) {
- nbd_client_connection_do_free(conn);
- }
-
- return NULL;
-}
-
-static void nbd_client_connection_release(NBDClientConnection *conn)
-{
- bool do_free = false;
-
- if (!conn) {
- return;
- }
-
- qemu_mutex_lock(&conn->mutex);
- assert(!conn->detached);
- if (conn->running) {
- conn->detached = true;
- } else {
- do_free = true;
- }
- qemu_mutex_unlock(&conn->mutex);
-
- if (do_free) {
- nbd_client_connection_do_free(conn);
- }
-}
-
-/*
- * Get a new connection in context of @conn:
- * if the thread is running, wait for completion
- * if the thread already succeeded in the background, and user didn't get the
- * result, just return it now
- * otherwise the thread is not running, so start a thread and wait for
- * completion
- */
-static coroutine_fn QIOChannelSocket *
-nbd_co_establish_connection(NBDClientConnection *conn, Error **errp)
-{
- QIOChannelSocket *sioc = NULL;
- QemuThread thread;
-
- qemu_mutex_lock(&conn->mutex);
-
- /*
- * Don't call nbd_co_establish_connection() in several coroutines in
- * parallel. Only one call at once is supported.
- */
- assert(!conn->wait_co);
-
- if (!conn->running) {
- if (conn->sioc) {
- /* Previous attempt finally succeeded in background */
- sioc = g_steal_pointer(&conn->sioc);
- qemu_mutex_unlock(&conn->mutex);
-
- return sioc;
- }
-
- conn->running = true;
- error_free(conn->err);
- conn->err = NULL;
- qemu_thread_create(&thread, "nbd-connect",
- connect_thread_func, conn, QEMU_THREAD_DETACHED);
- }
-
- conn->wait_co = qemu_coroutine_self();
-
- qemu_mutex_unlock(&conn->mutex);
-
- /*
- * We are going to wait for connect-thread finish, but
- * nbd_co_establish_connection_cancel() can interrupt.
- */
- qemu_coroutine_yield();
-
- qemu_mutex_lock(&conn->mutex);
-
- if (conn->running) {
- /*
- * The connection attempt was canceled and the coroutine resumed
- * before the connection thread finished its job. Report the
- * attempt as failed, but leave the connection thread running,
- * to reuse it for the next connection attempt.
- */
- error_setg(errp, "Connection attempt cancelled by other operation");
- } else {
- error_propagate(errp, conn->err);
- conn->err = NULL;
- sioc = g_steal_pointer(&conn->sioc);
- }
-
- qemu_mutex_unlock(&conn->mutex);
-
- return sioc;
-}
-
-/*
- * nbd_co_establish_connection_cancel
- * Cancel nbd_co_establish_connection() asynchronously.
- *
- * Note that this function neither directly stops the thread nor closes the
- * socket, but rather safely wakes nbd_co_establish_connection() which is
- * sleeping in yield()
- */
-static void nbd_co_establish_connection_cancel(NBDClientConnection *conn)
-{
- Coroutine *wait_co;
-
- qemu_mutex_lock(&conn->mutex);
-
- wait_co = g_steal_pointer(&conn->wait_co);
-
- qemu_mutex_unlock(&conn->mutex);
-
- if (wait_co) {
- aio_co_wake(wait_co);
- }
-}
-
static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
{
int ret;
diff --git a/nbd/client-connection.c b/nbd/client-connection.c
new file mode 100644
index 000000000000..f3b270d38cc8
--- /dev/null
+++ b/nbd/client-connection.c
@@ -0,0 +1,232 @@
+/*
+ * QEMU Block driver for NBD
+ *
+ * Copyright (c) 2021 Virtuozzo International GmbH.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+
+#include "block/nbd.h"
+
+#include "qapi/qapi-visit-sockets.h"
+#include "qapi/clone-visitor.h"
+
+struct NBDClientConnection {
+ /* Initialization constants */
+ SocketAddress *saddr; /* address to connect to */
+
+ QemuMutex mutex;
+
+ /*
+ * @sioc and @err represent a connection attempt. While running
+ * is true, they are only used by the connection thread, and mutex
+ * locking is not needed. Once the thread finishes,
+ * nbd_co_establish_connection then steals these pointers while
+ * under the mutex.
+ */
+ QIOChannelSocket *sioc;
+ Error *err;
+
+ /* All further fields are accessed only under mutex */
+ bool running; /* thread is running now */
+ bool detached; /* thread is detached and should cleanup the state */
+
+ /*
+ * wait_co: if non-NULL, which coroutine to wake in
+ * nbd_co_establish_connection() after yield()
+ */
+ Coroutine *wait_co;
+};
+
+NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr)
+{
+ NBDClientConnection *conn = g_new(NBDClientConnection, 1);
+
+ *conn = (NBDClientConnection) {
+ .saddr = QAPI_CLONE(SocketAddress, saddr),
+ };
+
+ qemu_mutex_init(&conn->mutex);
+
+ return conn;
+}
+
+static void nbd_client_connection_do_free(NBDClientConnection *conn)
+{
+ if (conn->sioc) {
+ qio_channel_close(QIO_CHANNEL(conn->sioc), NULL);
+ object_unref(OBJECT(conn->sioc));
+ }
+ error_free(conn->err);
+ qapi_free_SocketAddress(conn->saddr);
+ g_free(conn);
+}
+
+static void *connect_thread_func(void *opaque)
+{
+ NBDClientConnection *conn = opaque;
+ int ret;
+ bool do_free;
+
+ conn->sioc = qio_channel_socket_new();
+
+ error_free(conn->err);
+ conn->err = NULL;
+ ret = qio_channel_socket_connect_sync(conn->sioc, conn->saddr, &conn->err);
+ if (ret < 0) {
+ object_unref(OBJECT(conn->sioc));
+ conn->sioc = NULL;
+ }
+
+ qio_channel_set_delay(QIO_CHANNEL(conn->sioc), false);
+
+ qemu_mutex_lock(&conn->mutex);
+
+ assert(conn->running);
+ conn->running = false;
+ if (conn->wait_co) {
+ aio_co_wake(conn->wait_co);
+ conn->wait_co = NULL;
+ }
+ do_free = conn->detached;
+
+ qemu_mutex_unlock(&conn->mutex);
+
+ if (do_free) {
+ nbd_client_connection_do_free(conn);
+ }
+
+ return NULL;
+}
+
+void nbd_client_connection_release(NBDClientConnection *conn)
+{
+ bool do_free = false;
+
+ if (!conn) {
+ return;
+ }
+
+ qemu_mutex_lock(&conn->mutex);
+ assert(!conn->detached);
+ if (conn->running) {
+ conn->detached = true;
+ } else {
+ do_free = true;
+ }
+ qemu_mutex_unlock(&conn->mutex);
+
+ if (do_free) {
+ nbd_client_connection_do_free(conn);
+ }
+}
+
+/*
+ * Get a new connection in context of @conn:
+ * if the thread is running, wait for completion
+ * if the thread already succeeded in the background, and user didn't get the
+ * result, just return it now
+ * otherwise the thread is not running, so start a thread and wait for
+ * completion
+ */
+QIOChannelSocket *coroutine_fn
+nbd_co_establish_connection(NBDClientConnection *conn, Error **errp)
+{
+ QIOChannelSocket *sioc = NULL;
+ QemuThread thread;
+
+ qemu_mutex_lock(&conn->mutex);
+
+ /*
+ * Don't call nbd_co_establish_connection() in several coroutines in
+ * parallel. Only one call at once is supported.
+ */
+ assert(!conn->wait_co);
+
+ if (!conn->running) {
+ if (conn->sioc) {
+ /* Previous attempt finally succeeded in background */
+ sioc = g_steal_pointer(&conn->sioc);
+ qemu_mutex_unlock(&conn->mutex);
+
+ return sioc;
+ }
+
+ conn->running = true;
+ error_free(conn->err);
+ conn->err = NULL;
+ qemu_thread_create(&thread, "nbd-connect",
+ connect_thread_func, conn, QEMU_THREAD_DETACHED);
+ }
+
+ conn->wait_co = qemu_coroutine_self();
+
+ qemu_mutex_unlock(&conn->mutex);
+
+ /*
+ * We are going to wait for connect-thread finish, but
+ * nbd_co_establish_connection_cancel() can interrupt.
+ */
+ qemu_coroutine_yield();
+
+ qemu_mutex_lock(&conn->mutex);
+
+ if (conn->running) {
+ /*
+ * The connection attempt was canceled and the coroutine resumed
+ * before the connection thread finished its job. Report the
+ * attempt as failed, but leave the connection thread running,
+ * to reuse it for the next connection attempt.
+ */
+ error_setg(errp, "Connection attempt cancelled by other operation");
+ } else {
+ error_propagate(errp, conn->err);
+ conn->err = NULL;
+ sioc = g_steal_pointer(&conn->sioc);
+ }
+
+ qemu_mutex_unlock(&conn->mutex);
+
+ return sioc;
+}
+
+/*
+ * nbd_co_establish_connection_cancel
+ * Cancel nbd_co_establish_connection() asynchronously.
+ *
+ * Note that this function neither directly stops the thread nor closes the
+ * socket, but rather safely wakes nbd_co_establish_connection() which is
+ * sleeping in yield()
+ */
+void nbd_co_establish_connection_cancel(NBDClientConnection *conn)
+{
+ Coroutine *wait_co;
+
+ qemu_mutex_lock(&conn->mutex);
+
+ wait_co = g_steal_pointer(&conn->wait_co);
+
+ qemu_mutex_unlock(&conn->mutex);
+
+ if (wait_co) {
+ aio_co_wake(wait_co);
+ }
+}
diff --git a/nbd/meson.build b/nbd/meson.build
index 2baaa3694801..b26d70565ebb 100644
--- a/nbd/meson.build
+++ b/nbd/meson.build
@@ -1,5 +1,6 @@
block_ss.add(files(
'client.c',
+ 'client-connection.c',
'common.c',
))
blockdev_ss.add(files(
--
2.31.1
next prev parent reply other threads:[~2021-06-15 21:12 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-15 20:47 [PULL 00/34] NBD patches for 2021-06-15 Eric Blake
2021-06-15 20:47 ` [PULL 01/34] async: the main AioContext is only "current" if under the BQL Eric Blake
2021-06-15 20:47 ` [PULL 02/34] tests: cover aio_co_enter from a worker thread without BQL taken Eric Blake
2021-06-15 20:47 ` [PULL 03/34] co-queue: drop extra coroutine_fn marks Eric Blake
2021-06-15 20:47 ` [PULL 04/34] block/nbd: fix channel object leak Eric Blake
2021-06-15 20:47 ` [PULL 05/34] block/nbd: fix how state is cleared on nbd_open() failure paths Eric Blake
2021-06-15 20:47 ` [PULL 06/34] block/nbd: connect_thread_func(): do qio_channel_set_delay(false) Eric Blake
2021-06-15 20:47 ` [PULL 07/34] qemu-sockets: introduce socket_address_parse_named_fd() Eric Blake
2021-06-15 20:47 ` [PULL 08/34] block/nbd: call socket_address_parse_named_fd() in advance Eric Blake
2021-06-15 20:47 ` [PULL 09/34] block/nbd: ensure ->connection_thread is always valid Eric Blake
2021-06-15 20:47 ` [PULL 10/34] block/nbd: nbd_client_handshake(): fix leak of s->ioc Eric Blake
2021-06-15 20:47 ` [PULL 11/34] block/nbd: BDRVNBDState: drop unused connect_err and connect_status Eric Blake
2021-06-15 20:47 ` [PULL 12/34] block/nbd: simplify waking of nbd_co_establish_connection() Eric Blake
2021-06-15 20:47 ` [PULL 13/34] block/nbd: drop thr->state Eric Blake
2021-06-15 20:47 ` [PULL 14/34] block/nbd: bs-independent interface for nbd_co_establish_connection() Eric Blake
2021-06-15 20:47 ` [PULL 15/34] block/nbd: make nbd_co_establish_connection_cancel() bs-independent Eric Blake
2021-06-15 20:47 ` [PULL 16/34] block/nbd: rename NBDConnectThread to NBDClientConnection Eric Blake
2021-06-15 20:47 ` [PULL 17/34] block/nbd: introduce nbd_client_connection_new() Eric Blake
2021-06-15 20:47 ` [PULL 18/34] block/nbd: introduce nbd_client_connection_release() Eric Blake
2021-06-15 20:47 ` Eric Blake [this message]
2021-06-15 20:47 ` [PULL 20/34] nbd/client-connection: use QEMU_LOCK_GUARD Eric Blake
2021-06-15 20:47 ` [PULL 21/34] nbd/client-connection: add possibility of negotiation Eric Blake
2021-06-15 20:47 ` [PULL 22/34] nbd/client-connection: implement connection retry Eric Blake
2021-06-15 20:47 ` [PULL 23/34] nbd/client-connection: shutdown connection on release Eric Blake
2021-06-15 20:47 ` [PULL 24/34] block/nbd: split nbd_handle_updated_info out of nbd_client_handshake() Eric Blake
2021-06-15 20:47 ` [PULL 25/34] block/nbd: use negotiation of NBDClientConnection Eric Blake
2021-06-15 20:47 ` [PULL 26/34] block/nbd: don't touch s->sioc in nbd_teardown_connection() Eric Blake
2021-06-15 20:47 ` [PULL 27/34] block/nbd: drop BDRVNBDState::sioc Eric Blake
2021-06-15 20:47 ` [PULL 28/34] nbd/client-connection: return only one io channel Eric Blake
2021-06-17 18:32 ` Vladimir Sementsov-Ogievskiy
2021-06-18 15:55 ` Eric Blake
2021-06-15 20:47 ` [PULL 29/34] block-coroutine-wrapper: allow non bdrv_ prefix Eric Blake
2021-06-15 20:47 ` [PULL 30/34] block/nbd: split nbd_co_do_establish_connection out of nbd_reconnect_attempt Eric Blake
2021-06-15 20:47 ` [PULL 31/34] nbd/client-connection: add option for non-blocking connection attempt Eric Blake
2021-06-15 20:47 ` [PULL 32/34] block/nbd: reuse nbd_co_do_establish_connection() in nbd_open() Eric Blake
2021-06-15 20:47 ` [PULL 33/34] block/nbd: add nbd_client_connected() helper Eric Blake
2021-06-15 20:47 ` [PULL 34/34] block/nbd: safer transition to receiving request Eric Blake
2021-06-17 9:42 ` [PULL 00/34] NBD patches for 2021-06-15 Peter Maydell
2021-06-17 18:35 ` Vladimir Sementsov-Ogievskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210615204756.281505-20-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.