Re: [Qemu-devel] [PULL 10/21] migration: Create socket-address parameter

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 3/5/19 12:15 PM, Dr. David Alan Gilbert (git) wrote:
>> From: Juan Quintela <quintela@redhat.com>
>>
>> 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.
>>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>
>> --
>>
>
> This was not the usual '---' divider, and hence:
>
>> This used to be uri parameter, but it has so many troubles to
>> reproduce that it don't just make sense.
>>
>> This used to be a port parameter.  I was asked to move to
>> SocketAddress, done.
>> I also merged the setting of the migration tcp port in this one
>> because now I need to free the address, and this makes it easier.
>> This used to be x-socket-address with a single direction, now it is a
>> list of addresses.
>> Move SocketAddress_to_str here.  I used to try to generalize the one
>> in chardev/char-socket.c, but it is not worth it.
>>
>> Free string (eric)
>> Handle VSOCK address nicely (not that migration can use them yet).
>> Remove useless breaks (dave)
>> rename socket_address to socket_address_list to avoid confusion
>> Update to 4.0 (eric)
>> Put a comment indicating that there is a problem on the qapi
>> generator (markus).
>
> ...all this got included, even if it was perhaps not intended.
>
>> Message-Id: <20190227105128.1655-3-quintela@redhat.com>
>>
>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> At any rate, it certainly looks odd to split *-by: tags by so much text.
>
>> +++ b/qapi/sockets.json
>> @@ -152,3 +152,21 @@
>>               'unix': 'UnixSocketAddress',
>>               'vsock': 'VsockSocketAddress',
>>               'fd': 'String' } }
>> +
>> +##
>> +# @DummyStruct:
>> +#
>> +# Both block-core and migration needs SocketAddressList
>> +# I am open to comments about how to share it
>> +#
>> +# @dummy-list: A dummy list
>> +#
>> +# FIXME: This shouldn't be needed, but this struct has two users, and
>> +# current qapi generator generates it on the 1st place that uses it,
>> +# so the second user don't see it.  Putting it here it is seen in both
>> +# sides.
>
> If Markus' pull request lands first, we don't need this.
> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg01185.html

It did: merge commit a3e3b0a7bd5.  Please drop this hunk, either in a
respin or in a follow-up patch.

A respin would give you a chance to clean up the commit message.