From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, vsementsov@virtuozzo.com,
eblake@redhat.com, mreitz@redhat.com, kwolf@redhat.com,
pbonzini@redhat.com, Roman Kagan <rvkagan@yandex-team.ru>
Subject: [PATCH v4 03/32] block/nbd: fix how state is cleared on nbd_open() failure paths
Date: Thu, 10 Jun 2021 13:07:33 +0300 [thread overview]
Message-ID: <20210610100802.5888-4-vsementsov@virtuozzo.com> (raw)
In-Reply-To: <20210610100802.5888-1-vsementsov@virtuozzo.com>
We have two "return error" paths in nbd_open() after
nbd_process_options(). Actually we should call nbd_clear_bdrvstate()
on these paths. Interesting that nbd_process_options() calls
nbd_clear_bdrvstate() by itself.
Let's fix leaks and refactor things to be more obvious:
- intialize yank at top of nbd_open()
- move yank cleanup to nbd_clear_bdrvstate()
- refactor nbd_open() so that all failure paths except for
yank-register goes through nbd_clear_bdrvstate()
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/nbd.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index f4b3407587..01d2c2efad 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -152,8 +152,12 @@ static void nbd_co_establish_connection_cancel(BlockDriverState *bs,
static int nbd_client_handshake(BlockDriverState *bs, Error **errp);
static void nbd_yank(void *opaque);
-static void nbd_clear_bdrvstate(BDRVNBDState *s)
+static void nbd_clear_bdrvstate(BlockDriverState *bs)
{
+ BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
+
+ yank_unregister_instance(BLOCKDEV_YANK_INSTANCE(bs->node_name));
+
object_unref(OBJECT(s->tlscreds));
qapi_free_SocketAddress(s->saddr);
s->saddr = NULL;
@@ -2275,9 +2279,6 @@ static int nbd_process_options(BlockDriverState *bs, QDict *options,
ret = 0;
error:
- if (ret < 0) {
- nbd_clear_bdrvstate(s);
- }
qemu_opts_del(opts);
return ret;
}
@@ -2288,11 +2289,6 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
int ret;
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
- ret = nbd_process_options(bs, options, errp);
- if (ret < 0) {
- return ret;
- }
-
s->bs = bs;
qemu_co_mutex_init(&s->send_mutex);
qemu_co_queue_init(&s->free_sema);
@@ -2301,20 +2297,23 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
return -EEXIST;
}
+ ret = nbd_process_options(bs, options, errp);
+ if (ret < 0) {
+ goto fail;
+ }
+
/*
* establish TCP connection, return error if it fails
* TODO: Configurable retry-until-timeout behaviour.
*/
if (nbd_establish_connection(bs, s->saddr, errp) < 0) {
- yank_unregister_instance(BLOCKDEV_YANK_INSTANCE(bs->node_name));
- return -ECONNREFUSED;
+ ret = -ECONNREFUSED;
+ goto fail;
}
ret = nbd_client_handshake(bs, errp);
if (ret < 0) {
- yank_unregister_instance(BLOCKDEV_YANK_INSTANCE(bs->node_name));
- nbd_clear_bdrvstate(s);
- return ret;
+ goto fail;
}
/* successfully connected */
s->state = NBD_CLIENT_CONNECTED;
@@ -2326,6 +2325,10 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
aio_co_schedule(bdrv_get_aio_context(bs), s->connection_co);
return 0;
+
+fail:
+ nbd_clear_bdrvstate(bs);
+ return ret;
}
static int nbd_co_flush(BlockDriverState *bs)
@@ -2369,11 +2372,8 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
static void nbd_close(BlockDriverState *bs)
{
- BDRVNBDState *s = bs->opaque;
-
nbd_client_close(bs);
- yank_unregister_instance(BLOCKDEV_YANK_INSTANCE(bs->node_name));
- nbd_clear_bdrvstate(s);
+ nbd_clear_bdrvstate(bs);
}
/*
--
2.29.2
next prev parent reply other threads:[~2021-06-10 10:14 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 10:07 [PATCH v4 00/32] block/nbd: rework client connection Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 01/32] co-queue: drop extra coroutine_fn marks Vladimir Sementsov-Ogievskiy
2021-06-10 17:22 ` Eric Blake
2021-06-10 17:37 ` Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 02/32] block/nbd: fix channel object leak Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` Vladimir Sementsov-Ogievskiy [this message]
2021-06-10 10:07 ` [PATCH v4 04/32] block/nbd: connect_thread_func(): do qio_channel_set_delay(false) Vladimir Sementsov-Ogievskiy
2021-06-10 18:37 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 05/32] qemu-sockets: introduce socket_address_parse_named_fd() Vladimir Sementsov-Ogievskiy
2021-06-11 13:22 ` Eric Blake
2021-06-11 14:10 ` Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 06/32] block/nbd: call socket_address_parse_named_fd() in advance Vladimir Sementsov-Ogievskiy
2021-06-11 13:54 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 07/32] block/nbd: ensure ->connection_thread is always valid Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 08/32] block/nbd: nbd_client_handshake(): fix leak of s->ioc Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 09/32] block/nbd: BDRVNBDState: drop unused connect_err and connect_status Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 10/32] block/nbd: simplify waking of nbd_co_establish_connection() Vladimir Sementsov-Ogievskiy
2021-06-11 14:06 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 11/32] block/nbd: drop thr->state Vladimir Sementsov-Ogievskiy
2021-06-11 14:25 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 12/32] block/nbd: bs-independent interface for nbd_co_establish_connection() Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 13/32] block/nbd: make nbd_co_establish_connection_cancel() bs-independent Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 14/32] block/nbd: rename NBDConnectThread to NBDClientConnection Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 15/32] block/nbd: introduce nbd_client_connection_new() Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 16/32] block/nbd: introduce nbd_client_connection_release() Vladimir Sementsov-Ogievskiy
2021-06-11 14:28 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 17/32] nbd: move connection code from block/nbd to nbd/client-connection Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 18/32] nbd/client-connection: use QEMU_LOCK_GUARD Vladimir Sementsov-Ogievskiy
2021-06-11 14:31 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 19/32] nbd/client-connection: add possibility of negotiation Vladimir Sementsov-Ogievskiy
2021-06-11 15:07 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 20/32] nbd/client-connection: implement connection retry Vladimir Sementsov-Ogievskiy
2021-06-11 15:12 ` Eric Blake
2021-11-22 16:30 ` Eric Blake
2021-11-22 17:17 ` Vladimir Sementsov-Ogievskiy
2021-11-22 21:51 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 21/32] nbd/client-connection: shutdown connection on release Vladimir Sementsov-Ogievskiy
2021-06-11 15:27 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 22/32] block/nbd: split nbd_handle_updated_info out of nbd_client_handshake() Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 23/32] block/nbd: use negotiation of NBDClientConnection Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 24/32] block/nbd: don't touch s->sioc in nbd_teardown_connection() Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 25/32] block/nbd: drop BDRVNBDState::sioc Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 26/32] nbd/client-connection: return only one io channel Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 27/32] block-coroutine-wrapper: allow non bdrv_ prefix Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 28/32] block/nbd: split nbd_co_do_establish_connection out of nbd_reconnect_attempt Vladimir Sementsov-Ogievskiy
2021-06-11 15:29 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 29/32] nbd/client-connection: add option for non-blocking connection attempt Vladimir Sementsov-Ogievskiy
2021-06-10 10:08 ` [PATCH v4 30/32] block/nbd: reuse nbd_co_do_establish_connection() in nbd_open() Vladimir Sementsov-Ogievskiy
2021-06-10 10:08 ` [PATCH v4 31/32] block/nbd: add nbd_client_connected() helper Vladimir Sementsov-Ogievskiy
2021-06-10 10:08 ` [PATCH v4 32/32] block/nbd: safer transition to receiving request Vladimir Sementsov-Ogievskiy
2021-06-11 15:55 ` [PATCH v4 00/32] block/nbd: rework client connection Eric Blake
2021-06-11 17:23 ` 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=20210610100802.5888-4-vsementsov@virtuozzo.com \
--to=vsementsov@virtuozzo.com \
--cc=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=rvkagan@yandex-team.ru \
/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.