Re: [Qemu-devel] [for-2.9 7/8] nbd: Tidy up blockdev-add interface

[Max Reitz]

On 29.03.2017 18:45, Markus Armbruster wrote:
> SocketAddress is a simple union, and simple unions are awkward: they
> have their variant members wrapped in a "data" object on the wire, and
> require additional indirections in C.  I intend to limit its use to
> existing external interfaces, and convert all internal interfaces to
> SocketAddressFlat.
> 
> BlockdevOptionsNbd is an external interface using SocketAddress, but
> it's new in 2.9.  Replace it by SocketAddressFlat while we can.  This
> simplifies
> 
>     { "execute": "blockdev-add",
>       "arguments": { "node-name": "foo", "driver": "nbd",
>                      "server": { "type": "inet",
>                                "data": { "host": "localhost",
>                                          "port": "12345" } } } }
> 
> to
> 
>     { "execute": "blockdev-add",
>       "arguments": { "node-name": "foo", "driver": "nbd",
>                      "server": { "type": "inet",
>                                "host": "localhost", "port": "12345" } } }
> 
> Since the internal interfaces still take SocketAddress, this requires
> conversion function socket_address_crumple().  It'll go away when I
> update the interfaces.
> 
> Unfortunately, SocketAddress is also visible in -drive since 2.8.  Add
> still more gunk to nbd_process_legacy_socket_options().  You can now
> shorten
> 
>     -drive 
> if=none,driver=nbd,server.type=inet,server.data.host=127.0.0.1,server.data.port=12345
> 
> to
> 
>     -drive 
> if=none,driver=nbd,server.type=inet,server.host=127.0.0.1,server.port=12345
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  block/nbd.c          | 131 
> ++++++++++++++++++++++++++++++++++++++++-----------
>  qapi/block-core.json |   2 +-
>  2 files changed, 104 insertions(+), 29 deletions(-)

To be completely frank: If there is any drawback to using
SocketAddressFlat instead of SocketAdress, I would be against using it.
Less indirections in C is an argument, but less {} on the wire is not,
in my opinion. QMP is a machine interface and machines don't care about
that additional level.

Therefore, if you think this somehow affects compatibility, I would not
want to do this change.

I personally agree with Paolo that we can just break what we have, and
in addition I think that we *should* break what we have or we should not
do this change at all.

Max

signature.asc
Description: OpenPGP digital signature