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

[Dr. David Alan Gilbert]

* Markus Armbruster (address@hidden) wrote:
> Eric Blake <address@hidden> writes:
> 
> > On 3/5/19 12:15 PM, Dr. David Alan Gilbert (git) wrote:
> >> From: Juan Quintela <address@hidden>
> >>
> >> 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 <address@hidden>
> >> Signed-off-by: Juan Quintela <address@hidden>
> >>
> >> --
> >>
> >
> > 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: <address@hidden>
> >>
> >> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> >
> > 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.

OK, respin coming up.

Dave

--
Dr. David Alan Gilbert / address@hidden / Manchester, UK