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

[Juan Quintela]

Eric Blake <address@hidden> wrote:

>> +++ 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...

done.

>
>>   #
>>   # 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.

done. (didn't like to put so many s's, but ...)

>>     ##
>>   # @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.

Fixed thanks.

>
>> 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/

done

>> +# I am open to comments about how to share it
>
> Since this is two sentences, trailing '.' would help.

done

>> +#
>> +# @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.

Not done.  It needs more qapi magic that puttingthe socket.json include
there, so this one stays.  Putting it there means including
qapi/<vhateve>-misc.h in several places, I think it is easier this way
(famous last words).

Later, Juan.