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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list