[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/3] qemu: RDMA migration support using 'x-rdma' URI



On Fri, Jul 26, 2013 at 13:47:44 -0400, mrhines linux vnet ibm com wrote:
> From: "Michael R. Hines" <mrhines us ibm com>
> 
> QEMU has in tree now for version 1.6 support for RDMA Live migration.
> Full documenation of the feature: http://wiki.qemu.org/Features/RDMALiveMigration
> 
> This patch includes mainly making all the locations in libvirt where
> the 'tcp' string was hard-coded to be more flexible to use more than
> one protocol.
> 
> While the RDMA protocol has been extensively tested (from multiple
> companies as well as virt-test), the protocol 'x-rdma' will later be
> renamed to 'rdma' after the community has allowed the feature more cooking.

This does not prevent us from calling the protocol "rdma" right away and
possibly translating it to "x-rdma". However, I don't think we actually
want to commit patches for rdma migration before QEMU changes the name
to "rdma".

> Example usage:
> 
> virsh migrate --live --migrateuri x-rdma:hostname domain qemu+tcp://hostname/system
> 
> Signed-off-by: Michael R. Hines <mrhines us ibm com>
> ---
>  src/qemu/qemu_capabilities.c |    7 ++++
>  src/qemu/qemu_capabilities.h |    4 +++
>  src/qemu/qemu_command.c      |    8 +++++
>  src/qemu/qemu_migration.c    |   75 +++++++++++++++++++++++++++++++-----------
>  src/qemu/qemu_monitor.c      |    3 +-
>  src/qemu/qemu_monitor.h      |    1 +
>  6 files changed, 78 insertions(+), 20 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 5dc3c9e..94d17c6 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -234,6 +234,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>  
>                "vnc-share-policy", /* 150 */
>                "device-del-event",
> +
> +              "x-rdma", /* 152 */
>      );
>  
>  struct _virQEMUCaps {
> @@ -1101,6 +1103,7 @@ virQEMUCapsComputeCmdFlags(const char *help,
>       *  -incoming unix   (qemu >= 0.12.0)
>       *  -incoming fd     (qemu >= 0.12.0)
>       *  -incoming stdio  (all earlier kvm)
> +     *  -incoming x-rdma (qemu >= 1.6.0)
>       *
>       * NB, there was a pre-kvm-79 'tcp' support, but it
>       * was broken, because it blocked the monitor console
> @@ -2437,6 +2440,10 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps,
>      char *archstr = NULL;
>      int ret = -1;
>  
> +    if (qemuCaps->version >= MIN_X_RDMA_VERSION) {
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_RDMA);
> +    }
> +
>      if (!(archstr = qemuMonitorGetTargetArch(mon)))
>          return -1;
>  

This is wrong. First, you're adding this into a totally wrong place and
second, we need a better detection which is not based on qemu version.

> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index f5f685d..5069552 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -191,9 +191,13 @@ enum virQEMUCapsFlags {
>      QEMU_CAPS_VNC_SHARE_POLICY   = 150, /* set display sharing policy */
>      QEMU_CAPS_DEVICE_DEL_EVENT   = 151, /* DEVICE_DELETED event */
>  
> +    QEMU_CAPS_MIGRATE_QEMU_X_RDMA  = 152, /* have qemu x-rdma migration */
> +
>      QEMU_CAPS_LAST,                   /* this must always be the last item */
>  };
>  
> +#define MIN_X_RDMA_VERSION 1006000
> +
>  typedef struct _virQEMUCaps virQEMUCaps;
>  typedef virQEMUCaps *virQEMUCapsPtr;
>  
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index aa3a2fd..a26acd7 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8657,6 +8657,14 @@ qemuBuildCommandLine(virConnectPtr conn,
>                  goto error;
>              }
>              virCommandAddArg(cmd, migrateFrom);
> +        } else if (STRPREFIX(migrateFrom, "x-rdma")) {
> +            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_RDMA)) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               "%s", _("RDMA migration is not supported with "
> +                                       "this QEMU binary"));
> +                goto error;
> +            }
> +            virCommandAddArg(cmd, migrateFrom);
>          } else if (STREQ(migrateFrom, "stdio")) {
>              if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) {
>                  virCommandAddArgFormat(cmd, "fd:%d", migrateFd);
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 19001b9..de20d23 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2169,7 +2169,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>                          virDomainDefPtr *def,
>                          virStreamPtr st,
>                          unsigned int port,
> -                        unsigned long flags)
> +                        unsigned long flags,
> +                        const char *protocol)
>  {
>      virDomainObjPtr vm = NULL;
>      virDomainEventPtr event = NULL;
> @@ -2280,7 +2281,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>           * and there is at least one IPv6 address configured
>           */
>          if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION) &&
> -            getaddrinfo("::", NULL, &hints, &info) == 0) {
> +            getaddrinfo("::", NULL, &hints, &info) == 0 &&
> +                !strstr(protocol, "rdma")) {
>              freeaddrinfo(info);
>              listenAddr = "[::]";
>          } else {

Is there any reason why RDMA migration does not work over IPv6?

> @@ -2291,7 +2293,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>          /* QEMU will be started with -incoming [::]:port
>           * or -incoming 0.0.0.0:port
>           */
> -        if (virAsprintf(&migrateFrom, "tcp:%s:%d", listenAddr, port) < 0)
> +        if (virAsprintf(&migrateFrom, "%s:%s:%d", protocol, 
> +                                           listenAddr, port) < 0)
>              goto cleanup;
>      }
>  
> @@ -2482,7 +2485,7 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver,
>  
>      ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
>                                    cookieout, cookieoutlen, def,
> -                                  st, 0, flags);
> +                                  st, 0, flags, "tcp");
>      return ret;
>  }
>  
> @@ -2502,6 +2505,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
>      static int port = 0;
>      int this_port;
>      char *hostname = NULL;
> +    const char *protocol = NULL;
> +    char *well_formed_protocol = NULL;
>      const char *p;
>      char *uri_str = NULL;
>      int ret = -1;
> @@ -2550,20 +2555,29 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
>          if (virAsprintf(uri_out, "tcp:%s:%d", hostname, this_port) < 0)
>              goto cleanup;
>      } else {
> -        /* Check the URI starts with "tcp:".  We will escape the
> +        /* Check the URI starts with a valid prefix.  We will escape the
>           * URI when passing it to the qemu monitor, so bad
>           * characters in hostname part don't matter.
>           */
> -        if (!(p = STRSKIP(uri_in, "tcp:"))) {
> -            virReportError(VIR_ERR_INVALID_ARG, "%s",
> -                           _("only tcp URIs are supported for KVM/QEMU"
> -                             " migrations"));
> +
> +        protocol = strtok(strdup(uri_in), ":");
> +        if (protocol) {
> +            if (virAsprintf(&well_formed_protocol, "%s://", protocol) < 0)
> +                goto cleanup;
> +        }
> +
> +        /* Make sure it's a valid protocol */
> +        if (!(p = STRSKIP(uri_in, "tcp:")) && 
> +            !(p = STRSKIP(uri_in, "x-rdma:"))) {
> +            virReportError(VIR_ERR_INVALID_ARG, _("URI %s (%s) not supported"
> +                            " for KVM/QEMU migrations"), protocol, uri_in);
>              goto cleanup;
>          }
>  
> -        /* Convert uri_in to well-formed URI with // after tcp: */
> -        if (!(STRPREFIX(uri_in, "tcp://"))) {
> -            if (virAsprintf(&uri_str, "tcp://%s", p) < 0)
> +
> +        /* Convert uri_in to well-formed URI with // after colon */
> +        if (!(STRPREFIX(uri_in, well_formed_protocol))) {
> +            if (virAsprintf(&uri_str, "%s://%s", protocol, p) < 0)
>                  goto cleanup;
>          }

Just because we did the mistake with tcp protocol we don't have to
repeat it with rdma. Just change the rdma protocol to always be
well-formed, i.e., rdma://host

>  
> @@ -2602,10 +2616,20 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
>  
>      ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
>                                    cookieout, cookieoutlen, def,
> -                                  NULL, this_port, flags);
> +                                  NULL, this_port, flags,
> +                                  protocol ? protocol : "tcp");
>  cleanup:
>      virURIFree(uri);
>      VIR_FREE(hostname);
> +
> +    if (protocol) {
> +        VIR_FREE(protocol);
> +    }
> +
> +    if (well_formed_protocol) {
> +        VIR_FREE(well_formed_protocol);
> +    }
> +

You're not supposed to check if a variable you're about to free is
non-NULL. Running make syntax-check would have warned you.

>      if (ret != 0)
>          VIR_FREE(*uri_out);
>      return ret;
> @@ -2800,6 +2824,7 @@ struct _qemuMigrationSpec {
>      enum qemuMigrationDestinationType destType;
>      union {
>          struct {
> +            const char *proto;
>              const char *name;
>              int port;
>          } host;
> @@ -3161,6 +3186,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>      switch (spec->destType) {
>      case MIGRATION_DEST_HOST:
>          ret = qemuMonitorMigrateToHost(priv->mon, migrate_flags,
> +                                       spec->dest.host.proto,
>                                         spec->dest.host.name,
>                                         spec->dest.host.port);
>          break;
> @@ -3291,7 +3317,7 @@ cancel:
>      goto cleanup;
>  }
>  
> -/* Perform migration using QEMU's native TCP migrate support,
> +/* Perform migration using QEMU's native migrate support,
>   * not encrypted obviously
>   */
>  static int doNativeMigrate(virQEMUDriverPtr driver,
> @@ -3309,6 +3335,8 @@ static int doNativeMigrate(virQEMUDriverPtr driver,
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      virURIPtr uribits = NULL;
>      int ret = -1;
> +    char *tmp = NULL;
> +    bool rdma = false;
>      qemuMigrationSpec spec;
>  
>      VIR_DEBUG("driver=%p, vm=%p, uri=%s, cookiein=%s, cookieinlen=%d, "
> @@ -3318,20 +3346,29 @@ static int doNativeMigrate(virQEMUDriverPtr driver,
>                cookieout, cookieoutlen, flags, resource,
>                NULLSTR(graphicsuri));
>  
> +    /* HACK: source host generates bogus URIs, so fix them up */
>      if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://")) {
> -        char *tmp;
> -        /* HACK: source host generates bogus URIs, so fix them up */
>          if (virAsprintf(&tmp, "tcp://%s", uri + strlen("tcp:")) < 0)
>              return -1;
> -        uribits = virURIParse(tmp);
> -        VIR_FREE(tmp);
> +        spec.dest.host.proto = "tcp";
> +    } else if (STRPREFIX(uri, "x-rdma:") && !STRPREFIX(uri, "x-rdma://")) {
> +        if (virAsprintf(&tmp, "x-rdma://%s", uri + strlen("x-rdma:")) < 0)
> +            return -1;
> +        rdma = true;
> +        spec.dest.host.proto = "x-rdma";
>      } else {
>          uribits = virURIParse(uri);
>      }
> +
> +    if (tmp) {
> +        uribits = virURIParse(tmp);
> +        VIR_FREE(tmp);
> +    }
> +
>      if (!uribits)
>          return -1;
>  
> -    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD))
> +    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD) && !rdma)
>          spec.destType = MIGRATION_DEST_CONNECT_HOST;
>      else
>          spec.destType = MIGRATION_DEST_HOST;
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 86aed75..ce95174 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2098,6 +2098,7 @@ int qemuMonitorMigrateToFd(qemuMonitorPtr mon,
>  
>  int qemuMonitorMigrateToHost(qemuMonitorPtr mon,
>                               unsigned int flags,
> +                             const char *proto,
>                               const char *hostname,
>                               int port)
>  {
> @@ -2113,7 +2114,7 @@ int qemuMonitorMigrateToHost(qemuMonitorPtr mon,
>      }
>  
>  
> -    if (virAsprintf(&uri, "tcp:%s:%d", hostname, port) < 0)
> +    if (virAsprintf(&uri, "%s:%s:%d", proto, hostname, port) < 0)
>          return -1;
>  
>      if (mon->json)
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 82e6ae2..d722e12 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -429,6 +429,7 @@ int qemuMonitorMigrateToFd(qemuMonitorPtr mon,
>  
>  int qemuMonitorMigrateToHost(qemuMonitorPtr mon,
>                               unsigned int flags,
> +                             const char *proto, 
>                               const char *hostname,
>                               int port);
>  

Jirka


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]