Re: [libvirt] [PATCH 01/15] Introduce virDomainSuspendFlags
On 02/03/2014 09:16 AM, Michal Privoznik wrote:
So far, we have just bare virDomainSuspend() API that suspends a
domain.
However, in the future there might occur a case, in which we may want
to modify suspend behavior slightly. In that case, @flags are useful.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
include/libvirt/libvirt.h.in | 2 ++
src/driver.h | 5 +++++
src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++++++----
src/libvirt_public.syms | 5 +++++
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 13 +++++++++++-
src/remote_protocol-structs | 5 +++++
7 files changed, 75 insertions(+), 5 deletions(-)
Back when we added virDomainShutdownFlags in 0.9.10, I asked if we
should also add *Flags variants for remaining functions without them; at
the time, we decided against it, but I'm not quite sure why.
I'm perfectly fine with adding this for the sake of making future
additions easier, even if we don't have a use for the flags now - it's
easier to support a flag than it is to rebase to pick up a new function
for any situation where the .so contains a flags function, but it may be
worth getting a second opinion before pushing, if you don't have a plan
to use flags right away.
As to the code itself:
+++ b/src/libvirt.c
@@ -2347,13 +2347,54 @@ error:
/**
+ * virDomainSuspendFlags:
+ * @domain: a domain object
+ * @flags: extra flags, not used yet, so callers should always pass 0
+ *
+ * Suspends an active domain, the process is frozen without further
+ * access to CPU resources and I/O but the memory used by the domain at
+ * the hypervisor level will stay allocated. Use virDomainResume() to
+ * reactivate the domain. This function may require privileged access.
+ * Moreover, suspend may not be supported if domain is in some special
+ * state like VIR_DOMAIN_PMSUSPENDED.
+ *
+ * Returns 0 in case of success and -1 in case of failure.
Might be worth adding a comment to virDomainSuspend() that mentions it
is equivalent to virDomainSuspendFlags() with flags 0.
* virDomainResume:
* @domain: a domain object
*
- * Resume a suspended domain, the process is restarted from the state where
- * it was frozen by calling virDomainSuspend().
- * This function may require privileged access
- * Moreover, resume may not be supported if domain is in some
+ * Resume a suspended domain, the process is restarted from the state
+ * where it was frozen by calling virDomainSuspend() or
+ * virDomainSuspendFlags(). This function may require privileged
+ * access Moreover, resume may not be supported if domain is in some
s/access/access./
+++ b/src/remote/remote_protocol.x
+
+ /**
+ * @generate: both
+ * @acl: domain:suspend
+ */
+ REMOTE_PROC_DOMAIN_SUSPEND_FLAGS = 316
Trivial conflict with my pending review for server-side event filtering.
Otherwise looks okay.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)