Prepare for the next patch, so that the diff is less confusing.
nbd_client_connecting is moved closer to the definition point.
nbd_client_connecting_wait() is kept only for the reconnection
logic; when it is used to check if a request has to be reissued,
use the renamed function nbd_client_will_reconnect(). In the
next patch, the two cases will have different locking requirements.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/nbd.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index a2414566d1..37d466e435 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -254,18 +254,15 @@ static void open_timer_init(BDRVNBDState *s, uint64_t expire_time_ns)
timer_mod(s->open_timer, expire_time_ns);
}
-static bool nbd_client_connecting(BDRVNBDState *s)
-{
- NBDClientState state = qatomic_load_acquire(&s->state);
- return state == NBD_CLIENT_CONNECTING_WAIT ||
- state == NBD_CLIENT_CONNECTING_NOWAIT;
-}
-
static bool nbd_client_connecting_wait(BDRVNBDState *s)
{
return qatomic_load_acquire(&s->state) == NBD_CLIENT_CONNECTING_WAIT;
}
+static bool nbd_client_will_reconnect(BDRVNBDState *s)
+{
+ return qatomic_load_acquire(&s->state) == NBD_CLIENT_CONNECTING_WAIT;
+}
/*
* Update @bs with information learned during a completed negotiation process.
* Return failure if the server's advertised options are incompatible with the
@@ -355,6 +352,13 @@ int coroutine_fn nbd_co_do_establish_connection(BlockDriverState *bs,
return 0;
}
+static bool nbd_client_connecting(BDRVNBDState *s)
+{
+ NBDClientState state = qatomic_load_acquire(&s->state);
+ return state == NBD_CLIENT_CONNECTING_WAIT ||
+ state == NBD_CLIENT_CONNECTING_NOWAIT;
+}
+
/* Called with s->requests_lock taken. */
static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
{
@@ -1190,7 +1194,7 @@ static int coroutine_fn nbd_co_request(BlockDriverState *bs, NBDRequest *request
error_free(local_err);
local_err = NULL;
}
- } while (ret < 0 && nbd_client_connecting_wait(s));
+ } while (ret < 0 && nbd_client_will_reconnect(s));
return ret ? ret : request_ret;
}
@@ -1249,7 +1253,7 @@ static int coroutine_fn nbd_client_co_preadv(BlockDriverState *bs, int64_t offse
error_free(local_err);
local_err = NULL;
}
- } while (ret < 0 && nbd_client_connecting_wait(s));
+ } while (ret < 0 && nbd_client_will_reconnect(s));
return ret ? ret : request_ret;
}
@@ -1407,7 +1411,7 @@ static int coroutine_fn nbd_client_co_block_status(
error_free(local_err);
local_err = NULL;
}
- } while (ret < 0 && nbd_client_connecting_wait(s));
+ } while (ret < 0 && nbd_client_will_reconnect(s));
if (ret < 0 || request_ret < 0) {
return ret ? ret : request_ret;
--
2.35.114.04.2022 20:57, Paolo Bonzini wrote:
> Prepare for the next patch, so that the diff is less confusing.
>
> nbd_client_connecting is moved closer to the definition point.
Amm. To usage-point you mean?
The original idea was to keep simple state-reading helpers definitions together :)
>
> nbd_client_connecting_wait() is kept only for the reconnection
> logic; when it is used to check if a request has to be reissued,
> use the renamed function nbd_client_will_reconnect(). In the
> next patch, the two cases will have different locking requirements.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
> block/nbd.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index a2414566d1..37d466e435 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -254,18 +254,15 @@ static void open_timer_init(BDRVNBDState *s, uint64_t expire_time_ns)
> timer_mod(s->open_timer, expire_time_ns);
> }
>
> -static bool nbd_client_connecting(BDRVNBDState *s)
> -{
> - NBDClientState state = qatomic_load_acquire(&s->state);
> - return state == NBD_CLIENT_CONNECTING_WAIT ||
> - state == NBD_CLIENT_CONNECTING_NOWAIT;
> -}
> -
> static bool nbd_client_connecting_wait(BDRVNBDState *s)
> {
> return qatomic_load_acquire(&s->state) == NBD_CLIENT_CONNECTING_WAIT;
> }
>
> +static bool nbd_client_will_reconnect(BDRVNBDState *s)
> +{
> + return qatomic_load_acquire(&s->state) == NBD_CLIENT_CONNECTING_WAIT;
> +}
> /*
> * Update @bs with information learned during a completed negotiation process.
> * Return failure if the server's advertised options are incompatible with the
> @@ -355,6 +352,13 @@ int coroutine_fn nbd_co_do_establish_connection(BlockDriverState *bs,
> return 0;
> }
>
> +static bool nbd_client_connecting(BDRVNBDState *s)
> +{
> + NBDClientState state = qatomic_load_acquire(&s->state);
> + return state == NBD_CLIENT_CONNECTING_WAIT ||
> + state == NBD_CLIENT_CONNECTING_NOWAIT;
> +}
> +
> /* Called with s->requests_lock taken. */
> static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
> {
> @@ -1190,7 +1194,7 @@ static int coroutine_fn nbd_co_request(BlockDriverState *bs, NBDRequest *request
> error_free(local_err);
> local_err = NULL;
> }
> - } while (ret < 0 && nbd_client_connecting_wait(s));
> + } while (ret < 0 && nbd_client_will_reconnect(s));
>
> return ret ? ret : request_ret;
> }
> @@ -1249,7 +1253,7 @@ static int coroutine_fn nbd_client_co_preadv(BlockDriverState *bs, int64_t offse
> error_free(local_err);
> local_err = NULL;
> }
> - } while (ret < 0 && nbd_client_connecting_wait(s));
> + } while (ret < 0 && nbd_client_will_reconnect(s));
>
> return ret ? ret : request_ret;
> }
> @@ -1407,7 +1411,7 @@ static int coroutine_fn nbd_client_co_block_status(
> error_free(local_err);
> local_err = NULL;
> }
> - } while (ret < 0 && nbd_client_connecting_wait(s));
> + } while (ret < 0 && nbd_client_will_reconnect(s));
>
> if (ret < 0 || request_ret < 0) {
> return ret ? ret : request_ret;
--
Best regards,
Vladimir
On 4/16/22 14:37, Vladimir Sementsov-Ogievskiy wrote:
> 14.04.2022 20:57, Paolo Bonzini wrote:
>> Prepare for the next patch, so that the diff is less confusing.
>>
>> nbd_client_connecting is moved closer to the definition point.
>
> Amm. To usage-point you mean?
> The original idea was to keep simple state-reading helpers definitions
> together :)
Yes and it makes sense. The new idea is to keep requests_lock functions
together instead. :)
Paolo
>>
>> nbd_client_connecting_wait() is kept only for the reconnection
>> logic; when it is used to check if a request has to be reissued,
>> use the renamed function nbd_client_will_reconnect(). In the
>> next patch, the two cases will have different locking requirements.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
>
>> ---
>> block/nbd.c | 24 ++++++++++++++----------
>> 1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index a2414566d1..37d466e435 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -254,18 +254,15 @@ static void open_timer_init(BDRVNBDState *s,
>> uint64_t expire_time_ns)
>> timer_mod(s->open_timer, expire_time_ns);
>> }
>> -static bool nbd_client_connecting(BDRVNBDState *s)
>> -{
>> - NBDClientState state = qatomic_load_acquire(&s->state);
>> - return state == NBD_CLIENT_CONNECTING_WAIT ||
>> - state == NBD_CLIENT_CONNECTING_NOWAIT;
>> -}
>> -
>> static bool nbd_client_connecting_wait(BDRVNBDState *s)
>> {
>> return qatomic_load_acquire(&s->state) ==
>> NBD_CLIENT_CONNECTING_WAIT;
>> }
>> +static bool nbd_client_will_reconnect(BDRVNBDState *s)
>> +{
>> + return qatomic_load_acquire(&s->state) ==
>> NBD_CLIENT_CONNECTING_WAIT;
>> +}
>> /*
>> * Update @bs with information learned during a completed
>> negotiation process.
>> * Return failure if the server's advertised options are
>> incompatible with the
>> @@ -355,6 +352,13 @@ int coroutine_fn
>> nbd_co_do_establish_connection(BlockDriverState *bs,
>> return 0;
>> }
>> +static bool nbd_client_connecting(BDRVNBDState *s)
>> +{
>> + NBDClientState state = qatomic_load_acquire(&s->state);
>> + return state == NBD_CLIENT_CONNECTING_WAIT ||
>> + state == NBD_CLIENT_CONNECTING_NOWAIT;
>> +}
>> +
>> /* Called with s->requests_lock taken. */
>> static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
>> {
>> @@ -1190,7 +1194,7 @@ static int coroutine_fn
>> nbd_co_request(BlockDriverState *bs, NBDRequest *request
>> error_free(local_err);
>> local_err = NULL;
>> }
>> - } while (ret < 0 && nbd_client_connecting_wait(s));
>> + } while (ret < 0 && nbd_client_will_reconnect(s));
>> return ret ? ret : request_ret;
>> }
>> @@ -1249,7 +1253,7 @@ static int coroutine_fn
>> nbd_client_co_preadv(BlockDriverState *bs, int64_t offse
>> error_free(local_err);
>> local_err = NULL;
>> }
>> - } while (ret < 0 && nbd_client_connecting_wait(s));
>> + } while (ret < 0 && nbd_client_will_reconnect(s));
>> return ret ? ret : request_ret;
>> }
>> @@ -1407,7 +1411,7 @@ static int coroutine_fn nbd_client_co_block_status(
>> error_free(local_err);
>> local_err = NULL;
>> }
>> - } while (ret < 0 && nbd_client_connecting_wait(s));
>> + } while (ret < 0 && nbd_client_will_reconnect(s));
>> if (ret < 0 || request_ret < 0) {
>> return ret ? ret : request_ret;
>
>
On Thu, Apr 14, 2022 at 07:57:53PM +0200, Paolo Bonzini wrote: > Prepare for the next patch, so that the diff is less confusing. > > nbd_client_connecting is moved closer to the definition point. > > nbd_client_connecting_wait() is kept only for the reconnection > logic; when it is used to check if a request has to be reissued, > use the renamed function nbd_client_will_reconnect(). In the > next patch, the two cases will have different locking requirements. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block/nbd.c | 24 ++++++++++++++---------- > 1 file changed, 14 insertions(+), 10 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> Yes, this split makes the next patch easier to read ;) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
© 2016 - 2026 Red Hat, Inc.