[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v11 2/7] block/nbd.c: Add yank feature
From: |
Eric Blake |
Subject: |
Re: [PATCH v11 2/7] block/nbd.c: Add yank feature |
Date: |
Tue, 1 Dec 2020 14:50:06 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 |
On 11/15/20 5:36 AM, Lukas Straub wrote:
> Register a yank function which shuts down the socket and sets
> s->state = NBD_CLIENT_QUIT. This is the same behaviour as if an
> error occured.
occurred
>
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> block/nbd.c | 154 +++++++++++++++++++++++++++++++---------------------
> 1 file changed, 93 insertions(+), 61 deletions(-)
>
> @@ -166,12 +168,12 @@ static void nbd_clear_bdrvstate(BDRVNBDState *s)
> static void nbd_channel_error(BDRVNBDState *s, int ret)
> {
> if (ret == -EIO) {
> - if (s->state == NBD_CLIENT_CONNECTED) {
> + if (qatomic_load_acquire(&s->state) == NBD_CLIENT_CONNECTED) {
This may have interesting interactions with Vladimir's latest patches to
make NBD connection re-startable, but we'll sort that out as needed.
https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg07012.html
The patch seems big; I might have broken it into two pieces (conversion
of existing logic to use qatomic_*() accesses instead of direct s->state
manipulation, and then adding new logic). But I'm not going to hold up
the series demanding for a split at this time.
> @@ -424,12 +427,12 @@ static void *connect_thread_func(void *opaque)
> return NULL;
> }
>
> -static QIOChannelSocket *coroutine_fn
> +static int coroutine_fn
> nbd_co_establish_connection(BlockDriverState *bs, Error **errp)
> {
> + int ret;
> QemuThread thread;
> BDRVNBDState *s = bs->opaque;
> - QIOChannelSocket *res;
> NBDConnectThread *thr = s->connect_thread;
>
> qemu_mutex_lock(&thr->mutex);
> @@ -446,10 +449,12 @@ nbd_co_establish_connection(BlockDriverState *bs, Error
> **errp)
> case CONNECT_THREAD_SUCCESS:
> /* Previous attempt finally succeeded in background */
> thr->state = CONNECT_THREAD_NONE;
> - res = thr->sioc;
> + 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 res;
> + return 0;
Looks sensible.
> @@ -1745,6 +1759,15 @@ static int nbd_client_reopen_prepare(BDRVReopenState
> *state,
> return 0;
> }
>
> +static void nbd_yank(void *opaque)
> +{
> + BlockDriverState *bs = opaque;
> + BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
> +
> + qatomic_store_release(&s->state, NBD_CLIENT_QUIT);
> + qio_channel_shutdown(QIO_CHANNEL(s->sioc), QIO_CHANNEL_SHUTDOWN_BOTH,
> NULL);
> +}
> +
Yep, that does indeed tell qemu to give up on the NBD socket right away.
Reviewed-by: Eric Blake <eblake@redhat.com>
And sorry it's taken me so long to actually stare at this series.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- Re: [PATCH v11 2/7] block/nbd.c: Add yank feature,
Eric Blake <=