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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list