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

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(a)linux.vnet.ibm.com&gt; > > --- > > > > 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.