Re: [Qemu-devel] [PATCH 1/1] Migration: libvirt live migration over RDMA of ipv6 addr failed

[David Z. Dai]

On Tue, 2017-01-24 at 17:36 +0000, Dr. David Alan Gilbert wrote:
> * David Dai (address@hidden) wrote:
> > Using libvirt to do live migration over RDMA via ip v6 address failed. 
> > For example:
> > # virsh migrate  --live --migrateuri rdma://[deba::2222]:49152  \
> >               rhel73_host1_guest1 qemu+ssh://[deba::2222]/system --verbose
> > address@hidden::2222's password:
> > error: internal error: unable to execute QEMU command 'migrate': RDMA 
> > ERROR:  
> >                   could not rdma_getaddrinfo address deba
> > 
> > As we can see, the ip v6 address used by rdma_getaddrinfo() has only "deba" 
> > part. It should be "deba::2222".
> > 
> > 1) According to rfc 3986, a literal ip v6 address should be enclosed 
> > in '[' and ']'.
> > When using virsh command to do live migration via ip v6 addresss, user
> > will input the ip v6 address with brackets (i.e. rdma://[deba::2222]:49152).
> > libvirt will parse command line option by calling virURIParse(). 
> > Inside it calls virStringStripIPv6Brackets() to strip off the brackets.
> > The uri passed in to virURIParse()  is:
> >    "uri = rdma://[deba::2222]:49152"
> > Inside virURIParse() routine, it will strip off the bracket '[' and ']' if
> > it's ip v6 address. Then save the ip v6 address in this format "deba::2222" 
> > in the virURI->server field, and to be passed to qemu.
> > 
> > 2) At the beginning of migration, in qemu's qemu_rdma_data_init(host_port) 
> > routine, it calls inet_parse(host_port) routine to parse the ip v6 address 
> > and 
> > port string obtained from libvirt.
> > The input string host_port passed to qemu_rdma_data_init() can be:
> >     "hostname:port", or
> >     "ipv4address:port", or
> >     "[ipv6address]:port" (i.e "[deba::2222]:49152"), or
> >     "ipv6address:port" (i.e "deba::2222:49152").
> 
> 
> Hi David,
>   I'm ccing in some libvirt guys here; this might be a bug that
> libvirt should be leaving the []'s on when it calls qemu.
> 
> Lets just check with Jiri and Dan (who I've added to the mail) to see what
> they thing libvirt should be doing here.
> 
> Dave
> 
Hi, Dr. Gilbert, and Daniel:
   Thanks for reviewing the patch, and the comment!
   Mike Roth and I did talk about the alternative solution in libvirt
side since ipv6 address is passed down from libvirt to qemu.
   This is what I see in libvirt, virURIParse(const char *uri) will be
called for:
   "uri = qemu+ssh://[deba::2222]/system"
   "uri = rdma://[deba::2222]:49152"
Inside virURIParse() it will take out the server part "[deba::2222]",
then call virStringStripIPv6Brackets() to strip off the brackets '[' and
']'.
I did a test to not calling virStringStripIPv6Brackets() in
virURIParse(). This caused ipv6 ssh authentication failure in libvirt
side (I documented in the commit comment).
  So I pick the solution to fix it in qemu's inet_parse() routine to
handle all of the possible ipv6 address input.

  As you suggested, let's wait and see what libvirt maintainer(s) think
what libvirt should be doing with regarding to this virsh live migration
over rdma using ipv6 address case.

David Dai

> > Existing qemu api inet_parse() can handle the above first 3 cases properly, 
> >  
> > but didn't handle the last case ("ipv6address:port") correctly.
> > In this live migration over rdma via ip v6 address case, the server ip v6 
> > address obtained from libvirt doesn't contain the brackets '[' and ']' 
> > (i.e. "deba::2222:49152"). It caused inet_parse() to parse only "deba" 
> > part, 
> > and stopped at the 1st colon ':'. As the result, the subsequent 
> > rdma_getaddrinfo() with ip address "deba" will fail.
> > 
> > If we don't strip off brackets '[' and ']' for an ip v6 address in 
> > libvirt's 
> > virURIParse(), it will cause libvirt ipv6 ssh authentication failure.
> > 
> > NOTE:
> > If using libvirt to do live migration over TCP via ip v6 address:
> > # virsh migrate  --live --migrateuri tcp://[deba::2222]:49152  \
> >               rhel73_host1_guest1 qemu+ssh://[deba::2222]/system --verbose
> > It works fine.
> > In migrateuri of tcp case, libvirt will call virNetSocketNewConnectTCP()
> > directly to connect to remote "deba::2222:49152" after it strips off
> > the bracket '[' and ']' for an ip v6 address. 
> > On qemu side, fd_start_outgoing_migration() will be called to do migration.
> > It doesn't call inet_parse(). So we don't see issue in tcp case.
> > 
> > Solution:
> > I choose to fix the code in qemu's inet_parse() routine to parse the
> > ip v6 addresss w/o brackets properly (i.e. "deba::2222:49152" format).
> > 
> > Signed-off-by: David Dai <address@hidden>
> > ---
> >  util/qemu-sockets.c |   28 +++++++++++++++++++++++-----
> >  1 files changed, 23 insertions(+), 5 deletions(-)
> > 
> > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> > index 7c120c4..e09191c 100644
> > --- a/util/qemu-sockets.c
> > +++ b/util/qemu-sockets.c
> > @@ -584,6 +584,8 @@ InetSocketAddress *inet_parse(const char *str, Error 
> > **errp)
> >      char port[33];
> >      int to;
> >      int pos;
> > +    char *first_col_p = strchr(str, ':');
> > +    char *last_col_p = strrchr(str, ':');
> >  
> >      addr = g_new0(InetSocketAddress, 1);
> >  
> > @@ -595,11 +597,27 @@ InetSocketAddress *inet_parse(const char *str, Error 
> > **errp)
> >              error_setg(errp, "error parsing port in address '%s'", str);
> >              goto fail;
> >          }
> > -    } else if (str[0] == '[') {
> > -        /* IPv6 addr */
> > -        if (sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos) != 2) {
> > -            error_setg(errp, "error parsing IPv6 address '%s'", str);
> > -            goto fail;
> > +    } else if (first_col_p != last_col_p) {
> > +        if (str[0] != '[') {
> > +            /* IPv6 addr w/o brackets */
> > +            char *port_p;
> > +            char *comma_p;
> > +
> > +            pstrcpy(host, sizeof(host), str);
> > +            port_p = strrchr(host, ':');
> > +            *port_p++ = '\0';
> > +            pstrcpy(port, sizeof(port), port_p);
> > +            comma_p = strchr(port, ',');
> > +            if (comma_p != NULL) {
> > +                *comma_p = '\0';
> > +            }
> > +            pos = strlen(host) + strlen(port) + 1;
> > +        } else {
> > +            /* IPv6 addr with brackets */
> > +            if (2 != sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos)) 
> > {
> > +                error_setg(errp, "error parsing IPv6 address '%s'", str);
> > +                goto fail;
> > +            }
> >          }
> >          addr->ipv6 = addr->has_ipv6 = true;
> >      } else {
> > -- 
> > 1.7.1
> > 
> > 
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK
>