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

Re: [libvirt] [PATCH 2/3] add virsh --suspend



On Wed, Nov 25, 2009 at 04:40:07PM +0100, Paolo Bonzini wrote:
> This adds a new flag, VIR_MIGRATE_PAUSED, that mandates pausing
> the migrated VM before starting it.
> 
> * include/libvirt/libvirt.h.in (virDomainMigrateFlags): Add
> VIR_MIGRATE_PAUSED.
> * src/qemu/qemu_driver.c (qemudDomainMigrateFinish2): Handle
> VIR_MIGRATE_PAUSED.
> * tools/virsh.c (opts_migrate): Add --suspend.
> (cmdMigrate): Handle it.
> * tools/virsh.pod (migrate): Document it.
> ---
> 	The complicated part of the patch is simply this with diff -b:
> 
> 	@@ -7320,6 +7320,7 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
> 	         qemuDomainObjPrivatePtr priv = vm->privateData;
> 	         dom = virGetDomain (dconn, vm->def->name, vm->def->uuid);
> 	 
> 	+        if (!(flags & VIR_MIGRATE_PAUSED)) {
> 	         /* run 'cont' on the destination, which allows migration on qemu
> 	          * >= 0.10.6 to work properly.  This isn't strictly necessary on
> 	          * older qemu's, but it also doesn't hurt anything there
> 	@@ -7335,9 +7336,17 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
> 	         qemuDomainObjExitMonitorWithDriver(driver, vm);
> 	 
> 	         vm->state = VIR_DOMAIN_RUNNING;
> 	+        }
> 
>  include/libvirt/libvirt.h.in |    1 +
>  src/libvirt.c                |    1 +
>  src/qemu/qemu_driver.c       |   33 +++++++++++++++++++++------------
>  tools/virsh.c                |    5 ++++-
>  tools/virsh.pod              |    7 ++++---
>  5 files changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 5bc7694..0488cbf 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -341,6 +341,7 @@ typedef enum {
>      VIR_MIGRATE_TUNNELLED         = (1 << 2), /* tunnel migration data over libvirtd connection */
>      VIR_MIGRATE_PERSIST_DEST      = (1 << 3), /* persist the VM on the destination */
>      VIR_MIGRATE_UNDEFINE_SOURCE   = (1 << 4), /* undefine the VM on the source */
> +    VIR_MIGRATE_PAUSED            = (1 << 5), /* pause on remote side */
>  } virDomainMigrateFlags;
>  
>  /* Domain migration. */
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 05e45f3..2ced604 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -3179,6 +3179,7 @@ virDomainMigrateDirect (virDomainPtr domain,
>   *                            on the destination host.
>   *   VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful, undefine the
>   *                               domain on the source host.
> + *   VIR_MIGRATE_PAUSED    Leave the domain suspended on the remote side.
>   *
>   * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set.
>   * Applications using the VIR_MIGRATE_PEER2PEER flag will probably
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 44cec6c..4d20fb7 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7320,24 +7320,33 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
>          qemuDomainObjPrivatePtr priv = vm->privateData;
>          dom = virGetDomain (dconn, vm->def->name, vm->def->uuid);
>  
> -        /* run 'cont' on the destination, which allows migration on qemu
> -         * >= 0.10.6 to work properly.  This isn't strictly necessary on
> -         * older qemu's, but it also doesn't hurt anything there
> -         */
> -        qemuDomainObjEnterMonitorWithDriver(driver, vm);
> -        if (qemuMonitorStartCPUs(priv->mon, dconn) < 0) {
> -            if (virGetLastError() == NULL)
> -                qemudReportError(dconn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> -                                 "%s", _("resume operation failed"));
> +        if (!(flags & VIR_MIGRATE_PAUSED)) {
> +            /* run 'cont' on the destination, which allows migration on qemu
> +             * >= 0.10.6 to work properly.  This isn't strictly necessary on
> +             * older qemu's, but it also doesn't hurt anything there
> +             */
> +            qemuDomainObjEnterMonitorWithDriver(driver, vm);
> +            if (qemuMonitorStartCPUs(priv->mon, dconn) < 0) {
> +                if (virGetLastError() == NULL)
> +                    qemudReportError(dconn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> +                                     "%s", _("resume operation failed"));
> +                qemuDomainObjExitMonitorWithDriver(driver, vm);
> +                goto endjob;
> +            }
>              qemuDomainObjExitMonitorWithDriver(driver, vm);
> -            goto endjob;
> +
> +            vm->state = VIR_DOMAIN_RUNNING;
>          }
> -        qemuDomainObjExitMonitorWithDriver(driver, vm);
>  
> -        vm->state = VIR_DOMAIN_RUNNING;
>          event = virDomainEventNewFromObj(vm,
>                                           VIR_DOMAIN_EVENT_RESUMED,
>                                           VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
> +        if (vm->state == VIR_DOMAIN_PAUSED) {
> +            qemuDomainEventQueue(driver, event);
> +            event = virDomainEventNewFromObj(vm,
> +                                             VIR_DOMAIN_EVENT_SUSPENDED,
> +                                             VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
> +        }
>          virDomainSaveStatus(dconn, driver->stateDir, vm);
>      } else {
>          qemudShutdownVMDaemon (dconn, driver, vm);
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 9faac35..9871b4b 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -2478,6 +2478,7 @@ static const vshCmdOptDef opts_migrate[] = {
>      {"tunnelled", VSH_OT_BOOL, 0, gettext_noop("tunnelled migration")},
>      {"persistent", VSH_OT_BOOL, 0, gettext_noop("persist VM on destination")},
>      {"undefinesource", VSH_OT_BOOL, 0, gettext_noop("undefine VM on source")},
> +    {"suspend", VSH_OT_BOOL, 0, gettext_noop("do not restart the domain on the destination host")},
>      {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")},
>      {"desturi", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("connection URI of the destination host")},
>      {"migrateuri", VSH_OT_DATA, 0, gettext_noop("migration URI, usually can be omitted")},
> @@ -2519,10 +2520,12 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd)
>  
>      if (vshCommandOptBool (cmd, "persistent"))
>          flags |= VIR_MIGRATE_PERSIST_DEST;
> -
>      if (vshCommandOptBool (cmd, "undefinesource"))
>          flags |= VIR_MIGRATE_UNDEFINE_SOURCE;
>  
> +    if (vshCommandOptBool (cmd, "suspend"))
> +        flags |= VIR_MIGRATE_PAUSED;
> +
>      if ((flags & VIR_MIGRATE_PEER2PEER) ||
>          vshCommandOptBool (cmd, "direct")) {
>          /* For peer2peer migration or direct migration we only expect one URI
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 6ff0151..3830464 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -302,10 +302,11 @@ except that it does some error checking.
>  The editor used can be supplied by the C<$EDITOR> environment
>  variable, or if that is not defined defaults to C<vi>.
>  
> -=item B<migrate> optional I<--live> I<domain-id> I<desturi> I<migrateuri>
> +=item B<migrate> optional I<--live> I<--suspend> I<domain-id> I<desturi> I<migrateuri>
>  
> -Migrate domain to another host.  Add --live for live migration. The I<desturi>
> -is the connection URI of the destination host, and I<migrateuri> is the
> +Migrate domain to another host.  Add --live for live migration; --suspend
> +leaves the domain paused on the destination host. The I<desturi> is the
> +connection URI of the destination host, and I<migrateuri> is the
>  migration URI, which usually can be omitted.
>  
>  =item B<reboot> I<domain-id>

ACK, though one thing I notice is that the QEMU driver isn't validating
the flags passed in


For example Xen does

    if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
       undefined_source = 1;
       flags &= ~VIR_MIGRATE_UNDEFINE_SOURCE;
    }

    /* Ignore the persist_dest flag here */
    if (flags & VIR_MIGRATE_PERSIST_DEST)
        flags &= ~VIR_MIGRATE_PERSIST_DEST;

    /* XXX we could easily do tunnelled & peer2peer migration too
       if we want to. support these... */
    if (flags != 0) {
        virXendError (conn, VIR_ERR_NO_SUPPORT,
                      "%s", _("xenDaemonDomainMigrate: unsupported flag"));
        return -1;
    }


This means if you tried to use this new '--suspend' feature with Xen you'd
get a nice error. If you used it with an old QEMU it'd silently complete
and do nothing. We can't do much about this now, but I think it is worth
adding in such a check, so if we add more migration flags later we wil be
validating them

ACK to this patch anyway

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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