nbd: keep send_mutex/free_sema handling outside nbd_co_do_establish_connection
Elevate s->in_flight early so that other incoming requests will wait on the CoQueue in nbd_co_send_request; restart them after getting back from nbd_reconnect_attempt. This could be after the reconnect timer or nbd_cancel_in_flight have cancelled the attempt, so there is no need anymore to cancel the requests there. nbd_co_send_request now handles both stopping and restarting pending requests after a successful connection, and there is no need to hold send_mutex in nbd_co_do_establish_connection. The current setup is confusing because nbd_co_do_establish_connection is called both with send_mutex taken and without it. Before the patch it uses free_sema which (at least in theory...) is protected by send_mutex, after the patch it does not anymore. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20220414175756.671165-5-pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: wrap long line] Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org> Reviewed-by: Lukas Straub <lukasstraub2@web.de> Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
parent
172f5f1a40
commit
8610b4491f
@ -59,7 +59,8 @@ int coroutine_fn bdrv_co_writev_vmstate(BlockDriverState *bs,
|
|||||||
QEMUIOVector *qiov, int64_t pos);
|
QEMUIOVector *qiov, int64_t pos);
|
||||||
|
|
||||||
int coroutine_fn
|
int coroutine_fn
|
||||||
nbd_co_do_establish_connection(BlockDriverState *bs, Error **errp);
|
nbd_co_do_establish_connection(BlockDriverState *bs, bool blocking,
|
||||||
|
Error **errp);
|
||||||
|
|
||||||
|
|
||||||
int coroutine_fn
|
int coroutine_fn
|
||||||
@ -109,7 +110,7 @@ bdrv_common_block_status_above(BlockDriverState *bs,
|
|||||||
BlockDriverState **file,
|
BlockDriverState **file,
|
||||||
int *depth);
|
int *depth);
|
||||||
int generated_co_wrapper
|
int generated_co_wrapper
|
||||||
nbd_do_establish_connection(BlockDriverState *bs, Error **errp);
|
nbd_do_establish_connection(BlockDriverState *bs, bool blocking, Error **errp);
|
||||||
|
|
||||||
int generated_co_wrapper
|
int generated_co_wrapper
|
||||||
blk_do_preadv(BlockBackend *blk, int64_t offset, int64_t bytes,
|
blk_do_preadv(BlockBackend *blk, int64_t offset, int64_t bytes,
|
||||||
|
66
block/nbd.c
66
block/nbd.c
@ -187,9 +187,6 @@ static void reconnect_delay_timer_cb(void *opaque)
|
|||||||
if (qatomic_load_acquire(&s->state) == NBD_CLIENT_CONNECTING_WAIT) {
|
if (qatomic_load_acquire(&s->state) == NBD_CLIENT_CONNECTING_WAIT) {
|
||||||
s->state = NBD_CLIENT_CONNECTING_NOWAIT;
|
s->state = NBD_CLIENT_CONNECTING_NOWAIT;
|
||||||
nbd_co_establish_connection_cancel(s->conn);
|
nbd_co_establish_connection_cancel(s->conn);
|
||||||
while (qemu_co_enter_next(&s->free_sema, NULL)) {
|
|
||||||
/* Resume all queued requests */
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
reconnect_delay_timer_del(s);
|
reconnect_delay_timer_del(s);
|
||||||
@ -310,11 +307,10 @@ static int nbd_handle_updated_info(BlockDriverState *bs, Error **errp)
|
|||||||
}
|
}
|
||||||
|
|
||||||
int coroutine_fn nbd_co_do_establish_connection(BlockDriverState *bs,
|
int coroutine_fn nbd_co_do_establish_connection(BlockDriverState *bs,
|
||||||
Error **errp)
|
bool blocking, Error **errp)
|
||||||
{
|
{
|
||||||
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
|
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
|
||||||
int ret;
|
int ret;
|
||||||
bool blocking = nbd_client_connecting_wait(s);
|
|
||||||
IO_CODE();
|
IO_CODE();
|
||||||
|
|
||||||
assert(!s->ioc);
|
assert(!s->ioc);
|
||||||
@ -350,7 +346,6 @@ int coroutine_fn nbd_co_do_establish_connection(BlockDriverState *bs,
|
|||||||
|
|
||||||
/* successfully connected */
|
/* successfully connected */
|
||||||
s->state = NBD_CLIENT_CONNECTED;
|
s->state = NBD_CLIENT_CONNECTED;
|
||||||
qemu_co_queue_restart_all(&s->free_sema);
|
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
@ -358,25 +353,25 @@ int coroutine_fn nbd_co_do_establish_connection(BlockDriverState *bs,
|
|||||||
/* called under s->send_mutex */
|
/* called under s->send_mutex */
|
||||||
static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
|
static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
|
||||||
{
|
{
|
||||||
assert(nbd_client_connecting(s));
|
bool blocking = nbd_client_connecting_wait(s);
|
||||||
assert(s->in_flight == 0);
|
|
||||||
|
|
||||||
if (nbd_client_connecting_wait(s) && s->reconnect_delay &&
|
|
||||||
!s->reconnect_delay_timer)
|
|
||||||
{
|
|
||||||
/*
|
|
||||||
* It's first reconnect attempt after switching to
|
|
||||||
* NBD_CLIENT_CONNECTING_WAIT
|
|
||||||
*/
|
|
||||||
reconnect_delay_timer_init(s,
|
|
||||||
qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
|
|
||||||
s->reconnect_delay * NANOSECONDS_PER_SECOND);
|
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Now we are sure that nobody is accessing the channel, and no one will
|
* Now we are sure that nobody is accessing the channel, and no one will
|
||||||
* try until we set the state to CONNECTED.
|
* try until we set the state to CONNECTED.
|
||||||
*/
|
*/
|
||||||
|
assert(nbd_client_connecting(s));
|
||||||
|
assert(s->in_flight == 1);
|
||||||
|
|
||||||
|
if (blocking && !s->reconnect_delay_timer) {
|
||||||
|
/*
|
||||||
|
* It's the first reconnect attempt after switching to
|
||||||
|
* NBD_CLIENT_CONNECTING_WAIT
|
||||||
|
*/
|
||||||
|
g_assert(s->reconnect_delay);
|
||||||
|
reconnect_delay_timer_init(s,
|
||||||
|
qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
|
||||||
|
s->reconnect_delay * NANOSECONDS_PER_SECOND);
|
||||||
|
}
|
||||||
|
|
||||||
/* Finalize previous connection if any */
|
/* Finalize previous connection if any */
|
||||||
if (s->ioc) {
|
if (s->ioc) {
|
||||||
@ -387,7 +382,9 @@ static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
|
|||||||
s->ioc = NULL;
|
s->ioc = NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
nbd_co_do_establish_connection(s->bs, NULL);
|
qemu_co_mutex_unlock(&s->send_mutex);
|
||||||
|
nbd_co_do_establish_connection(s->bs, blocking, NULL);
|
||||||
|
qemu_co_mutex_lock(&s->send_mutex);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* The reconnect attempt is done (maybe successfully, maybe not), so
|
* The reconnect attempt is done (maybe successfully, maybe not), so
|
||||||
@ -472,21 +469,21 @@ static int coroutine_fn nbd_co_send_request(BlockDriverState *bs,
|
|||||||
qemu_co_mutex_lock(&s->send_mutex);
|
qemu_co_mutex_lock(&s->send_mutex);
|
||||||
|
|
||||||
while (s->in_flight == MAX_NBD_REQUESTS ||
|
while (s->in_flight == MAX_NBD_REQUESTS ||
|
||||||
(!nbd_client_connected(s) && s->in_flight > 0))
|
(!nbd_client_connected(s) && s->in_flight > 0)) {
|
||||||
{
|
|
||||||
qemu_co_queue_wait(&s->free_sema, &s->send_mutex);
|
qemu_co_queue_wait(&s->free_sema, &s->send_mutex);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (nbd_client_connecting(s)) {
|
|
||||||
nbd_reconnect_attempt(s);
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!nbd_client_connected(s)) {
|
|
||||||
rc = -EIO;
|
|
||||||
goto err;
|
|
||||||
}
|
|
||||||
|
|
||||||
s->in_flight++;
|
s->in_flight++;
|
||||||
|
if (!nbd_client_connected(s)) {
|
||||||
|
if (nbd_client_connecting(s)) {
|
||||||
|
nbd_reconnect_attempt(s);
|
||||||
|
qemu_co_queue_restart_all(&s->free_sema);
|
||||||
|
}
|
||||||
|
if (!nbd_client_connected(s)) {
|
||||||
|
rc = -EIO;
|
||||||
|
goto err;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
for (i = 0; i < MAX_NBD_REQUESTS; i++) {
|
for (i = 0; i < MAX_NBD_REQUESTS; i++) {
|
||||||
if (s->requests[i].coroutine == NULL) {
|
if (s->requests[i].coroutine == NULL) {
|
||||||
@ -526,8 +523,8 @@ err:
|
|||||||
nbd_channel_error(s, rc);
|
nbd_channel_error(s, rc);
|
||||||
if (i != -1) {
|
if (i != -1) {
|
||||||
s->requests[i].coroutine = NULL;
|
s->requests[i].coroutine = NULL;
|
||||||
s->in_flight--;
|
|
||||||
}
|
}
|
||||||
|
s->in_flight--;
|
||||||
qemu_co_queue_next(&s->free_sema);
|
qemu_co_queue_next(&s->free_sema);
|
||||||
}
|
}
|
||||||
qemu_co_mutex_unlock(&s->send_mutex);
|
qemu_co_mutex_unlock(&s->send_mutex);
|
||||||
@ -1882,7 +1879,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
|
|||||||
}
|
}
|
||||||
|
|
||||||
s->state = NBD_CLIENT_CONNECTING_WAIT;
|
s->state = NBD_CLIENT_CONNECTING_WAIT;
|
||||||
ret = nbd_do_establish_connection(bs, errp);
|
ret = nbd_do_establish_connection(bs, true, errp);
|
||||||
if (ret < 0) {
|
if (ret < 0) {
|
||||||
goto fail;
|
goto fail;
|
||||||
}
|
}
|
||||||
@ -2048,7 +2045,6 @@ static void nbd_cancel_in_flight(BlockDriverState *bs)
|
|||||||
|
|
||||||
if (s->state == NBD_CLIENT_CONNECTING_WAIT) {
|
if (s->state == NBD_CLIENT_CONNECTING_WAIT) {
|
||||||
s->state = NBD_CLIENT_CONNECTING_NOWAIT;
|
s->state = NBD_CLIENT_CONNECTING_NOWAIT;
|
||||||
qemu_co_queue_restart_all(&s->free_sema);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
nbd_co_establish_connection_cancel(s->conn);
|
nbd_co_establish_connection_cancel(s->conn);
|
||||||
|
Loading…
Reference in New Issue
Block a user