Re: [RFC PATCH 15/22] block/export: Move device to BlockExportOptions

[Max Reitz]

On 13.08.20 18:29, Kevin Wolf wrote:
> Every block export needs a block node to export, so move the 'device'
> option from BlockExportOptionsNbd to BlockExportOptions.
> 
> To maintain compatibility in nbd-server-add, BlockExportOptionsNbd needs
> to be wrapped by a new type NbdServerAddOptions that adds 'device' back
> because nbd-server-add doesn't use the BlockExportOptions base type.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/block-export.json         | 27 +++++++++++++++++++++------
>  block/export/export.c          | 26 ++++++++++++++++++++------
>  block/monitor/block-hmp-cmds.c |  6 +++---
>  blockdev-nbd.c                 |  4 ++--
>  qemu-nbd.c                     |  2 +-
>  5 files changed, 47 insertions(+), 18 deletions(-)

(Code diff looks good, just a question on the interface:)

> diff --git a/qapi/block-export.json b/qapi/block-export.json
> index 4ce163411f..d68f3bf87e 100644
> --- a/qapi/block-export.json
> +++ b/qapi/block-export.json

[...]

> @@ -167,6 +179,8 @@
>  # Describes a block export, i.e. how single node should be exported on an
>  # external interface.
>  #
> +# @device: The device name or node name of the node to be exported
> +#

Wouldn’t it be better to restrict ourselves to a node name here?
(Bluntly ignoring the fact that doing so would make this patch an
incompatible change, which would require some reordering in this series,
unless we decide to just ignore that intra-series incompatibility.)

OTOH...  What does “better” mean.  It won’t hurt anyone to keep this as
@device.  It’s just that I feel like if we had no legacy burden and did
this all from scratch, we would just make it @node-name.

Did you deliberately decide against @node-name?

Max

signature.asc
Description: OpenPGP digital signature