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

Re: [libvirt] [PATCH v4 1/4] Add a public API to invoke suspend/resume on the host



On Mon, Nov 28, 2011 at 05:33:06PM +0530, Srivatsa S. Bhat wrote:
> Implement the public definitions for the new API
> virNodeSuspendForDuration() which will be subsequently used to
> do a timed suspend on the host.
> 
> Signed-off-by: Srivatsa S. Bhat <srivatsa bhat linux vnet ibm com>
> ---
> 
>  include/libvirt/libvirt.h.in |   17 +++++++++++-
>  src/driver.h                 |    6 ++++
>  src/libvirt.c                |   61 ++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms      |    5 +++
>  4 files changed, 88 insertions(+), 1 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 2ab89f5..ad9e8f1 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -249,7 +249,17 @@ typedef enum {
>      VIR_DOMAIN_START_FORCE_BOOT   = 1 << 3, /* Boot, discarding any managed save */
>  } virDomainCreateFlags;
>  
> -
> +/**
> + * virNodeSuspendState:
> + *
> + * Flags to indicate which system-wide sleep state the host must be
> + * transitioned to.
> + */
> +typedef enum {
> +    VIR_NODE_S3             = (1 << 0),              /* Suspend-to-RAM */
> +    VIR_NODE_S4             = (1 << 1),              /* Suspend-to-Disk */
> +    VIR_NODE_HYBRID_SUSPEND = (1 << 2),              /* Hybrid-Suspend */
> +} virNodeSuspendState;

It is desirable for the constant prefix to be consistent & match the
function name. In addition S3/S4 are x86 specific terminology. Lets
actually use descriptive names which are architecture indepedant,
whereupon you don't need comments to explain what the codename
means.

eg

   typedef enum {
       VIR_NODE_SUSPEND_TARGET_RAM     = (1 << 0),
       VIR_NODE_SUSPEND_TARGET_DISK    = (1 << 1),
       VIR_NODE_SUSPEND_TARGET_HYBRID  = (1 << 2)
   } virNodeSuspendTarget;


>  
>  /**
>   * virStream:
> @@ -1085,6 +1095,11 @@ unsigned long long      virNodeGetFreeMemory    (virConnectPtr conn);
>  int                     virNodeGetSecurityModel (virConnectPtr conn,
>                                                   virSecurityModelPtr secmodel);
>  
> +int                     virNodeSuspendForDuration (virConnectPtr conn,
> +                                                   int state,
> +                                                   unsigned long long duration,
> +                                                   unsigned int flags);

'state' is better named 'target' IMHO, and can be unsigned, eg.

   int                     virNodeSuspendForDuration (virConnectPtr conn,
                                                      unsigned int target,
                                                      unsigned long long duration,
                                                      unsigned int flags);


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 :|


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