Re: [PATCH v2 2/2] nbd: Allow description when creating NBD blockdev

[Maxim Levitsky]

On Thu, 2019-10-10 at 16:00 -0500, Eric Blake wrote:
> Allow blockdevs to match the feature already present in qemu-nbd -D.
> Enhance iotest 223 to cover it.
> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
>  qapi/block.json            | 8 +++++---
>  blockdev-nbd.c             | 9 ++++++++-
>  monitor/hmp-cmds.c         | 4 ++--
>  tests/qemu-iotests/223     | 2 +-
>  tests/qemu-iotests/223.out | 1 +
>  5 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/qapi/block.json b/qapi/block.json
> index 145c268bb646..a6617b5bd03a 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -250,9 +250,11 @@
>  # @name: Export name. If unspecified, the @device parameter is used as the
>  #        export name. (Since 2.12)
>  #
> +# @description: Free-form description of the export. (Since 4.2)
> +#
>  # @writable: Whether clients should be able to write to the device via the
>  #     NBD connection (default false).
> -
> +#
>  # @bitmap: Also export the dirty bitmap reachable from @device, so the
>  #          NBD client can use NBD_OPT_SET_META_CONTEXT with
>  #          "qemu:dirty-bitmap:NAME" to inspect the bitmap. (since 4.0)
> @@ -263,8 +265,8 @@
>  # Since: 1.3.0
>  ##
>  { 'command': 'nbd-server-add',
> -  'data': {'device': 'str', '*name': 'str', '*writable': 'bool',
> -           '*bitmap': 'str' } }
> +  'data': {'device': 'str', '*name': 'str', '*description': 'str',
> +           '*writable': 'bool', '*bitmap': 'str' } }
> 
>  ##
>  # @NbdServerRemoveMode:
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 8c20baa4a4b9..de2f2ff71320 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -144,6 +144,7 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
>  }
> 
>  void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
> +                        bool has_description, const char *description,
>                          bool has_writable, bool writable,
>                          bool has_bitmap, const char *bitmap, Error **errp)
>  {
> @@ -167,6 +168,11 @@ void qmp_nbd_server_add(const char *device, bool 
> has_name, const char *name,
>          return;
>      }
> 
> +    if (has_description && strlen(description) > NBD_MAX_STRING_SIZE) {
> +        error_setg(errp, "description '%s' too long", description);
> +        return;
> +    }
> +
>      if (nbd_export_find(name)) {
>          error_setg(errp, "NBD server already has export named '%s'", name);
>          return;
> @@ -195,7 +201,8 @@ void qmp_nbd_server_add(const char *device, bool 
> has_name, const char *name,
>          writable = false;
>      }
> 
> -    exp = nbd_export_new(bs, 0, len, name, NULL, bitmap, !writable, 
> !writable,
> +    exp = nbd_export_new(bs, 0, len, name, description, bitmap,
> +                         !writable, !writable,
>                           NULL, false, on_eject_blk, errp);
>      if (!exp) {
>          goto out;
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index b2551c16d129..574c6321c9d0 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -2352,7 +2352,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict 
> *qdict)
>              continue;
>          }
> 
> -        qmp_nbd_server_add(info->value->device, false, NULL,
> +        qmp_nbd_server_add(info->value->device, false, NULL, false, NULL,
>                             true, writable, false, NULL, &local_err);
> 
>          if (local_err != NULL) {
> @@ -2374,7 +2374,7 @@ void hmp_nbd_server_add(Monitor *mon, const QDict 
> *qdict)
>      bool writable = qdict_get_try_bool(qdict, "writable", false);
>      Error *local_err = NULL;
> 
> -    qmp_nbd_server_add(device, !!name, name, true, writable,
> +    qmp_nbd_server_add(device, !!name, name, false, NULL, true, writable,
>                         false, NULL, &local_err);
>      hmp_handle_error(mon, &local_err);
>  }
> diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223
> index 2ba3d8124b4f..06bdc96be48f 100755
> --- a/tests/qemu-iotests/223
> +++ b/tests/qemu-iotests/223
> @@ -144,7 +144,7 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
>    "bitmap":"b3"}}' "error" # Missing bitmap
>  _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
>    "arguments":{"device":"n", "name":"n2", "writable":true,
> -  "bitmap":"b2"}}' "return"
> +  "description":"some text", "bitmap":"b2"}}' "return"
>  $QEMU_NBD_PROG -L -k "$TEST_DIR/nbd"
> 
>  echo
> diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
> index 23b34fcd202e..16d597585b4f 100644
> --- a/tests/qemu-iotests/223.out
> +++ b/tests/qemu-iotests/223.out
> @@ -49,6 +49,7 @@ exports available: 2
>     base:allocation
>     qemu:dirty-bitmap:b
>   export: 'n2'
> +  description: some text
>    size:  4194304
>    flags: 0xced ( flush fua trim zeroes df cache fast-zero )
>    min block: 1

Reviewed-by: Maxim Levitsky <address@hidden>

Best regards,
        Maxim Levitsky