Re: [PATCH] Add 'Hybrid-Suspend' power management discovery for the host

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

 



On Tue, Nov 29, 2011 at 08:44:11AM +0000, Daniel P. Berrange wrote:
> On Tue, Nov 29, 2011 at 03:07:29PM +0800, Daniel Veillard wrote:
> > On Thu, Nov 24, 2011 at 02:22:41PM +0530, Srivatsa S. Bhat wrote:
> > > Some systems support a feature known as 'Hybrid-Suspend', apart from the
> > > usual system-wide sleep states such as Suspend-to-RAM (S3) or Suspend-to-Disk
> > > (S4). Add the functionality to discover this power management feature and
> > > export it in the capabilities XML under the <power_management> tag.
> > > 
> > > This addition was suggested in [1].
> > > 
> > > References:
> > > 1. http://www.redhat.com/archives/libvir-list/2011-November/msg01298.html
> > > 
> > > Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@xxxxxxxxxxxxxxxxxx>
> > > ---
> > > 
> > >  docs/formatcaps.html.in     |   12 +++--
> > >  docs/schemas/capability.rng |    5 ++
> > >  src/conf/capabilities.c     |    2 -
> > >  src/util/util.c             |   99 ++++++++++++++++++++++++++++++-------------
> > >  src/util/util.h             |    8 ++-
> > >  5 files changed, 88 insertions(+), 38 deletions(-)
> > > 
> > > diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
> > > index ce6f9a6..c5fb925 100644
> > > --- a/docs/formatcaps.html.in
> > > +++ b/docs/formatcaps.html.in
> > > @@ -31,6 +31,7 @@ BIOS you will see</p>
> > >      &lt;power_management&gt;
> > >        &lt;S3/&gt;
> > >        &lt;S4/&gt;
> > > +      &lt;Hybrid-Suspend/&gt;
> > >      &lt;power_management/&gt;
> > >    &lt;/host&gt;</span>
> > >  
> > > @@ -70,11 +71,12 @@ the host platform, but other information may be available, it shows the CPU arch
> > >  topology, model name, and additional features which are not included in the model but the
> > >  CPU provides them. Features of the chip are shown within the feature block (the block is
> > >  similar to what you will find in a Xen fully virtualized domain description). Further,
> > > -the power management features supported by the host are shown, such as Suspend-to-RAM (S3)
> > > -and Suspend-to-Disk (S4). In case the query for power management features succeeded but the
> > > -host does not support any such feature, then an empty &lt;power_management/&gt;
> > > -tag will be shown. Otherwise, if the query itself failed, no such tag will
> > > -be displayed (i.e., there will not be any power_management block or empty tag in the XML).</p>
> > > +the power management features supported by the host are shown, such as Suspend-to-RAM (S3),
> > > +Suspend-to-Disk (S4) and Hybrid-Suspend (a combination of S3 and S4).
> > > +In case the query for power management features succeeded but the host does not support
> > > +any such feature, then an empty &lt;power_management/&gt; tag will be shown. Otherwise,
> > > +if the query itself failed, no such tag will be displayed (i.e., there will not be any
> > > +power_management block or empty tag in the XML).</p>
> > >          <p>The second block (in blue) indicates the paravirtualization support of the
> > >  Xen support, you will see the os_type of xen to indicate a paravirtual
> > >  kernel, then architecture information and potential features.</p>
> > > diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> > > index 645769e..6cf2188 100644
> > > --- a/docs/schemas/capability.rng
> > > +++ b/docs/schemas/capability.rng
> > > @@ -121,6 +121,11 @@
> > >              <empty/>
> > >            </element>
> > >          </optional>
> > > +        <optional>
> > > +          <element name='Hybrid-Suspend'>
> > > +            <empty/>
> > > +          </element>
> > > +        </optional>
> > >        </interleave>
> > >      </element>
> > >    </define>
> > > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > > index 87b60b0..8468861 100644
> > > --- a/src/conf/capabilities.c
> > > +++ b/src/conf/capabilities.c
> > > @@ -35,7 +35,7 @@
> > >  #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
> > >  
> > >  VIR_ENUM_IMPL(virHostPMCapability, VIR_HOST_PM_LAST,
> > > -              "S3", "S4")
> > > +              "S3", "S4", "Hybrid-Suspend")
> > >  
> > >  /**
> > >   * virCapabilitiesNew:
> > > diff --git a/src/util/util.c b/src/util/util.c
> > > index ce697fb..ccd1a0e 100644
> > > --- a/src/util/util.c
> > > +++ b/src/util/util.c
> > > @@ -2623,47 +2623,50 @@ virTypedParameterArrayClear(virTypedParameterPtr params, int nparams)
> > >  }
> > >  
> > >  /**
> > > - * Get the Power Management Capabilities of the host system.
> > > - * The script 'pm-is-supported' (from the pm-utils package) is run
> > > - * to find out all the power management features supported by the host,
> > > - * such as Suspend-to-RAM (S3) and Suspend-to-Disk (S4).
> > > + * virDiscoverHostPMFeature:
> > > + * @bitmask: The bitmask which should be populated with the result of
> > > + *           the query
> > > + * @feature: The power management feature to check whether it is supported
> > > + *           by the host. Values could be:
> > > + *           VIR_HOST_PM_S3 for Suspend-to-RAM
> > > + *           VIR_HOST_PM_S4 for Suspend-to-Disk
> > > + *           VIR_HOST_PM_HYBRID_SUSPEND for Hybrid-Suspend
> > >   *
> > > - * @bitmask: Pointer to the bitmask which will be set appropriately to
> > > - *           indicate all the supported host power management features.
> > > + * Run the script 'pm-is-supported' (from the pm-utils package)
> > > + * to find out if @feature is supported by the host.
> > >   *
> > > - * Returns 0 if the query was successful, -1 upon failure.
> > > + * Returns 0 if the query was successful, -1 on failure.
> > >   */
> > >  int
> > > -virGetPMCapabilities(unsigned int *bitmask)
> > > +virDiscoverHostPMFeature(unsigned int *bitmask, unsigned int feature)
> > >  {
> > > -    int ret = -1;
> > > -    int status;
> > >      virCommandPtr cmd;
> > > +    int status;
> > > +    int ret = -1;
> > >  
> > > -    *bitmask = 0;
> > > -
> > > -    /* Check support for Suspend-to-RAM (S3) */
> > > -    cmd = virCommandNewArgList("pm-is-supported", "--suspend", NULL);
> > > -    if (virCommandRun(cmd, &status) < 0)
> > > -        goto cleanup;
> > > -
> > > -    /* Check return code of command == 0 for success
> > > -     * (i.e., the PM capability is supported)
> > > -     */
> > > -    if (status == 0)
> > > -        *bitmask |= 1U << VIR_HOST_PM_S3;
> > > -    virCommandFree(cmd);
> > > +    switch (feature) {
> > > +    case VIR_HOST_PM_S3:
> > > +        cmd = virCommandNewArgList("pm-is-supported", "--suspend", NULL);
> > > +        break;
> > > +    case VIR_HOST_PM_S4:
> > > +        cmd = virCommandNewArgList("pm-is-supported", "--hibernate", NULL);
> > > +        break;
> > > +    case VIR_HOST_PM_HYBRID_SUSPEND:
> > > +        cmd = virCommandNewArgList("pm-is-supported", "--suspend-hybrid", NULL);
> > > +        break;
> > > +    default:
> > > +        return ret;
> > > +    }
> > >  
> > > -    /* Check support for Suspend-to-Disk (S4) */
> > > -    cmd = virCommandNewArgList("pm-is-supported", "--hibernate", NULL);
> > >      if (virCommandRun(cmd, &status) < 0)
> > >          goto cleanup;
> > >  
> > > -    /* Check return code of command == 0 for success
> > > -     * (i.e., the PM capability is supported)
> > > -     */
> > > +   /*
> > > +    * Check return code of command == 0 for success
> > > +    * (i.e., the PM capability is supported)
> > > +    */
> > >      if (status == 0)
> > > -        *bitmask |= 1U << VIR_HOST_PM_S4;
> > > +        *bitmask |= 1U << feature;
> > >  
> > >      ret = 0;
> > >  
> > > @@ -2671,3 +2674,41 @@ cleanup:
> > >      virCommandFree(cmd);
> > >      return ret;
> > >  }
> > > +
> > > +/**
> > > + * virGetPMCapabilities:
> > > + *
> > > + * Get the Power Management Capabilities that the host system supports,
> > > + * such as Suspend-to-RAM (S3), Suspend-to-Disk (S4) and Hybrid-Suspend
> > > + * (a combination of S3 and S4).
> > > + *
> > > + * @bitmask: Pointer to the bitmask which will be set appropriately to
> > > + *           indicate all the supported host power management features.
> > > + *
> > > + * Returns 0 if the query was successful, -1 on failure.
> > > + */
> > > +int
> > > +virGetPMCapabilities(unsigned int *bitmask)
> > > +{
> > > +    int ret;
> > > +
> > > +    *bitmask = 0;
> > > +
> > > +    /* Check support for Suspend-to-RAM (S3) */
> > > +    ret = virDiscoverHostPMFeature(bitmask, VIR_HOST_PM_S3);
> > > +    if (ret < 0)
> > > +        return -1;
> > > +
> > > +    /* Check support for Suspend-to-Disk (S4) */
> > > +    ret = virDiscoverHostPMFeature(bitmask, VIR_HOST_PM_S4);
> > > +    if (ret < 0)
> > > +        return -1;
> > > +
> > > +    /* Check support for Hybrid-Suspend */
> > > +    ret = virDiscoverHostPMFeature(bitmask, VIR_HOST_PM_HYBRID_SUSPEND);
> > > +    if (ret < 0)
> > > +        return -1;
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > diff --git a/src/util/util.h b/src/util/util.h
> > > index 5afcf58..eda60d2 100644
> > > --- a/src/util/util.h
> > > +++ b/src/util/util.h
> > > @@ -264,14 +264,16 @@ void virTypedParameterArrayClear(virTypedParameterPtr params, int nparams);
> > >  /* Power Management Capabilities of the host system */
> > >  
> > >  enum virHostPMCapability {
> > > -    VIR_HOST_PM_S3,  /* Suspend-to-RAM */
> > > -    VIR_HOST_PM_S4,  /* Suspend-to-Disk */
> > > +    VIR_HOST_PM_S3,              /* Suspend-to-RAM */
> > > +    VIR_HOST_PM_S4,              /* Suspend-to-Disk */
> > > +    VIR_HOST_PM_HYBRID_SUSPEND,  /* Hybrid-Suspend */
> > >  
> > >      VIR_HOST_PM_LAST
> > >  };
> > >  
> > >  VIR_ENUM_DECL(virHostPMCapability)
> > >  
> > > -int virGetPMCapabilities(unsigned int *);
> > > +int virDiscoverHostPMFeature(unsigned int *bitmask, unsigned int feature);
> > > +int virGetPMCapabilities(unsigned int *bitmask);
> > 
> >   ACK, that looks reasonable, I actually have machines with and without
> >   that mode. I had to cleanup the patch a bit to apply to the
> >   documentation and pass "make synatx-check" though, I will push later,
> 
> Please don't push this version of the patch - there was a newer version
> posted with many changes.

Oh actually sorry, I was confusing this patch with the other PM related
patch. Looking at the previously applied patch there is actually a
little naming change I want to make to that, but I'll send a cleanup
patch todo that once you have applied this patch.

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]