[libvirt] [PATCH 2/2] migration: allow migration to succeed when given destination ip

[Daniel Veillard]

On Tue, Aug 10, 2010 at 07:42:50PM +1000, Justin Clift wrote:
> 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) {

  I we use my version of the previous patch, this need to change to
           if (virSocketParseAddr(dconn->uri->server, NULL, 0) != 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, ":");

  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);

> +            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;

  Hum .... shouldn't we free uri_out here since we don't use it ?

> +            }
> +            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
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/