On 09.08.2012 10:26, 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. --- docs/schemas/domaincommon.rng | 33 ++++++++++++++++++++++++++++++ src/conf/domain_conf.c | 44 +++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 15 ++++++++++++++ src/libvirt_private.syms | 2 + 4 files changed, 94 insertions(+), 0 deletions(-)
As I've mentioned in one of previous series - I'd like to see documentation and XML extension in one patch as it's advised here: http://libvirt.org/api_extension.html But then again, having a separate patch within set is okay too.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c85d763..d0c6d47 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -53,6 +53,9 @@ <ref name="features"/> <ref name="termination"/> <optional> + <ref name="pm"/> + </optional> + <optional> <ref name="devices"/> </optional> <optional> @@ -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="suspend-to-mem"> + <ref name="suspendChoices"/> + </attribute> + </optional> + <optional> + <attribute name="suspend-to-disk"> + <ref name="suspendChoices"/> + </attribute> + </optional> + </interleave> + <empty/> + </element> + </define> + <define name="suspendChoices"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </define> + <!--
Let's hope we won't need any other configurable knobs for s3/s4; otherwise we should move from attributes to elements: <pm> <s3 enabled='yes'/> <s4 enabled='no'/> </pm>
Specific setup for a qemu emulated character device. Note: this definition doesn't fully specify the constraints on this node. --> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d8c0969..04e0be5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -124,6 +124,11 @@ VIR_ENUM_IMPL(virDomainLifecycleCrash, VIR_DOMAIN_LIFECYCLE_CRASH_LAST, "coredump-destroy", "coredump-restart")
+VIR_ENUM_IMPL(virDomainPMState, VIR_DOMAIN_PM_STATE_LAST, + "default", + "on", + "off") + VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "none", "disk", @@ -8413,6 +8418,19 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainLifecycleCrashTypeFromString) < 0) goto error;
+ if (virXPathNode("./pm", ctxt)) { + tmp = virXPathString("string(./pm/@suspend-to-mem)", ctxt); + if (tmp) { + def->pm.s3 = virDomainPMStateTypeFromString(tmp); + VIR_FREE(tmp); + } + tmp = virXPathString("string(./pm/@suspend-to-disk)", ctxt); + if (tmp) { + def->pm.s4 = virDomainPMStateTypeFromString(tmp); + VIR_FREE(tmp); + } + } +
vir*TypeFromString() may fail and return < 0 value. We should not silently ignore not allowed values here. NB please don't use VIR_ERR_INTERNAL_ERROR as it is not an internal error but VIR_ERR_CONFIG_UNSUPPORTED.
tmp = virXPathString("string(./clock/@offset)", ctxt); if (tmp) { if ((def->clock.offset = virDomainClockOffsetTypeFromString(tmp)) < 0) { @@ -13092,6 +13110,32 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainLifecycleCrashTypeToString) < 0) goto cleanup;
+ if (def->pm.s3 || def->pm.s4) { + virBufferAddLit(buf, " <pm"); + + if (def->pm.s3) { + const char *tmp = virDomainPMStateTypeToString(def->pm.s3); + if (!tmp) {
This is useless. Libvirt must validate its inputs (esp. those from user). Outputs are believed to be trusted - that is - once we validate input and set def->pm.s3 to one of allowed values, nobody will hop into our address space and change it. It he will anyway, it's problem.
+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected PM state %d"), def->pm.s3); + goto cleanup; + } + virBufferAsprintf(buf, " suspend-to-mem='%s'", tmp); + } + + if (def->pm.s4) { + const char *tmp = virDomainPMStateTypeToString(def->pm.s4); + if (!tmp) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected PM state %d"), def->pm.s4); + goto cleanup; + } + virBufferAsprintf(buf, " suspend-to-disk='%s'", tmp); + } + + virBufferAddLit(buf, "/>\n"); + } + virBufferAddLit(buf, " <devices>\n");
virBufferEscapeString(buf, " <emulator>%s</emulator>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3f25ad2..ecca72f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1373,6 +1373,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, @@ -1619,6 +1627,12 @@ struct _virDomainDef { int onPoweroff; int onCrash;
+ struct { + /* These options are actually type of enum virDomainPMState */ + int s3; + int s4; + } pm; + virDomainOSDef os; char *emulator; int features; @@ -2166,6 +2180,7 @@ VIR_ENUM_DECL(virDomainBoot) VIR_ENUM_DECL(virDomainFeature) VIR_ENUM_DECL(virDomainLifecycle) VIR_ENUM_DECL(virDomainLifecycleCrash) +VIR_ENUM_DECL(virDomainPMState) VIR_ENUM_DECL(virDomainDevice) VIR_ENUM_DECL(virDomainDeviceAddress) VIR_ENUM_DECL(virDomainDeviceAddressPciMulti) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 79b4a18..d8f409a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -437,6 +437,8 @@ virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; virDomainPciRombarModeTypeFromString; virDomainPciRombarModeTypeToString; +virDomainPMStateTypeFromString; +virDomainPMStateTypeToString; virDomainRedirdevBusTypeFromString; virDomainRedirdevBusTypeToString; virDomainRemoveInactive;
Otherwise looking good. Michal