[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v4 11/32] block/nbd: drop thr->state
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
[PATCH v4 11/32] block/nbd: drop thr->state |
Date: |
Thu, 10 Jun 2021 13:07:41 +0300 |
We don't need all these states. The code refactored to use two boolean
variables looks simpler.
While moving the comment in nbd_co_establish_connection() rework it to
give better information. Also, we are going to move the connection code
to separate file and mentioning drained section would be confusing.
Improve also the comment in NBDConnectThread, while dropping removed
state names from it.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/nbd.c | 139 +++++++++++++++++-----------------------------------
1 file changed, 44 insertions(+), 95 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 653af62940..58463636f0 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -66,38 +66,24 @@ typedef enum NBDClientState {
NBD_CLIENT_QUIT
} NBDClientState;
-typedef enum NBDConnectThreadState {
- /* No thread, no pending results */
- CONNECT_THREAD_NONE,
-
- /* Thread is running, no results for now */
- CONNECT_THREAD_RUNNING,
-
- /*
- * Thread is running, but requestor exited. Thread should close
- * the new socket and free the connect state on exit.
- */
- CONNECT_THREAD_RUNNING_DETACHED,
-
- /* Thread finished, results are stored in a state */
- CONNECT_THREAD_FAIL,
- CONNECT_THREAD_SUCCESS
-} NBDConnectThreadState;
-
typedef struct NBDConnectThread {
/* Initialization constants */
SocketAddress *saddr; /* address to connect to */
+ QemuMutex mutex;
+
/*
- * Result of last attempt. Valid in FAIL and SUCCESS states.
- * If you want to steal error, don't forget to set pointer to NULL.
+ * @sioc and @err present a result of connection attempt.
+ * While running is true, they are used only by thread, mutex locking is
not
+ * needed. When thread is finished nbd_co_establish_connection steal these
+ * pointers under mutex.
*/
QIOChannelSocket *sioc;
Error *err;
- QemuMutex mutex;
- /* All further fields are protected by mutex */
- NBDConnectThreadState state; /* current state of the thread */
+ /* 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
@@ -152,17 +138,19 @@ static void nbd_clear_bdrvstate(BlockDriverState *bs)
{
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
NBDConnectThread *thr = s->connect_thread;
- bool thr_running;
+ bool do_free = false;
qemu_mutex_lock(&thr->mutex);
- thr_running = thr->state == CONNECT_THREAD_RUNNING;
- if (thr_running) {
- thr->state = CONNECT_THREAD_RUNNING_DETACHED;
+ assert(!thr->detached);
+ if (thr->running) {
+ thr->detached = true;
+ } else {
+ do_free = true;
}
qemu_mutex_unlock(&thr->mutex);
/* the runaway thread will clean up itself */
- if (!thr_running) {
+ if (do_free) {
nbd_free_connect_thread(thr);
}
@@ -374,7 +362,6 @@ static void nbd_init_connect_thread(BDRVNBDState *s)
*s->connect_thread = (NBDConnectThread) {
.saddr = QAPI_CLONE(SocketAddress, s->saddr),
- .state = CONNECT_THREAD_NONE,
};
qemu_mutex_init(&s->connect_thread->mutex);
@@ -395,7 +382,7 @@ static void *connect_thread_func(void *opaque)
{
NBDConnectThread *thr = opaque;
int ret;
- bool do_free = false;
+ bool do_free;
thr->sioc = qio_channel_socket_new();
@@ -411,20 +398,13 @@ static void *connect_thread_func(void *opaque)
qemu_mutex_lock(&thr->mutex);
- switch (thr->state) {
- case CONNECT_THREAD_RUNNING:
- thr->state = ret < 0 ? CONNECT_THREAD_FAIL : CONNECT_THREAD_SUCCESS;
- if (thr->wait_co) {
- aio_co_wake(thr->wait_co);
- thr->wait_co = NULL;
- }
- break;
- case CONNECT_THREAD_RUNNING_DETACHED:
- do_free = true;
- break;
- default:
- abort();
+ assert(thr->running);
+ thr->running = false;
+ if (thr->wait_co) {
+ aio_co_wake(thr->wait_co);
+ thr->wait_co = NULL;
}
+ do_free = thr->detached;
qemu_mutex_unlock(&thr->mutex);
@@ -438,36 +418,24 @@ static void *connect_thread_func(void *opaque)
static int coroutine_fn
nbd_co_establish_connection(BlockDriverState *bs, Error **errp)
{
- int ret;
QemuThread thread;
BDRVNBDState *s = bs->opaque;
NBDConnectThread *thr = s->connect_thread;
+ assert(!s->sioc);
+
qemu_mutex_lock(&thr->mutex);
- switch (thr->state) {
- case CONNECT_THREAD_FAIL:
- case CONNECT_THREAD_NONE:
+ if (!thr->running) {
+ if (thr->sioc) {
+ /* Previous attempt finally succeeded in background */
+ goto out;
+ }
+ thr->running = true;
error_free(thr->err);
thr->err = NULL;
- thr->state = CONNECT_THREAD_RUNNING;
qemu_thread_create(&thread, "nbd-connect",
connect_thread_func, thr, QEMU_THREAD_DETACHED);
- break;
- case CONNECT_THREAD_SUCCESS:
- /* Previous attempt finally succeeded in background */
- thr->state = CONNECT_THREAD_NONE;
- s->sioc = thr->sioc;
- thr->sioc = NULL;
- yank_register_function(BLOCKDEV_YANK_INSTANCE(bs->node_name),
- nbd_yank, bs);
- qemu_mutex_unlock(&thr->mutex);
- return 0;
- case CONNECT_THREAD_RUNNING:
- /* Already running, will wait */
- break;
- default:
- abort();
}
thr->wait_co = qemu_coroutine_self();
@@ -482,10 +450,16 @@ nbd_co_establish_connection(BlockDriverState *bs, Error
**errp)
qemu_mutex_lock(&thr->mutex);
- switch (thr->state) {
- case CONNECT_THREAD_SUCCESS:
- case CONNECT_THREAD_FAIL:
- thr->state = CONNECT_THREAD_NONE;
+out:
+ if (thr->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, thr->err);
thr->err = NULL;
s->sioc = thr->sioc;
@@ -494,33 +468,11 @@ nbd_co_establish_connection(BlockDriverState *bs, Error
**errp)
yank_register_function(BLOCKDEV_YANK_INSTANCE(bs->node_name),
nbd_yank, bs);
}
- ret = (s->sioc ? 0 : -1);
- break;
- case CONNECT_THREAD_RUNNING:
- case CONNECT_THREAD_RUNNING_DETACHED:
- /*
- * Obviously, drained section wants to start. Report the attempt as
- * failed. Still connect thread is executing in background, and its
- * result may be used for next connection attempt.
- */
- ret = -1;
- error_setg(errp, "Connection attempt cancelled by other operation");
- break;
-
- case CONNECT_THREAD_NONE:
- /*
- * Impossible. We've seen this thread running. So it should be
- * running or at least give some results.
- */
- abort();
-
- default:
- abort();
}
qemu_mutex_unlock(&thr->mutex);
- return ret;
+ return s->sioc ? 0 : -1;
}
/*
@@ -532,14 +484,11 @@ static void
nbd_co_establish_connection_cancel(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
NBDConnectThread *thr = s->connect_thread;
- Coroutine *wait_co = NULL;
+ Coroutine *wait_co;
qemu_mutex_lock(&thr->mutex);
- if (thr->state == CONNECT_THREAD_RUNNING) {
- /* We can cancel only in running state, when bh is not yet scheduled */
- wait_co = g_steal_pointer(&thr->wait_co);
- }
+ wait_co = g_steal_pointer(&thr->wait_co);
qemu_mutex_unlock(&thr->mutex);
--
2.29.2
- [PATCH v4 06/32] block/nbd: call socket_address_parse_named_fd() in advance, (continued)
- [PATCH v4 06/32] block/nbd: call socket_address_parse_named_fd() in advance, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 07/32] block/nbd: ensure ->connection_thread is always valid, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 03/32] block/nbd: fix how state is cleared on nbd_open() failure paths, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 08/32] block/nbd: nbd_client_handshake(): fix leak of s->ioc, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 09/32] block/nbd: BDRVNBDState: drop unused connect_err and connect_status, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 04/32] block/nbd: connect_thread_func(): do qio_channel_set_delay(false), Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 10/32] block/nbd: simplify waking of nbd_co_establish_connection(), Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 11/32] block/nbd: drop thr->state,
Vladimir Sementsov-Ogievskiy <=
- [PATCH v4 17/32] nbd: move connection code from block/nbd to nbd/client-connection, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 18/32] nbd/client-connection: use QEMU_LOCK_GUARD, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 05/32] qemu-sockets: introduce socket_address_parse_named_fd(), Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 15/32] block/nbd: introduce nbd_client_connection_new(), Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 20/32] nbd/client-connection: implement connection retry, Vladimir Sementsov-Ogievskiy, 2021/06/10