[libvirt] [PATCH] qemu: enable direct migration over IPv6

[Peter Krempa]

On 02/15/13 11:00, Ján Tomko wrote:
> Use virURIParse in qemuMigrationPrepareDirect to allow parsing
> IPv6 addresses, which would cause an 'incorrect :port' error message
> before.
>
> To be able to migrate over IPv6, QEMU needs to listen on [::] instead
> of 0.0.0.0. This patch adds a call to getaddrinfo and sets the listen
> address based on the result.
>
> This will break migration if a hostname that can only be resolved on the
> source machine is passed in the migration URI, or if it does not resolve
> to the same address family on both sides.
>
> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=846013
> ---
>   src/qemu/qemu_migration.c | 65 +++++++++++++++++++++++++++++++++++++----------
>   1 file changed, 52 insertions(+), 13 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 36e55d2..c813c4a 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -22,7 +22,10 @@
>
>   #include <config.h>
>
> +#include <netdb.h>
> +#include <sys/socket.h>
>   #include <sys/time.h>
> +#include <sys/types.h>
>   #ifdef WITH_GNUTLS
>   # include <gnutls/gnutls.h>
>   # include <gnutls/x509.h>
> @@ -1835,8 +1838,11 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
>       int this_port;
>       char *hostname = NULL;
>       char migrateFrom [64];
> -    const char *p;
> +    char *uri_str;
>       int ret = -1;
> +    bool ipv6 = false;
> +    struct addrinfo *info;
> +    virURIPtr uri;
>
>       VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, "
>                 "cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, "
> @@ -1892,9 +1898,33 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
>               goto cleanup;
>           }
>
> -        /* Get the port number. */
> -        p = strrchr(uri_in, ':');
> -        if (p == strchr(uri_in, ':')) {
> +        /* Convert uri_in to well-formed URI with // after tcp: */
> +        if (!(STRPREFIX(uri_in, "tcp://"))) {
> +            if (virAsprintf(&uri_str, "tcp://%s",
> +                            uri_in + strlen("tcp:")) < 0) {

It might be better to use the STRSKIP macro instead here. That should 
also check if the old "not entirely URI" is formatted well.

> +                virReportOOMError();
> +                goto cleanup;
> +            }
> +        }
> +
> +        uri = virURIParse(uri_str ? uri_str : uri_in);
> +        VIR_FREE(uri_str);

Possible uninitialized pointer free.

> +
> +        if (uri == NULL) {
> +            virReportError(VIR_ERR_INVALID_ARG, _("unable to parse URI: %s"),
> +                           uri_in);
> +            goto cleanup;
> +        }
> +
> +        if (uri->server == NULL) {
> +            virReportError(VIR_ERR_INVALID_ARG, _("missing host in migration"
> +                                                  " URI: %s"), uri_in);
> +            goto cleanup;
> +        } else {
> +            hostname = uri->server;
> +        }
> +
> +        if (uri->port == 0) {
>               /* Generate a port */
>               this_port = QEMUD_MIGRATION_FIRST_PORT + port++;
>               if (port == QEMUD_MIGRATION_NUM_PORTS)
> @@ -1907,21 +1937,30 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
>               }
>
>           } else {
> -            p++; /* definitely has a ':' in it, see above */
> -            this_port = virParseNumber(&p);
> -            if (this_port == -1 || p-uri_in != strlen(uri_in)) {
> -                virReportError(VIR_ERR_INVALID_ARG,
> -                               "%s", _("URI ended with incorrect ':port'"));
> -                goto cleanup;
> -            }
> +            this_port = uri->port;

This cleans stuff up nicely.

>           }
>       }
>
> +    if (getaddrinfo(hostname, NULL, NULL, &info)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("unable to get address info for %s"), hostname);
> +        goto cleanup;

Isn't there a possibility to fall back on IPv4 if this fails to minimize 
the chance of regressing?

> +    } else {
> +        ipv6 = info->ai_family == AF_INET6;
> +    }
> +
>       if (*uri_out)
>           VIR_DEBUG("Generated uri_out=%s", *uri_out);
>
> -    /* QEMU will be started with -incoming tcp:0.0.0.0:port */
> -    snprintf(migrateFrom, sizeof(migrateFrom), "tcp:0.0.0.0:%d", this_port);
> +    /* QEMU will be started with -incoming tcp:0.0.0.0:port
> +     * or -incoming tcp:[::]:port for IPv6 */
> +    if (ipv6) {
> +        snprintf(migrateFrom, sizeof(migrateFrom),
> +                 "tcp:[::]:%d", this_port);
> +    } else {
> +        snprintf(migrateFrom, sizeof(migrateFrom),
> +                 "tcp:0.0.0.0:%d", this_port);

I thing this would be doable. Just do IPv4 by default if the resolution 
fails.

> +    }
>
>       ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
>                                     cookieout, cookieoutlen, dname, dom_xml,
>

Peter