qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Max Reitz
Subject: Re: [RFC PATCH 15/22] block/export: Move device to BlockExportOptions
Date: Mon, 17 Aug 2020 17:13:37 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]