10:36 a.m.
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list