Re: [PATCH v2 5/5] nbd: Add 'qemu-nbd -A' to expose allocation depth

[Vladimir Sementsov-Ogievskiy]

30.09.2020 15:11, Eric Blake wrote:
> Allow the server to expose an additional metacontext to be requested
> by savvy clients.  qemu-nbd adds a new option -A to expose the
> qemu:allocation-depth metacontext through NBD_CMD_BLOCK_STATUS; this
> can also be set via QMP when using block-export-add.
>
> qemu as client can be hacked into viewing this new context by using
> the now-misnamed x-dirty-bitmap option when creating an NBD blockdev
> (even though our x- naming means we could rename it, I did not think
> it worth breaking back-compat of tools that have been using it while
> waiting for a better solution).  It is worth noting the decoding of
> how such context information will appear in 'qemu-img map
> --output=json':
>
> NBD_STATE_DEPTH_UNALLOC => "zero":false, "data":true
> NBD_STATE_DEPTH_LOCAL   => "zero":false, "data":false
> NBD_STATE_DEPTH_BACKING => "zero":true,  "data":true
>
> libnbd as client is probably a nicer way to get at the information
> without having to decipher such hacks in qemu as client. ;)
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   docs/tools/qemu-nbd.rst    |  6 ++++
>   qapi/block-core.json       |  7 ++--
>   qapi/block-export.json     |  6 +++-
>   blockdev-nbd.c             |  2 ++
>   nbd/server.c               |  2 ++
>   qemu-nbd.c                 | 14 ++++++--
>   tests/qemu-iotests/309     | 73 ++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/309.out | 22 ++++++++++++
>   tests/qemu-iotests/group   |  1 +
>   9 files changed, 127 insertions(+), 6 deletions(-)
>   create mode 100755 tests/qemu-iotests/309
>   create mode 100644 tests/qemu-iotests/309.out
>
> diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst
> index 667861cb22e9..0e545a97cfa3 100644
> --- a/docs/tools/qemu-nbd.rst
> +++ b/docs/tools/qemu-nbd.rst
> @@ -72,6 +72,12 @@ driver options if ``--image-opts`` is specified.
>
>     Export the disk as read-only.
>
> +.. option:: -A, --allocation-depth
> +
> +  Expose allocation depth information via the
> +  ``qemu:allocation-depth`` context accessible through
> +  NBD_OPT_SET_META_CONTEXT.
> +
>   .. option:: -B, --bitmap=NAME
>
>     If *filename* has a qcow2 persistent bitmap *NAME*, expose
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index d620bd1302b2..0379eb992db8 100644


[..]

> +echo
> +echo "=== Check allocation over NBD ==="
> +echo
> +
> +# Normal -f raw NBD block status loses access to allocation information

^this comment is not for the next line, but for further lines ...

> +$QEMU_IMG map --output=json -f qcow2 "$TEST_IMG"
> +IMG="driver=nbd,server.type=unix,server.path=$nbd_unix_socket"
> +nbd_server_start_unix_socket -r -f qcow2 -A "$TEST_IMG"

... should it be here?

> +$QEMU_IMG map --output=json --image-opts \
> +    "$IMG" | _filter_qemu_img_map
> +# But since we used -A, and use x-dirty-bitmap as a hack for reading bitmaps,
> +# we can reconstruct it, by abusing block status to report:
> +#    NBD_STATE_DEPTH_UNALLOC => "zero":false, "data":true
> +#    NBD_STATE_DEPTH_LOCAL   => "zero":false, "data":false
> +#    NBD_STATE_DEPTH_BACKING => "zero":true,  "data":true
> +$QEMU_IMG map --output=json --image-opts \
> +    "$IMG,x-dirty-bitmap=qemu:allocation-depth" | _filter_qemu_img_map
> +
> +# success, all done
> +echo '*** done'


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

--
Best regards,
Vladimir