[libvirt] [PATCH v2 1/3] Add per-guest S3/S4 state configuration

[Eric Blake]

On 08/02/2012 06:05 AM, Martin Kletzander wrote:
> There is a new <pm/> element implemented that can control what ACPI
> sleeping states will be advertised by BIOS and allowed to be switched
> to by libvirt. The default keeps defaults on hypervisor, otherwise
> forces chosen setting.

You are proposing /domain/pm; but we also have /domain/os/bios, would
this be better as a subelement /domain/os/bios/pm, since it is related
to bios options?

> ---
>  docs/schemas/domaincommon.rng |   33 ++++++++++++++++++++++++++++++
>  src/conf/domain_conf.c        |   44 +++++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h        |   14 +++++++++++++
>  src/libvirt_private.syms      |    2 +
>  4 files changed, 93 insertions(+), 0 deletions(-)
> 

> @@ -2192,6 +2195,36 @@
>      </choice>
>    </define>
>    <!--
> +      Control ACPI sleep states (dis)allowed for the domain
> +      For each of the states the following rules apply:
> +      on: the state will be forcefully enabled
> +      off: the state will be forcefully disabled
> +      not specified: hypervisor will be left to decide its defaults
> +  -->
> +  <define name="pm">
> +    <element name="pm">
> +      <interleave>
> +        <optional>
> +          <attribute name="s3">

Is the name 's3' too x86-centric?  Remember, for
virNodeSuspendForDuration, we went with the names 'mem' (s3) and 'disk'
(s4) to give a more descriptive naming for where we were suspending.  So
controlling whether s3 is advertised is really controlling whether
'suspend-to-memory' is advertised.

> +++ b/src/conf/domain_conf.h
> @@ -1372,6 +1372,14 @@ enum virDomainLifecycleCrashAction {
>      VIR_DOMAIN_LIFECYCLE_CRASH_LAST
>  };
> 
> +enum virDomainPMState {
> +    VIR_DOMAIN_PM_STATE_DEFAULT = 0,
> +    VIR_DOMAIN_PM_STATE_ON,
> +    VIR_DOMAIN_PM_STATE_OFF,
> +
> +    VIR_DOMAIN_PM_STATE_LAST,
> +};
> +
>  enum virDomainBIOSUseserial {
>      VIR_DOMAIN_BIOS_USESERIAL_DEFAULT = 0,
>      VIR_DOMAIN_BIOS_USESERIAL_YES,
> @@ -1618,6 +1626,11 @@ struct _virDomainDef {
>      int onPoweroff;
>      int onCrash;
> 
> +    struct {
> +        int s3;
> +        int s4;

Add a comment that these values are type enum virDomainPMState.

The code itself looks decent (and thanks for the test cases), but I'd
like a second opinion on whether the choice of XML naming is the best;
if we choose a different naming scheme, I think you should be able to
adapt the rest of the patch pretty easily.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120802/cf65e6a6/attachment-0001.sig>