Re: [Qemu-devel] [PATCH v9 1/3] migration: Create socket-address parameter

[Eric Blake]

On 05/09/2018 07:01 AM, Juan Quintela wrote:
> It will be used to store the uri parameters. We want this only for
> tcp, so we don't set it for other uris.  We need it to know what port
> is migration running.
>
> Signed-off-by: Juan Quintela <address@hidden>
>
> --

> +++ b/qapi/migration.json
> @@ -6,6 +6,7 @@
>   ##
>   
>   { 'include': 'common.json' }
> +{ 'include': 'sockets.json' }
>   
>   ##
>   # @MigrationStats:
> @@ -169,6 +170,7 @@
>   #           only present when the postcopy-blocktime migration capability
>   #           is enabled. (Since 2.13)
>   #
> +# @socket-address: Only used for tcp, to know what the real port is (Since 
> 2.13)

Maybe s/real port is/real ports are/, since...

>   #
>   # Since: 0.14.0
>   ##
> @@ -183,7 +185,8 @@
>              '*cpu-throttle-percentage': 'int',
>              '*error-desc': 'str',
>              '*postcopy-blocktime' : 'uint32',
> -           '*postcopy-vcpu-blocktime': ['uint32']} }
> +           '*postcopy-vcpu-blocktime': ['uint32'],
> +           '*socket-address': ['SocketAddress'] } }

...an array is potentially plural.

>   
>   ##
>   # @query-migrate:
> @@ -690,6 +693,7 @@
>   #                     needs to be a multiple of the target page size
>   #                     and a power of 2
>   #                     (Since 2.11)
> +#
>   # Since: 2.4
>   ##
>   { 'struct': 'MigrationParameters',

Spurious hunk?  Although it looks reasonable, it could be a separate 
trivial cleanup patch.

> diff --git a/qapi/sockets.json b/qapi/sockets.json
> index fc81d8d5e8..f1ca09a927 100644
> --- a/qapi/sockets.json
> +++ b/qapi/sockets.json
> @@ -152,3 +152,16 @@
>               'unix': 'UnixSocketAddress',
>               'vsock': 'VsockSocketAddress',
>               'fd': 'String' } }
> +
> +##
> +# @DummyStruct:
> +#
> +# Both block-core and migration needs SocketAddressList

s/needs/need/

> +# I am open to comments about how to share it

Since this is two sentences, trailing '.' would help.

> +#
> +# @dummy-list: A dummy list
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'DummyStruct',
> +  'data': { 'dummy-list': ['SocketAddress'] } }

We've used this idiom elsewhere; it might be better to amend 
DummyForceArrays in qapi/misc.json, except then misc.json might need to 
include sockets.json for the definition of SocketAddress.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org