Am 17.08.2020 um 17:13 hat Max Reitz geschrieben: > 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 <kw...@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.)
We already have intra-series incompatibility, so I wouldn't mind that. > 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? At first I thought I could still share code between nbd-server-add and block-export-add, but that's not the case. Then I guess the only other reason I have is consistency with other QMP commands. I won't pretend it's a strong one. Kevin
signature.asc
Description: PGP signature