[libvirt] [PATCHv2 02/26] snapshot: better event when reverting qemu to paused snapshot

[Daniel P. Berrange]

On Mon, Aug 15, 2011 at 05:33:13PM -0600, Eric Blake wrote:
> When reverting a running domain to a paused snapshot, the event
> that fires should mention that the domain is suspended.
> 
> * include/libvirt/libvirt.h.in
> (VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT): New sub-event.
> * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Use it.
> ---
>  include/libvirt/libvirt.h.in |    3 ++-
>  src/qemu/qemu_driver.c       |   10 ++++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index aa29fb6..a625479 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2010,7 +2010,7 @@ typedef enum {
>      VIR_DOMAIN_EVENT_STARTED_BOOTED = 0,   /* Normal startup from boot */
>      VIR_DOMAIN_EVENT_STARTED_MIGRATED = 1, /* Incoming migration from another host */
>      VIR_DOMAIN_EVENT_STARTED_RESTORED = 2, /* Restored from a state file */
> -    VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT = 3, /* Restored from snapshot */
> +    VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT = 3, /* Restored from running snapshot */
>  } virDomainEventStartedDetailType;
> 
>  /**
> @@ -2023,6 +2023,7 @@ typedef enum {
>      VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED = 1, /* Suspended for offline migration */
>      VIR_DOMAIN_EVENT_SUSPENDED_IOERROR = 2,  /* Suspended due to a disk I/O error */
>      VIR_DOMAIN_EVENT_SUSPENDED_WATCHDOG = 3,  /* Suspended due to a watchdog firing */
> +    VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT = 4, /* Restored from paused snapshot */
>  } virDomainEventSuspendedDetailType;
> 
>  /**
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 421a98e..78d99f4 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8804,14 +8804,16 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
>                                       QEMU_ASYNC_JOB_NONE);
>              if (rc < 0)
>                  goto endjob;
> +            event = virDomainEventNewFromObj(vm,
> +                                             VIR_DOMAIN_EVENT_SUSPENDED,
> +                                             VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT);
>          } else {
>              virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
>                                   VIR_DOMAIN_RUNNING_FROM_SNAPSHOT);
> +            event = virDomainEventNewFromObj(vm,
> +                                             VIR_DOMAIN_EVENT_STARTED,
> +                                             VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT);
>          }
> -
> -        event = virDomainEventNewFromObj(vm,
> -                                         VIR_DOMAIN_EVENT_STARTED,
> -                                         VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT);
>      } else {

This isn't so nice. The lifecycle model for VMs is

    shutoff <----> running <------>paused

This change creates a direct transition from shutoff to paused, missing
out the running state, which will break any apps which are just looking
to find out when guests stop/start and don't care about pause/resume.
Also

    virsh start --paused $GUEST

will only emit a 'VIR_DOMAIN_EVENT_STARTED', so the current beahviour for
reverting to a paused snapshot matches that.

Arguably we could *also* emit an VIR_DOMAIN_EVENT_SUSPENDED, immediately
*after* the VIR_DOMAIN_EVENT_STARTED, in both cases though.


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 :|