Re: [Qemu-devel] [PATCH 5/6] migration: Now set the migration uri

[Daniel P. Berrange]

On Wed, Nov 22, 2017 at 12:54:58PM +0000, Daniel P. Berrange wrote:
> On Wed, Nov 22, 2017 at 01:29:57PM +0100, Juan Quintela wrote:
> > "Daniel P. Berrange" <address@hidden> wrote:
> > > On Mon, Oct 30, 2017 at 12:21:11PM +0100, Juan Quintela wrote:
> > >> Signed-off-by: Juan Quintela <address@hidden>
> > >> ---
> > >>  migration/migration.c | 25 ++++++++++++++++++-------
> > >>  migration/migration.h |  2 ++
> > >>  migration/socket.c    |  7 +++++++
> > >>  3 files changed, 27 insertions(+), 7 deletions(-)
> > >
> > >
> > >> diff --git a/migration/socket.c b/migration/socket.c
> > >> index 3a8232dd2d..c3ab81d1fb 100644
> > >> --- a/migration/socket.c
> > >> +++ b/migration/socket.c
> > >> @@ -187,7 +187,14 @@ void tcp_start_incoming_migration(const char 
> > >> *host_port, Error **errp)
> > >>      Error *err = NULL;
> > >>      SocketAddress *saddr = tcp_build_address(host_port, &err);
> > >>      if (!err) {
> > >> +        char *new_uri;
> > >>          socket_start_incoming_migration(saddr, &err);
> > >> +        if (!err) {
> > >> +            new_uri = g_strdup_printf("tcp:%s:%s", saddr->u.inet.host,
> > >> +                                      saddr->u.inet.port);
> > >
> > > This is bad as it is throwing away data that the original URI had. In 
> > > particular
> > > you loose the 'ipv4=on|off' and 'ipv6=on|off' flags. If you need to keep 
> > > the
> > > original URI for later, then why not just keep the 'host_port' parameter 
> > > that
> > > was passed into this function instead of trying to reverse engineeer the 
> > > URI ?
> > 
> > I don't need the original uri anymore, this is the incoming side of
> > migration, and we can only set that once, if migration fails, we need to
> > restart qemu anymore.
> > 
> > I changed it to this:
> > 
> >             new_uri = g_strdup_printf("tcp:%s:%s%s%s", address->u.inet.host,
> >                                       address->u.inet.port,
> >                                       iaddr->has_ipv4 ? ",ipv4" : "",
> >                                       iaddr->has_ipv6 ? ",ipv6" : "");
> > 
> > 
> > Clearly, we don't care about the to= parameter.
> > 
> > The whole point of this exercise is that in destination, we use
> > 
> > -incoming tcp:0:0
> > 
> > and with the output of info migrate_parameters (uri) we are able to
> > migrate to that place.  For rest of migration posibilities, there is no
> > changes at all.
> 
> That doesn't work typically. When the incoming QEMU listens for connections,
> by default it will listen on 0.0.0.0, or ::, so that's what the address
> you're creating will contain.  You can't use 0.0.0.0 or :: in a connect()
> call on the other side as that's a wildcard address that refers to "any 
> valid IP address on the current host".
> 
> When you connect to the listening QEMU you need the actual IP address
> of one (of the possibly many) NICs on the target host.  IOW, the only
> time the listen address is useful is if you have told QEMU to listen on
> a specific NIC's IP address, instead of the wildcard address.
> 
> This is the core reason why libvirt calls 'gethostname()' on the target
> host to identify the primary IP address of the host, and uses that on the
> source host to establish the connection.

I should have said the port number info will still be useful (if you're
using the dynamic port allocation), it is just the IP address that's not
usable in general.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|