Re: [libvirt] [PATCH RFC 3/4] Create new virDrvDomainSuspendFlags API
On Thu, Jan 26, 2012 at 03:06:15PM +0100, Michal Privoznik wrote:
define these flags:
VIR_DOMAIN_SUSPEND_SLEEP = 0, /* Suspend to RAM */
VIR_DOMAIN_SUSPEND_HIBERNATE = (1 << 0), /* Suspend to disk */
---
include/libvirt/libvirt.h.in | 8 ++++++
src/driver.h | 4 +++
src/libvirt.c | 55 ++++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 1 +
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 9 ++++++-
src/remote_protocol-structs | 5 ++++
7 files changed, 82 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index e99cd00..d5ac891 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1233,6 +1233,14 @@ int virDomainFree (virDomainPtr
domain);
int virDomainSuspend (virDomainPtr domain);
int virDomainResume (virDomainPtr domain);
+typedef enum {
+ VIR_DOMAIN_SUSPEND_SLEEP = 0, /* Suspend to RAM */
+ VIR_DOMAIN_SUSPEND_HIBERNATE = (1 << 0), /* Suspend to disk */
+} virDomainSuspendFlagValues;
+
+int virDomainSuspendFlags (virDomainPtr domain,
+ unsigned int flags);
I'm not entirely comfortable about adding this facility
to the virDomainSuspendAPI.
IMHO the only commonality between what the current code does, vs what
the guest agent does is that they have the misfortune to share the
word 'suspend'. Conceptually & functionally they are completely
different from each other. In particular the current API is a guest
lifecycle changing API, which causes emission of events. The proposed
flags don't change the guest lifecycle directly, but may indirectly
cause emission of a SHUTDOWN event some time later.
It is a shame that our current API is called 'Suspend' - I wish it had
been called 'Pause', but we can't change that now.
I've no objection to the new functionality, but I feel we need a different
name for the API. Perphaps virDomainPMSuspend (PM == Power Manager)
Or virDomainNodeSuspendForDuration to match the host API naming
Second, do we need some more parameters here, besides just the suspend
target. When we added virNodeSuspendForDuration, we had an unsigned
long long duration field to allow auto-wakeup after some time.
Ultimately on Linux, but virNodeSuspendForDuration and virDOmainPMSuspend
end up invoking 'pm-suspend' command line tool, so I feel we ought to have
the 'duration' parameter for this too. Even if QEMU agent doesn't currently
support it, I imagine it could in the future.
We could even use the same enum for the 'target' as with our existing
API
Regards,
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 :|