[libvirt] [PATCH v3 1/4] Add per-guest S3/S4 state configuration
Michal Privoznik
mprivozn at redhat.com
Wed Aug 22 13:37:38 UTC 2012
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
More information about the libvir-list
mailing list