This fixes a problem where migration fails when a destination host uses a wrongly configured hostname, that doesn't resolve correctly. If we're given the direct IP address of the destination host, the migration will now use this instead of the resolved host name, allowing things to work. Addresses BZ # 615941: https://bugzilla.redhat.com/show_bug.cgi?id=615941 --- Please note. The DEBUG() and DEBUG0() statements in the code below don't seem to output anywhere. Left them in for now, in the assumption the problem is somewhere in my system configuration. Happy to remove them entirely if that's a better idea. src/libvirt.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 3ec5724..05e8d76 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -39,6 +39,7 @@ #include "uuid.h" #include "util.h" #include "memory.h" +#include "network.h" #ifndef WITH_DRIVER_MODULES # ifdef WITH_TEST @@ -3280,6 +3281,7 @@ virDomainMigrateVersion2 (virDomainPtr domain, unsigned long bandwidth) { virDomainPtr ddomain = NULL; + char *new_uri = NULL; char *uri_out = NULL; char *cookie = NULL; char *dom_xml = NULL; @@ -3332,8 +3334,68 @@ virDomainMigrateVersion2 (virDomainPtr domain, virDispatchError(domain->conn); goto done; } - if (uri_out) - uri = uri_out; /* Did domainMigratePrepare2 change URI? */ + + if (uri_out) { + /* We need to determine the best server address to use for the + * destination host. We potentially have a choice of (up to) two: + * + * a) The server address given to the migration function on the + * source, whose info must be ok otherwise we wouldn't have + * made it this far through the code, or + * + * b) The server address returned to us by the destination + * host itself (may be broken in some situations) + * + * We decide by checking if the address given for a) above is a + * numeric IP address. If it is, we know that's good, so we + * prefer that over anything given to us by the destination host. + */ + + DEBUG("URI returned by the destination host: '%s'", uri_out); + + /* Check if the server adress we were originally given is an IP */ + if (virIsNumericIPAddr(dconn->uri->server) != 0) { + /* The server address we were passed from the source isn't + * a numeric IP address. It's likely a host name in text form. + * Therefore, we instead use the URI provided to us by the + * destination server. + */ + DEBUG0("Server address in URI is not numeric-only"); + uri = uri_out; + } else { + /* The server address we were passed from the source *is* a + * numeric IP address. We recreate the URI given to us by the + * destination server, but using this "known good" IP address + * instead. + */ + + DEBUG0("Server address in URI is numeric-only"); + + char *transport; + char *destGivenHostname; + char *destGivenPort; + transport = strtok(uri_out, ":"); + destGivenHostname = strtok(NULL, ":"); + destGivenPort = strtok(NULL, ":"); + DEBUG("URI extracted strings; transport: '%s', " + "destination address: '%s', destination port: '%s'", + transport, destGivenHostname, destGivenPort); + ret = virAsprintf(&new_uri, "%s:%s:%s", + transport, + dconn->uri->server, + destGivenPort); + if (ret < 0) { + /* An error occurred when creating the new URI string, so fall + * back to using the URI provided by the destination server. + */ + uri = uri_out; + } else { + /* String creation worked, so use it */ + uri = new_uri; + } + DEBUG("URI we'll be using: '%s'", uri); + } + } assert (uri != NULL); /* Perform the migration. The driver isn't supposed to return -- 1.7.2.1

On Tue, Aug 10, 2010 at 07:42:50PM +1000, Justin Clift wrote: I we use my version of the previous patch, this need to change to if (virSocketParseAddr(dconn->uri->server, NULL, 0) != 0) { aye aye aye ... strtok is not thread safe, so we would have to use strtok_r() instead to avoid this major problem. But the simplest (assuming uri_out is a valid URI) is to parse the URI using libxml2 xmlURIPtr uriptr = xmlParseURI(uri_out) then you can extract in an exact fashion the server, port, etc ... as fields i the uriptr structure, once done xmlFreeURI(uriptr); Hum .... shouldn't we free uri_out here since we don't use it ? -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Tue, Aug 10, 2010 at 07:42:49PM +1000, Justin Clift wrote: Hum ... I think that it's better to allow virSocketParseAddr() to take a NULL Addr pointer it's very simple, I'm attaching the patch and then virIsNumericIPAddr(foo) can be replaced by virSocketParseAddr(foo, NULL, 0) it also has the extensibility of being able to cope with IPv4 or IPv6 only if one need, just by adjusting the hint. Am I right ? I see you do more initializations on hints.ai_family but the memset means the two functions are just the same for ai_socktype and ai_protocol, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/