[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 11/32] block/nbd: drop thr->state
From: |
Eric Blake |
Subject: |
Re: [PATCH v4 11/32] block/nbd: drop thr->state |
Date: |
Fri, 11 Jun 2021 09:25:44 -0500 |
User-agent: |
NeoMutt/20210205 |
On Thu, Jun 10, 2021 at 01:07:41PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 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(-)
>
> 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.
@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 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 */
>
Okay, I'm understanding this better than I did in v3.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- Re: [PATCH v4 04/32] block/nbd: connect_thread_func(): do qio_channel_set_delay(false), (continued)
- [PATCH v4 10/32] block/nbd: simplify waking of nbd_co_establish_connection(), Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 17/32] nbd: move connection code from block/nbd to nbd/client-connection, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 12/32] block/nbd: bs-independent interface for nbd_co_establish_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 19/32] nbd/client-connection: add possibility of negotiation, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 11/32] block/nbd: drop thr->state, Vladimir Sementsov-Ogievskiy, 2021/06/10
- Re: [PATCH v4 11/32] block/nbd: drop thr->state,
Eric Blake <=
- [PATCH v4 05/32] qemu-sockets: introduce socket_address_parse_named_fd(), Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 13/32] block/nbd: make nbd_co_establish_connection_cancel() bs-independent, 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 21/32] nbd/client-connection: shutdown connection on release, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 25/32] block/nbd: drop BDRVNBDState::sioc, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 27/32] block-coroutine-wrapper: allow non bdrv_ prefix, Vladimir Sementsov-Ogievskiy, 2021/06/10
- [PATCH v4 16/32] block/nbd: introduce nbd_client_connection_release(), Vladimir Sementsov-Ogievskiy, 2021/06/10