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

Re: [libvirt] [PATCHv2 03/26] snapshot: improve reverting to qemu paused snapshots



On Mon, Aug 15, 2011 at 05:33:14PM -0600, Eric Blake wrote:
> If you take a checkpoint snapshot of a running domain, then pause
> qemu, then restore the snapshot, the result should be a running
> domain, but the code was leaving things paused.  Furthermore, if
> you take a checkpoint of a paused domain, then run, then restore,
> there was a brief but non-deterministic window of time where the
> domain was running rather than paused.  Fix both of these
> discrepancies by always pausing before restoring.
> 
> Also, check that the VM is active every time lock is dropped
> between two monitor calls.
> 
> * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Always
> pause before reversion.
> ---
>  src/qemu/qemu_driver.c |   61 +++++++++++++++++++++++++++++++++--------------
>  1 files changed, 43 insertions(+), 18 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 78d99f4..aa53c63 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8772,44 +8772,69 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> 
>      if (snap->def->state == VIR_DOMAIN_RUNNING
>          || snap->def->state == VIR_DOMAIN_PAUSED) {
> -
> +        /* When using the loadvm monitor command, qemu does not know
> +         * whether to pause or run the reverted domain, and just stays
> +         * in the same state as before the monitor command, whether
> +         * that is paused or running.  We always pause before loadvm,
> +         * to have finer control.  */
>          if (virDomainObjIsActive(vm)) {
>              priv = vm->privateData;
> +            if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
> +                if (qemuProcessStopCPUs(driver, vm,
> +                                        VIR_DOMAIN_PAUSED_FROM_SNAPSHOT,
> +                                        QEMU_ASYNC_JOB_NONE) < 0)
> +                    goto endjob;
> +                /* Create an event now in case the restore fails, so
> +                 * that user will be alerted that they are now paused.
> +                 * If restore later succeeds to a running state, we
> +                 * replace this event with another.  */
> +                event = virDomainEventNewFromObj(vm,
> +                                                 VIR_DOMAIN_EVENT_SUSPENDED,
> +                                                 VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT);

The event detail doesn't match the main event there.
It should use one of the VIR_DOMAIN_EVENT_SUSPENDED_XXXX constants

ACK if that is fixed

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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