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

Re: [libvirt] [PATCH v2 1/3] Introduce virDomainPMWakeup API



On 02/15/2012 08:04 AM, Michal Privoznik wrote:
> This API allows a domain which previously called
> virDomainPMSuspendForDuration() to be woken up.
> ---
>  include/libvirt/libvirt.h.in |    2 +
>  src/driver.h                 |    4 +++
>  src/libvirt.c                |   50 ++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms      |    1 +
>  src/remote/remote_driver.c   |    1 +
>  src/remote/remote_protocol.x |    8 ++++++-
>  src/remote_protocol-structs  |    5 ++++
>  7 files changed, 70 insertions(+), 1 deletions(-)
> 

> @@ -868,6 +871,7 @@ struct _virDriver {
>      virDrvDomainLookupByName	domainLookupByName;
>      virDrvDomainSuspend		domainSuspend;
>      virDrvDomainPMSuspendForDuration domainPMSuspendForDuration;
> +    virDrvDomainPMWakeup    domainPMWakeup;
>      virDrvDomainResume		domainResume;

It looks weird to have Suspend/Resume (one logical pair) split in the
middle by PMSuspend/PMWakeup (another logical pair).  Please swap things
to put domainResume before either of the domainPM* members.

> +++ b/src/libvirt_public.syms
> @@ -523,6 +523,7 @@ LIBVIRT_0.9.10 {
>          virDomainGetDiskErrors;
>          virDomainGetMetadata;
>          virDomainPMSuspendForDuration;
> +        virDomainPMWakeup;

You already caught your error here.  :)

> +++ b/src/remote/remote_driver.c
> @@ -4782,6 +4782,7 @@ static virDriver remote_driver = {
>      .domainSuspend = remoteDomainSuspend, /* 0.3.0 */
>      .domainResume = remoteDomainResume, /* 0.3.0 */
>      .domainPMSuspendForDuration = remoteDomainPMSuspendForDuration, /* 0.9.10 */
> +    .domainPMWakeup = remoteDomainPMWakeup, /* 0.9.11 */
>      .domainShutdown = remoteDomainShutdown, /* 0.3.0 */

Another place worth re-shuffling.

> +++ b/src/remote/remote_protocol.x
> @@ -759,6 +759,11 @@ struct remote_domain_pm_suspend_for_duration_args {
>      unsigned int flags;
>  };
>  
> +struct remote_domain_pm_wakeup_args {
> +    remote_nonnull_domain dom;
> +    unsigned int flags;
> +};
> +
>  struct remote_domain_resume_args {

And another.

But the shuffling should be trivial, so I'm okay without having to see a v3.

ACK with nits fixed

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

Attachment: signature.asc
Description: OpenPGP digital signature


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