[libvirt] [PATCH] qemu: enable direct migration over IPv6
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) { + virReportOOMError(); + goto cleanup; + } + } + + uri = virURIParse(uri_str ? uri_str : uri_in); + VIR_FREE(uri_str); + + 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; } } + if (getaddrinfo(hostname, NULL, NULL, &info)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to get address info for %s"), hostname); + goto cleanup; + } 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); + } ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, dname, dom_xml, -- 1.7.12.4
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
On 02/15/2013 05:13 AM, Peter Krempa wrote:
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.
Yuck... "feature-wise" you can pass hints to getaddrinfo() to tell it what type of address family you want... You can walk the info->ai_next list looking for the family you want/prefer. Secondary feature-wise qemu could say which it prefers to use on the "-incoming" value... As for resolving hostname - does that matter here? You are telling the target to use it's localhost of the particular family that the host is using for it's FQDN. As long as that family enabled on the target, then the connection should be made...
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?
Yeah - this is tricky for some of the reasons I listed above. It seems we need a way to tell or force the target qemu to use one family style over the other because that's what the source resolved to using. Assuming I'm reading things correctly we are about to tell the target qemu how to start and to listen over the "first" style of address we found in the source hosts' list. The source connect will use that uri_out to perform the migration. The issue I can see becomes what if the target (for some reason) doesn't support/have/use the family that the host found first? When it goes to start up a localhost port, it will fail right? Then what?
+ } else { + ipv6 = info->ai_family == AF_INET6; + } +
Once you're done with 'info', a 'freeaddrinfo(info);' would be appropriate.
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);
I thought IPv6 localhost was "::1"... Or is "::" a synonym? It's been a while since I had to think about this and just took a quick google...
+ } 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.
Hmm... which resolution fails do you mean? Perhaps the "feature" needs to be set/check some field that says the target qemu wants/desires IPv6; otherwise, always fall back to use IPv4. John
+ }
ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, dname, dom_xml,
Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Sorry, I only noticed your reply today. On 02/19/13 17:36, John Ferlan wrote:
On 02/15/2013 05:13 AM, Peter Krempa wrote:
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.
Yuck... "feature-wise" you can pass hints to getaddrinfo() to tell it what type of address family you want... You can walk the info->ai_next list looking for the family you want/prefer. Secondary feature-wise qemu could say which it prefers to use on the "-incoming" value...
In this case, I assume the first family returned by getaddrinfo is the preferred one. I'm not sure how qemu would have more clue than libvirt about which address to use.
} }
+ 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?
Yeah - this is tricky for some of the reasons I listed above. It seems we need a way to tell or force the target qemu to use one family style over the other because that's what the source resolved to using.
Qemu does support ipv4 or ipv6 flags for hostnames. From a quick glance at the git history it seems it always has. Maybe we could always add the address family option to the migration URI to force this?
Assuming I'm reading things correctly we are about to tell the target qemu how to start and to listen over the "first" style of address we found in the source hosts' list. The source connect will use that uri_out to perform the migration. The issue I can see becomes what if the target (for some reason) doesn't support/have/use the family that the host found first? When it goes to start up a localhost port, it will fail right? Then what?
No, the perform phase happens on the destination, but the result is still the same. If the families do not match (or they do match but they can't connect to each other via that family), migration will fail. Then the user either has to change the network configuration or specify the IP address directly.
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);
I thought IPv6 localhost was "::1"... Or is "::" a synonym? It's been a while since I had to think about this and just took a quick google...
It's the IPv6 equivalent of 0.0.0.0, meaning any address. We don't allow migration on localhost.
+ } 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.
Hmm... which resolution fails do you mean? Perhaps the "feature" needs to be set/check some field that says the target qemu wants/desires IPv6; otherwise, always fall back to use IPv4.
If the hostname specified by the user can only be resolved on the source, migration still works, but erroring out on getaddrinfo failure would break it. We can definitely tell qemu to listen on v4/v6 if we received an IP address. As for hostnames, we can either guess it from how it resolves on the source, destination or get the input from the user. Maybe we could add a migration flag for this and add ipv4 or ipv6 option to the migration URI for qemu based on the presence/absence of this flag. Or only do IPv6 migration if a URI with a v6 address or tcp6: scheme was present? Jan
(Replying to myself to correct some mistakes) On 02/26/13 11:56, Ján Tomko wrote:
On 02/19/13 17:36, John Ferlan wrote:
On 02/15/2013 05:13 AM, Peter Krempa wrote:
On 02/15/13 11:00, Ján Tomko wrote:
} }
+ 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?
Yeah - this is tricky for some of the reasons I listed above. It seems we need a way to tell or force the target qemu to use one family style over the other because that's what the source resolved to using.
Qemu does support ipv4 or ipv6 flags for hostnames. From a quick glance at the git history it seems it always has. Maybe we could always add the address family option to the migration URI to force this?
Wrong. It does support these options in inet_listen/inet_connect functions, but it only uses these for migration since 1.1. For inet_listen we either pass :: or 0.0.0.0 so there is no need for these flags and the connection on the source is made by libvirt
Assuming I'm reading things correctly we are about to tell the target qemu how to start and to listen over the "first" style of address we found in the source hosts' list. The source connect will use that uri_out to perform the migration. The issue I can see becomes what if the target (for some reason) doesn't support/have/use the family that the host found first? When it goes to start up a localhost port, it will fail right? Then what?
No, the perform phase happens on the destination, but the result is
s/perform/prepare/
still the same. If the families do not match (or they do match but they can't connect to each other via that family), migration will fail. Then the user either has to change the network configuration or specify the IP address directly.
...
+ } 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.
Hmm... which resolution fails do you mean? Perhaps the "feature" needs to be set/check some field that says the target qemu wants/desires IPv6; otherwise, always fall back to use IPv4.
If the hostname specified by the user can only be resolved on the source, migration still works, but erroring out on getaddrinfo failure would break it.
We can definitely tell qemu to listen on v4/v6 if we received an IP address. As for hostnames, we can either guess it from how it resolves on the source, destination or get the input from the user. Maybe we could add a migration flag for this and add ipv4 or ipv6 option to the migration URI for qemu based on the presence/absence of this flag. Or only do IPv6 migration if a URI with a v6 address or tcp6: scheme was present?
Since we don't pass the URI to qemu, adding the ipv{4,6} options makes no sense.
participants (3)
-
John Ferlan -
Ján Tomko -
Peter Krempa