Re: [RFC PATCH 18/22] block/export: Add 'id' option to block-export-add

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1563 bytes --]

On 13.08.20 18:29, Kevin Wolf wrote:
> We'll need an id to identify block exports in monitor commands. This
> adds one.
> 
> Note that this is different from the 'name' option in the NBD server,
> which is the externally visible export name. While block export ids need
> to be unique in the whole process, export names must be unique only for
> the same server. Different export types or (potentially in the future)
> multiple NBD servers can have the same export name externally, but still
> need different block export ids internally.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/block-export.json |  3 +++
>  include/block/export.h |  3 +++
>  block/export/export.c  | 27 +++++++++++++++++++++++++++
>  qemu-nbd.c             |  1 +
>  4 files changed, 34 insertions(+)

Looks good, just one thing:

> diff --git a/block/export/export.c b/block/export/export.c
> index 675db9a8b9..72f1fab975 100644
> --- a/block/export/export.c
> +++ b/block/export/export.c

[...]

> @@ -144,6 +170,7 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
>      BlockExportOptions *export_opts = g_new(BlockExportOptions, 1);
>      *export_opts = (BlockExportOptions) {
>          .type                   = BLOCK_EXPORT_TYPE_NBD,
> +        .id                     = g_strdup(arg->name ?: arg->device),

Maybe this behavior should be documented for nbd-server-add?

>          .device                 = g_strdup(arg->device),
>          .u.nbd = {
>              .has_name           = arg->has_name,


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]