[libvirt] [PATCHv2 17/20] snapshot: qemu: Add detail option for PMSUSPENDED event.

[Eric Blake]

On 11/01/2012 10:22 AM, Peter Krempa wrote:
> This patch adds an event to be raised if a PM suspended guest is
> restored from a snapshot.
> ---
>  examples/domain-events/events-c/event-test.c | 3 +++
>  include/libvirt/libvirt.h.in                 | 2 ++
>  2 files changed, 5 insertions(+)

Hmm, the fact that we allow a system checkpoint from a suspended guest
probably means that we have some cleanups to do in the snapshot-create
and snapshot-revert cases.  For example, during create, if the
CREATE_REDEFINE flags is set, right now we have logic that allows the
user to change between running and paused, but prevents changing between
running and shutoff; but we lack logic that prevents changing between
pmsuspended and other states.  I don't know if you cover this later in
your series, or if your v3 will have to touch more places in the code,
but we certainly have to think about it.

Also, does qemu even support the ability to restore a guest into an S3
state?  Remember, with S3, the qemu process still exists, and we can
wake it up; with S4, the qemu process is gone, and the guest appears to
be shutdown (the difference being that it will boot differently the next
time it starts).  Reverting into S4 is trivial - put the disk state
back.  But reverting into S3 requires cooperation from qemu; and right
now, I don't think we have that.  If you migrate an S3 guest from one
machine to another, right now qemu will wake the guest on the
destination, since it doesn't know how to migrate S3 state.

> 
> diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c
> index 39bea49..f0e3fde 100644
> --- a/examples/domain-events/events-c/event-test.c
> +++ b/examples/domain-events/events-c/event-test.c
> @@ -204,6 +204,9 @@ static const char *eventDetailToString(int event, int detail) {
>              case VIR_DOMAIN_EVENT_PMSUSPENDED_DISK:
>                  ret = "Disk";
>                  break;
> +            case VIR_DOMAIN_EVENT_PMSUSPENDED_FROM_SNAPSHOT:
> +                ret = "from snapshot";
> +                break;

Hmm.  I'm a bit wary of our existing code - given that
PMSUSPENDED_MEMORY (S3, qemu still exists) and PMSUSPENDED_DISK (S4,
qemu process no longer exists so guest appears to be shut down) are
quite different in effect, I'm not sure we are even tracking existing
states correctly.  Are we sure we should be adding yet another state,
and if so, does PMSUSPENDED_FROM_SNAPSHOT mean the guest is in S3 (a
qemu process exists and is ready to wake) or in S4 (no qemu process
exists, but booting will be fast)?  It is ambiguous to claim both S3 and
S4 from the same event, since they are so different.

In short, I'm not sure about this patch, and think we have stepped into
a rat's nest of awkward code to begin with in relation to S3/S4.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121106/064dce3b/attachment-0001.sig>