On 11/09/2011 05:05 AM, Srivatsa S. Bhat wrote:
> Define the required interfaces to export the API.
>
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@xxxxxxxxxxxxxxxxxx>
> ---
>
> include/libvirt/libvirt.h.in | 4 ++++
> src/driver.h | 5 ++++
> src/libvirt.c | 48 ++++++++++++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 1 +
These changes are good for the first patch, in introducing the API.
Plus see my comment in 1/2 about the hunk defining VIR_S3 (or whatever
name we settle on) being part of this patch.
> src/qemu/qemu_driver.c | 1 +
This change should be shuffled into the qemu driver implementation of
the driver backend.
> src/remote/remote_driver.c | 1 +
> src/remote/remote_protocol.x | 12 ++++++++++-
These changes belong to a second patch, implementing the RPC driver
backend. And don't forget a patch to virsh.
> +++ b/include/libvirt/libvirt.h.in
> @@ -1055,6 +1055,10 @@ unsigned long long virNodeGetFreeMemory (virConnectPtr conn);
> int virNodeGetSecurityModel (virConnectPtr conn,
> virSecurityModelPtr secmodel);
>
> +int virNodeSuspendForDuration (virConnectPtr conn,
> + int state,
> + unsigned long long duration);
Needs an 'unsigned int flags' argument, even if we always pass 0 for now.
> +++ b/src/libvirt.c
> @@ -6303,6 +6303,54 @@ error:
> }
>
> /**
> + * virNodeSuspendForDuration:
> + * @conn: pointer to the hypervisor connection
> + * @state: the state to which the host must be suspended to,
> + * such as: VIR_S3 (Suspend-to-RAM)
> + * VIR_S4 (Suspend-to-Disk)
Is this a bitmask or a linear list? I guess a list makes more sense.
Also, some machines support a hybrid between S3 and S4 (pm-is-supported
--suspend-hybrid), which saves state to disk like S4 but then drops to
S3 for faster resume as long as power is present. Perhaps a hybrid
state is deserving of a third value in the enum?
> + * @duration: the time duration in seconds, for which the host
> + * has to be suspended
> + *
> + * Suspend the node (host machine) for the given duration of time
> + * in the specified state (such as S3 or S4). Resume the node
> + * after the time duration is complete.
We might want to be a bit more "fuzzy" on how we word this, so that we
aren't making impossible promises. Something like:
Attempt to suspend the node (host machine) for the given duration of
time into the specified state (S3 for suspend to RAM, S4 for suspend to
disk). Schedule the node's real-time-clock interrupt to resume the node
after the time duration is complete.
> + *
> + * Returns 0 on success (i.e., the node will be suspended after a
> + * short delay), -1 on failure (the operation is not supported).
(the operation is not supported, or an attempted suspend is already
underway).
> + */
> +int
> +virNodeSuspendForDuration(virConnectPtr conn,
> + int state,
> + unsigned long long duration)
> +{
> +
> + VIR_DEBUG("conn=%p, state=%d, duration=%lld", conn, state, duration);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_CONNECT(conn)) {
> + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virDispatchError(NULL);
> + return -1;
> + }
> +
You must check for and reject an attempt to use this on a read-only
connection.
> + if (conn->driver->nodeSuspendForDuration) {
> + int ret;
> + ret = conn->driver->nodeSuspendForDuration(conn, state, duration);
Be sure to pass a flags argument on to the driver.
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> + virDispatchError(conn);
> + return -1;
> +}
> +
> +
> +/**
> * virDomainGetSchedulerType:
> * @domain: pointer to domain object
> * @nparams: pointer to number of scheduler parameters, can be NULL
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index bcefb10..fd44170 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -478,6 +478,7 @@ LIBVIRT_0.9.4 {
> virDomainGetBlockJobInfo;
> virDomainBlockJobSetSpeed;
> virDomainBlockPull;
> + virNodeSuspendForDuration;
> } LIBVIRT_0.9.3;
The new API must be in a LIBVIRT_0.9.8 section at the bottom of the
file; we are NOT re-doing 0.9.3.
>
> LIBVIRT_0.9.5 {
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b4dc582..f744539 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10911,6 +10911,7 @@ static virDriver qemuDriver = {
> .domainGetBlockJobInfo = qemuDomainGetBlockJobInfo, /* 0.9.4 */
> .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */
> .domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */
> + .nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.7 */
0.9.8.
> };
>
>
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index f3b8ad5..2f6b29a 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -4526,6 +4526,7 @@ static virDriver remote_driver = {
> .domainGetBlockJobInfo = remoteDomainGetBlockJobInfo, /* 0.9.4 */
> .domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */
> .domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */
> + .nodeSuspendForDuration = remoteNodeSuspendForDuration, /* 0.9.7 */
0.9.8.
> +++ b/src/remote/remote_protocol.x
> @@ -2267,6 +2267,15 @@ struct remote_domain_open_graphics_args {
> unsigned int flags;
> };
>
> +struct remote_node_suspend_for_duration_args {
> + int state;
> + unsigned hyper duration;
needs a flags member.
> +};
> +
> +struct remote_node_suspend_for_duration_ret {
> + int status;
> +};
Do we really need this? Given that we return exactly two values (0 on
success, -1 on failure), I think this is like other APIs that need no
_ret object (for example, remote_domain_set_blkio_parameters_args),
because the default return values are already handled by the basic RPC
operation.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list