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

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



On Wed, Aug 24, 2011 at 08:23:55AM -0600, Eric Blake wrote:
> On 08/24/2011 08:21 AM, Daniel P. Berrange wrote:
> >>+++ 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
> 
> Even when you use virDomainCreateWithFlags(, VIR_DOMAIN_START_PAUSED)?

When you do that, it should be viewed as 2 transitions. From shutoff
to running, to paused.

Currently we will emit a 'STARTED' event there, but are missing a
SUSPENDED event there.

> >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.
> 
> This may have bigger cleanup impact, then, if we want to guarantee
> both STARTED and SUSPENDED events on all code paths where we can
> start life paused.

I believe migration deals with it correctly. The restore/boot up paths
likely need fixing

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]