On Fri, Apr 16, 2021 at 11:08:57AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> To be reused in the following patch.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> ---
> block/nbd.c | 99 ++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 57 insertions(+), 42 deletions(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index 5e63caaf4b..03ffe95231 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -318,6 +318,50 @@ static bool nbd_client_connecting_wait(BDRVNBDState *s)
> return qatomic_load_acquire(&s->state) == NBD_CLIENT_CONNECTING_WAIT;
> }
>
> +/*
> + * Check s->info updated by negotiation process.
The parameter name is bs, not s; so this comment is a bit confusing...
> + * Update @bs correspondingly to new options.
> + */
> +static int nbd_handle_updated_info(BlockDriverState *bs, Error **errp)
> +{
> + BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
...until here. Maybe rewrite the entire comment as:
Update @bs with information learned during a completed negotiation
process. Return failure if the server's advertised options are
incompatible with the client's needs.
> + int ret;
> +
> + if (s->x_dirty_bitmap) {
> + if (!s->info.base_allocation) {
> + error_setg(errp, "requested x-dirty-bitmap %s not found",
> + s->x_dirty_bitmap);
> + return -EINVAL;
> + }
> + if (strcmp(s->x_dirty_bitmap, "qemu:allocation-depth") == 0) {
> + s->alloc_depth = true;
> + }
> + }
> +
> + if (s->info.flags & NBD_FLAG_READ_ONLY) {
> + ret = bdrv_apply_auto_read_only(bs, "NBD export is read-only", errp);
> + if (ret < 0) {
> + return ret;
> + }
> + }
> +
> + if (s->info.flags & NBD_FLAG_SEND_FUA) {
> + bs->supported_write_flags = BDRV_REQ_FUA;
> + bs->supported_zero_flags |= BDRV_REQ_FUA;
Code motion, so it is correct, but it looks odd to use = for one
assignment and |= for the other. Using |= in both places would be
more consistent.
> + }
> +
> + if (s->info.flags & NBD_FLAG_SEND_WRITE_ZEROES) {
> + bs->supported_zero_flags |= BDRV_REQ_MAY_UNMAP;
> + if (s->info.flags & NBD_FLAG_SEND_FAST_ZERO) {
> + bs->supported_zero_flags |= BDRV_REQ_NO_FALLBACK;
> + }
> + }
> +
> + trace_nbd_client_handshake_success(s->export);
> +
> + return 0;
> +}
> +
> static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
> {
> int ret;
> @@ -1579,49 +1623,13 @@ static int nbd_client_handshake(BlockDriverState *bs,
> Error **errp)
As updating the comment doesn't affect code correctness,
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
