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

Re: [libvirt] [PATCH v2 2/2] qemu: migration: new migration param for persistent destination XML



On Fri, Mar 11, 2016 at 18:13:40 +0300, Dmitry Andreev wrote:
> Migration API allows to specify a destination domain configuration.
> Offline domain has only inactive XML and it is replaced by configuration
> specified using VIR_MIGRATE_PARAM_DEST_XML param. In case of live
> migration VIR_MIGRATE_PARAM_DEST_XML param is applied for active XML.
> 
> This commit introduces the new VIR_MIGRATE_PARAM_DEST_PERSIST_XML param
> that can be used within live migration to replace persistent/inactive
> configuration.
> 
> Required for: https://bugzilla.redhat.com/show_bug.cgi?id=835300
> ---
>  include/libvirt/libvirt-domain.h | 15 +++++++++++
>  src/qemu/qemu_driver.c           | 10 +++++---
>  src/qemu/qemu_migration.c        | 54 +++++++++++++++++++++++++++-------------
>  src/qemu/qemu_migration.h        |  2 ++
>  4 files changed, 61 insertions(+), 20 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 8ea3df6..2a589fd 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -723,6 +723,21 @@ typedef enum {
>  # define VIR_MIGRATE_PARAM_DEST_XML          "destination_xml"
>  
>  /**
> + * VIR_MIGRATE_PARAM_DEST_PERSIST_XML:
> + *
> + * virDomainMigrate* params field: the new persistant configuration to be used
> + * for the domain on the destination host as VIR_TYPED_PARAM_STRING.
> + * This field cannot be used to rename the domain during migration (use
> + * VIR_MIGRATE_PARAM_DEST_NAME field for that purpose). Domain name in the
> + * destination XML must match the original domain name.
> + *
> + * Omitting this parameter keeps the original domain persistent configuration.
> + * Using this field with hypervisors that do not support changing domain
> + * configuration during migration will result in a failure.
> + */
> +# define VIR_MIGRATE_PARAM_DEST_PERSIST_XML  "destination_persistent_xml"

I think VIR_MIGRATE_PARAM_PERSIST_XML "persistent_xml" would be shorter
and still clear enough.

> +
> +/**
>   * VIR_MIGRATE_PARAM_BANDWIDTH:
>   *
>   * virDomainMigrate* params field: the maximum bandwidth (in MiB/s) that will
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a0d6596..72bc17a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12066,7 +12066,7 @@ qemuDomainMigratePerform(virDomainPtr dom,
>       *
>       * Consume any cookie we were able to decode though
>       */
> -    ret = qemuMigrationPerform(driver, dom->conn, vm,
> +    ret = qemuMigrationPerform(driver, dom->conn, vm, NULL,
>                                 NULL, dconnuri, uri, NULL, NULL, 0, NULL,
>                                 cookie, cookielen,
>                                 NULL, NULL, /* No output cookies in v2 */
> @@ -12450,7 +12450,7 @@ qemuDomainMigratePerform3(virDomainPtr dom,
>          return -1;
>      }
>  
> -    return qemuMigrationPerform(driver, dom->conn, vm, xmlin,
> +    return qemuMigrationPerform(driver, dom->conn, vm, xmlin, NULL,
>                                  dconnuri, uri, NULL, NULL, 0, NULL,
>                                  cookiein, cookieinlen,
>                                  cookieout, cookieoutlen,
> @@ -12471,6 +12471,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
>      virQEMUDriverPtr driver = dom->conn->privateData;
>      virDomainObjPtr vm;
>      const char *dom_xml = NULL;
> +    const char *persist_xml = NULL;
>      const char *dname = NULL;
>      const char *uri = NULL;
>      const char *graphicsuri = NULL;
> @@ -12488,6 +12489,9 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
>                                  VIR_MIGRATE_PARAM_DEST_XML,
>                                  &dom_xml) < 0 ||
>          virTypedParamsGetString(params, nparams,
> +                                VIR_MIGRATE_PARAM_DEST_PERSIST_XML,
> +                                &persist_xml) < 0 ||
> +        virTypedParamsGetString(params, nparams,
>                                  VIR_MIGRATE_PARAM_DEST_NAME,
>                                  &dname) < 0 ||
>          virTypedParamsGetString(params, nparams,
> @@ -12519,7 +12523,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    ret = qemuMigrationPerform(driver, dom->conn, vm, dom_xml,
> +    ret = qemuMigrationPerform(driver, dom->conn, vm, dom_xml, persist_xml,
>                                 dconnuri, uri, graphicsuri, listenAddress,
>                                 nmigrate_disks, migrate_disks,
>                                 cookiein, cookieinlen, cookieout, cookieoutlen,
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 92d2ce9..872b82f 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -4282,6 +4282,7 @@ qemuMigrationConnect(virQEMUDriverPtr driver,
>  static int
>  qemuMigrationRun(virQEMUDriverPtr driver,
>                   virDomainObjPtr vm,
> +                 const char *xmlin_persist,

Any reason for using different name? I'd stick with persist_xml.

>                   const char *cookiein,
>                   int cookieinlen,
>                   char **cookieout,
> @@ -4296,6 +4297,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>  {
>      int ret = -1;
>      unsigned int migrate_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
> +    virDomainDefPtr def = NULL;
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      qemuMigrationCookiePtr mig = NULL;
>      qemuMigrationIOThreadPtr iothread = NULL;
> @@ -4546,14 +4548,25 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>  
>      cookieFlags |= QEMU_MIGRATION_COOKIE_NETWORK |
>                     QEMU_MIGRATION_COOKIE_STATS;
> -    if (flags & VIR_MIGRATE_PERSIST_DEST)
> -        cookieFlags |= QEMU_MIGRATION_COOKIE_PERSISTENT;
> +
> +    if (flags & VIR_MIGRATE_PERSIST_DEST) {
> +        if (!xmlin_persist) {
> +            cookieFlags |= QEMU_MIGRATION_COOKIE_PERSISTENT;
> +        } else {
> +            def = qemuMigrationPrepareDef(driver, xmlin_persist, NULL, NULL);
> +
> +            if (!def || qemuMigrationCookieAddPersistent(mig, def) < 0)
> +                ret = -1;
> +        }
> +    }
> +

I think it would be a bit nicer if qemuMigrationCookieAddPersistent was
removed from qemuMigrationBakeCookie and the code above would just call
it directly with either vm's def or the one supplied in params.

>      if (ret == 0 &&
>          qemuMigrationBakeCookie(mig, driver, vm, cookieout,
>                                  cookieoutlen, cookieFlags) < 0) {
>          VIR_WARN("Unable to encode migration cookie");
>      }
>  
> +    virDomainDefFree(def);
>      qemuMigrationCookieFree(mig);
>  
>      if (events)
> @@ -4588,6 +4601,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>   */
>  static int doNativeMigrate(virQEMUDriverPtr driver,
>                             virDomainObjPtr vm,
> +                           const char *xmlin_persist,
>                             const char *uri,
>                             const char *cookiein,
>                             int cookieinlen,
> @@ -4646,7 +4660,7 @@ static int doNativeMigrate(virQEMUDriverPtr driver,
>      spec.dest.host.port = uribits->port;
>      spec.fwdType = MIGRATION_FWD_DIRECT;
>  
> -    ret = qemuMigrationRun(driver, vm, cookiein, cookieinlen, cookieout,
> +    ret = qemuMigrationRun(driver, vm, xmlin_persist, cookiein, cookieinlen, cookieout,
>                             cookieoutlen, flags, resource, &spec, dconn,
>                             graphicsuri, nmigrate_disks, migrate_disks);
>  
> @@ -4663,6 +4677,7 @@ static int doNativeMigrate(virQEMUDriverPtr driver,
>  static int doTunnelMigrate(virQEMUDriverPtr driver,
>                             virDomainObjPtr vm,
>                             virStreamPtr st,
> +                           const char *xml_persist,
>                             const char *cookiein,
>                             int cookieinlen,
>                             char **cookieout,
> @@ -4707,7 +4722,7 @@ static int doTunnelMigrate(virQEMUDriverPtr driver,
>          goto cleanup;
>      }
>  
> -    ret = qemuMigrationRun(driver, vm, cookiein, cookieinlen, cookieout,
> +    ret = qemuMigrationRun(driver, vm, xml_persist, cookiein, cookieinlen, cookieout,
>                             cookieoutlen, flags, resource, &spec, dconn,
>                             graphicsuri, nmigrate_disks, migrate_disks);
>  
> @@ -4817,12 +4832,12 @@ static int doPeer2PeerMigrate2(virQEMUDriverPtr driver,
>      VIR_DEBUG("Perform %p", sconn);
>      qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM2);
>      if (flags & VIR_MIGRATE_TUNNELLED)
> -        ret = doTunnelMigrate(driver, vm, st,
> +        ret = doTunnelMigrate(driver, vm, st, NULL,
>                                NULL, 0, NULL, NULL,
>                                flags, resource, dconn,
>                                NULL, 0, NULL);
>      else
> -        ret = doNativeMigrate(driver, vm, uri_out,
> +        ret = doNativeMigrate(driver, vm, NULL, uri_out,
>                                cookie, cookielen,
>                                NULL, NULL, /* No out cookie with v2 migration */
>                                flags, resource, dconn, NULL, 0, NULL);
> @@ -4883,6 +4898,7 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver,
>                      const char *dconnuri,
>                      virDomainObjPtr vm,
>                      const char *xmlin,
> +                    const char *xmlin_persist,
>                      const char *dname,
>                      const char *uri,
>                      const char *graphicsuri,
> @@ -5047,13 +5063,13 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver,
>      cookieout = NULL;
>      cookieoutlen = 0;
>      if (flags & VIR_MIGRATE_TUNNELLED) {
> -        ret = doTunnelMigrate(driver, vm, st,
> +        ret = doTunnelMigrate(driver, vm, st, xmlin_persist,
>                                cookiein, cookieinlen,
>                                &cookieout, &cookieoutlen,
>                                flags, bandwidth, dconn, graphicsuri,
>                                nmigrate_disks, migrate_disks);
>      } else {
> -        ret = doNativeMigrate(driver, vm, uri,
> +        ret = doNativeMigrate(driver, vm, xmlin_persist, uri,
>                                cookiein, cookieinlen,
>                                &cookieout, &cookieoutlen,
>                                flags, bandwidth, dconn, graphicsuri,
> @@ -5226,6 +5242,7 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver,
>                                virConnectPtr sconn,
>                                virDomainObjPtr vm,
>                                const char *xmlin,
> +                              const char *xmlin_persist,
>                                const char *dconnuri,
>                                const char *uri,
>                                const char *graphicsuri,
> @@ -5344,9 +5361,9 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver,
>  
>      if (*v3proto) {
>          ret = doPeer2PeerMigrate3(driver, sconn, dconn, dconnuri, vm, xmlin,
> -                                  dname, uri, graphicsuri, listenAddress,
> -                                  nmigrate_disks, migrate_disks, resource,
> -                                  useParams, flags);
> +                                  xmlin_persist, dname, uri, graphicsuri,
> +                                  listenAddress, nmigrate_disks, migrate_disks,
> +                                  resource, useParams, flags);
>      } else {
>          ret = doPeer2PeerMigrate2(driver, sconn, dconn, vm,
>                                    dconnuri, flags, dname, resource);
> @@ -5377,6 +5394,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver,
>                          virConnectPtr conn,
>                          virDomainObjPtr vm,
>                          const char *xmlin,
> +                        const char *xmlin_persist,
>                          const char *dconnuri,
>                          const char *uri,
>                          const char *graphicsuri,
> @@ -5416,13 +5434,13 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver,
>      qemuMigrationStoreDomainState(vm);
>  
>      if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) {
> -        ret = doPeer2PeerMigrate(driver, conn, vm, xmlin,
> +        ret = doPeer2PeerMigrate(driver, conn, vm, xmlin, xmlin_persist,
>                                   dconnuri, uri, graphicsuri, listenAddress,
>                                   nmigrate_disks, migrate_disks,
>                                   flags, dname, resource, &v3proto);
>      } else {
>          qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM2);
> -        ret = doNativeMigrate(driver, vm, uri, cookiein, cookieinlen,
> +        ret = doNativeMigrate(driver, vm, xmlin_persist, uri, cookiein, cookieinlen,
>                                cookieout, cookieoutlen,
>                                flags, resource, NULL, NULL, 0, NULL);
>      }
> @@ -5481,6 +5499,7 @@ static int
>  qemuMigrationPerformPhase(virQEMUDriverPtr driver,
>                            virConnectPtr conn,
>                            virDomainObjPtr vm,
> +                          const char *xmlin_persist,
>                            const char *uri,
>                            const char *graphicsuri,
>                            size_t nmigrate_disks,
> @@ -5507,7 +5526,7 @@ qemuMigrationPerformPhase(virQEMUDriverPtr driver,
>      virCloseCallbacksUnset(driver->closeCallbacks, vm,
>                             qemuMigrationCleanup);
>  
> -    ret = doNativeMigrate(driver, vm, uri, cookiein, cookieinlen,
> +    ret = doNativeMigrate(driver, vm, xmlin_persist, uri, cookiein, cookieinlen,
>                            cookieout, cookieoutlen,
>                            flags, resource, NULL, graphicsuri,
>                            nmigrate_disks, migrate_disks);
> @@ -5546,6 +5565,7 @@ qemuMigrationPerform(virQEMUDriverPtr driver,
>                       virConnectPtr conn,
>                       virDomainObjPtr vm,
>                       const char *xmlin,
> +                     const char *xmlin_persist,
>                       const char *dconnuri,
>                       const char *uri,
>                       const char *graphicsuri,
> @@ -5578,7 +5598,7 @@ qemuMigrationPerform(virQEMUDriverPtr driver,
>              return -1;
>          }
>  
> -        return qemuMigrationPerformJob(driver, conn, vm, xmlin, dconnuri, uri,
> +        return qemuMigrationPerformJob(driver, conn, vm, xmlin, xmlin_persist, dconnuri, uri,
>                                         graphicsuri, listenAddress,
>                                         nmigrate_disks, migrate_disks,
>                                         cookiein, cookieinlen,
> @@ -5592,14 +5612,14 @@ qemuMigrationPerform(virQEMUDriverPtr driver,
>          }
>  
>          if (v3proto) {
> -            return qemuMigrationPerformPhase(driver, conn, vm, uri,
> +            return qemuMigrationPerformPhase(driver, conn, vm, xmlin_persist, uri,
>                                               graphicsuri,
>                                               nmigrate_disks, migrate_disks,
>                                               cookiein, cookieinlen,
>                                               cookieout, cookieoutlen,
>                                               flags, resource);
>          } else {
> -            return qemuMigrationPerformJob(driver, conn, vm, xmlin, NULL,
> +            return qemuMigrationPerformJob(driver, conn, vm, xmlin, xmlin_persist, NULL,
>                                             uri, graphicsuri, listenAddress,
>                                             nmigrate_disks, migrate_disks,
>                                             cookiein, cookieinlen,
> diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
> index 2c67a02..ac16a8f 100644
> --- a/src/qemu/qemu_migration.h
> +++ b/src/qemu/qemu_migration.h
> @@ -48,6 +48,7 @@
>      VIR_MIGRATE_PARAM_URI,              VIR_TYPED_PARAM_STRING,   \
>      VIR_MIGRATE_PARAM_DEST_NAME,        VIR_TYPED_PARAM_STRING,   \
>      VIR_MIGRATE_PARAM_DEST_XML,         VIR_TYPED_PARAM_STRING,   \
> +    VIR_MIGRATE_PARAM_DEST_PERSIST_XML, VIR_TYPED_PARAM_STRING,   \

What's wrong with adding the new parameter at the end of this list?

>      VIR_MIGRATE_PARAM_BANDWIDTH,        VIR_TYPED_PARAM_ULLONG,   \
>      VIR_MIGRATE_PARAM_GRAPHICS_URI,     VIR_TYPED_PARAM_STRING,   \
>      VIR_MIGRATE_PARAM_LISTEN_ADDRESS,   VIR_TYPED_PARAM_STRING,   \
> @@ -140,6 +141,7 @@ int qemuMigrationPerform(virQEMUDriverPtr driver,
>                           virConnectPtr conn,
>                           virDomainObjPtr vm,
>                           const char *xmlin,
> +                         const char *xmlin_persist,
>                           const char *dconnuri,
>                           const char *uri,
>                           const char *graphicsuri,

Looks OK apart from a few nits.

Jirka


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