[libvirt] [PATCH v3 0/4] Per-guest S3/S4 configuration
There is a way to tell qemu how (not) to advertise S3/S4 ACPI sleep state capability to the guest OS. This series includes the capability to set this in the XML and also covers all the handling from qemu point of view including checking for the support, parameter parsing, command building, checking before sending the signals through guest agent and also tests and documentation. -- v3: - Option names are change according to Eric and Doug - Added docs (formatdomain) v2: - Modified the patch to reflect danpb's notes (according to qemu people the setting the disable_s[34] parameter to 0/1 ensures that the states will be enabled/disabled respectively) Martin Kletzander (4): Add per-guest S3/S4 state configuration qemu: Add support for S3/S4 state configuration tests: Add tests for qemu S3/S4 state configuration docs: Add pm element into documentation docs/formatdomain.html.in | 21 ++++ docs/schemas/domaincommon.rng | 33 ++++++ src/conf/domain_conf.c | 44 +++++++++ src/conf/domain_conf.h | 15 +++ src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 7 ++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 103 ++++++++++++++++++++ src/qemu/qemu_driver.c | 17 +++ tests/qemuargv2xmltest.c | 3 + .../qemuxml2argv-misc-disable-s3.args | 4 + .../qemuxml2argv-misc-disable-s3.xml | 27 +++++ .../qemuxml2argv-misc-disable-suspends.args | 4 + .../qemuxml2argv-misc-disable-suspends.xml | 27 +++++ .../qemuxml2argv-misc-enable-s4.args | 4 + .../qemuxml2argv-misc-enable-s4.xml | 27 +++++ tests/qemuxml2argvtest.c | 4 + tests/qemuxml2xmltest.c | 3 + 18 files changed, 347 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.xml -- 1.7.8.6
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(-) 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> + <!-- 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); + } + } + 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) { + 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; -- 1.7.8.6
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
This patch adds support for running qemu guests with the required parameters to forcefully enable or disable BIOS advertising of S3 and S4 states. The support for this is added to capabilities and there is also a qemu command parameter parsing implemented. --- src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 103 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 17 +++++++ 4 files changed, 129 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5472267..b8160b6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -172,6 +172,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "bridge", /* 100 */ "lsi", "virtio-scsi-pci", + "disable-s3", + "disable-s4", ); @@ -1403,6 +1405,7 @@ qemuCapsExtractDeviceStr(const char *qemu, "-device", "virtio-blk-pci,?", "-device", "virtio-net-pci,?", "-device", "scsi-disk,?", + "-device", "PIIX4_PM,?", NULL); /* qemu -help goes to stdout, but qemu -device ? goes to stderr. */ virCommandSetErrorBuffer(cmd, &output); @@ -1499,6 +1502,10 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags) qemuCapsSet(flags, QEMU_CAPS_SCSI_CD); if (strstr(str, "ide-cd")) qemuCapsSet(flags, QEMU_CAPS_IDE_CD); + if (strstr(str, "PIIX4_PM.disable_s3=")) + qemuCapsSet(flags, QEMU_CAPS_DISABLE_S3); + if (strstr(str, "PIIX4_PM.disable_s4=")) + qemuCapsSet(flags, QEMU_CAPS_DISABLE_S4); return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d606890..e49424a 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -138,6 +138,8 @@ enum qemuCapsFlags { QEMU_CAPS_NETDEV_BRIDGE = 100, /* bridge helper support */ QEMU_CAPS_SCSI_LSI = 101, /* -device lsi */ QEMU_CAPS_VIRTIO_SCSI_PCI = 102, /* -device virtio-scsi-pci */ + QEMU_CAPS_DISABLE_S3 = 103, /* S3 BIOS Advertisement on/off */ + QEMU_CAPS_DISABLE_S4 = 104, /* S4 BIOS Advertisement on/off */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9383530..34ee00e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4761,6 +4761,48 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-no-acpi"); } + if (def->pm.s3) { + if (qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S3)) { + if (def->pm.s3 == VIR_DOMAIN_PM_STATE_ON) + virCommandAddArgList(cmd, + "-global", + "PIIX4_PM.disable_s3=0", + NULL); + + else if (def->pm.s3 == VIR_DOMAIN_PM_STATE_OFF) + virCommandAddArgList(cmd, + "-global", + "PIIX4_PM.disable_s3=1", + NULL); + + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S3 not supported")); + goto error; + } + } + + if (def->pm.s4) { + if (qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S4)) { + if (def->pm.s4 == VIR_DOMAIN_PM_STATE_ON) + virCommandAddArgList(cmd, + "-global", + "PIIX4_PM.disable_s4=0", + NULL); + + else if (def->pm.s4 == VIR_DOMAIN_PM_STATE_OFF) + virCommandAddArgList(cmd, + "-global", + "PIIX4_PM.disable_s4=1", + NULL); + + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S4 not supported")); + goto error; + } + } + if (!def->os.bootloader) { /* * We prefer using explicit bootindex=N parameters for predictable @@ -8279,6 +8321,67 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, *monConfig = chr; } + } else if (STREQ(arg, "-global") && + STRPREFIX(progargv[i + 1], "PIIX4_PM.disable_s3=")) { + /* We want to parse only the known "-global" parameters, + * so the ones that we don't know are still added to the + * namespace */ + WANT_VALUE(); + + val += strlen("PIIX4_PM.disable_s3="); + + if (strlen(val) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid value for disable_s3 parameter: " + "'%s'"), val); + goto error; + } + + switch (*val) { + case '0': + def->pm.s3 = VIR_DOMAIN_PM_STATE_ON; + break; + + case '1': + def->pm.s3 = VIR_DOMAIN_PM_STATE_OFF; + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid value for disable_s3 parameter: " + "'%s'"), val); + goto error; + } + + } else if (STREQ(arg, "-global") && + STRPREFIX(progargv[i + 1], "PIIX4_PM.disable_s4=")) { + WANT_VALUE(); + + val += strlen("PIIX4_PM.disable_s4="); + + if (strlen(val) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid value for disable_s4 parameter: " + "'%s'"), val); + goto error; + } + + switch (*val) { + case '0': + def->pm.s4 = VIR_DOMAIN_PM_STATE_ON; + break; + + case '1': + def->pm.s4 = VIR_DOMAIN_PM_STATE_OFF; + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid value for disable_s4 parameter: " + "'%s'"), val); + goto error; + } + } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dee1268..13d5917 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13097,6 +13097,23 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, goto cleanup; } + if (vm->def->pm.s3 || vm->def->pm.s4) { + if (!vm->def->pm.s3 == VIR_DOMAIN_PM_STATE_OFF && + (target == VIR_NODE_SUSPEND_TARGET_MEM || + target == VIR_NODE_SUSPEND_TARGET_HYBRID)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("S3 signals are disabled for this domain")); + goto cleanup; + } + + if (vm->def->pm.s4 == VIR_DOMAIN_PM_STATE_OFF && + target == VIR_NODE_SUSPEND_TARGET_DISK) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("S4 signals are disabled for this domain")); + goto cleanup; + } + } + if (priv->agentError) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("QEMU guest agent is not available due to an error")); -- 1.7.8.6
On 09.08.2012 10:26, Martin Kletzander wrote:
This patch adds support for running qemu guests with the required parameters to forcefully enable or disable BIOS advertising of S3 and S4 states. The support for this is added to capabilities and there is also a qemu command parameter parsing implemented. --- src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 103 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 17 +++++++ 4 files changed, 129 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5472267..b8160b6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -172,6 +172,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "bridge", /* 100 */ "lsi", "virtio-scsi-pci", + "disable-s3", + "disable-s4",
);
@@ -1403,6 +1405,7 @@ qemuCapsExtractDeviceStr(const char *qemu, "-device", "virtio-blk-pci,?", "-device", "virtio-net-pci,?", "-device", "scsi-disk,?", + "-device", "PIIX4_PM,?", NULL); /* qemu -help goes to stdout, but qemu -device ? goes to stderr. */ virCommandSetErrorBuffer(cmd, &output); @@ -1499,6 +1502,10 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags) qemuCapsSet(flags, QEMU_CAPS_SCSI_CD); if (strstr(str, "ide-cd")) qemuCapsSet(flags, QEMU_CAPS_IDE_CD); + if (strstr(str, "PIIX4_PM.disable_s3=")) + qemuCapsSet(flags, QEMU_CAPS_DISABLE_S3); + if (strstr(str, "PIIX4_PM.disable_s4=")) + qemuCapsSet(flags, QEMU_CAPS_DISABLE_S4);
return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d606890..e49424a 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -138,6 +138,8 @@ enum qemuCapsFlags { QEMU_CAPS_NETDEV_BRIDGE = 100, /* bridge helper support */ QEMU_CAPS_SCSI_LSI = 101, /* -device lsi */ QEMU_CAPS_VIRTIO_SCSI_PCI = 102, /* -device virtio-scsi-pci */ + QEMU_CAPS_DISABLE_S3 = 103, /* S3 BIOS Advertisement on/off */ + QEMU_CAPS_DISABLE_S4 = 104, /* S4 BIOS Advertisement on/off */
QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9383530..34ee00e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4761,6 +4761,48 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-no-acpi"); }
+ if (def->pm.s3) { + if (qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S3)) { + if (def->pm.s3 == VIR_DOMAIN_PM_STATE_ON) + virCommandAddArgList(cmd, + "-global", + "PIIX4_PM.disable_s3=0", + NULL); + + else if (def->pm.s3 == VIR_DOMAIN_PM_STATE_OFF)
Redundant if. After the first condition def->pm.s3 can be either _ON or _OFF. Nothing else.
+ virCommandAddArgList(cmd, + "-global", + "PIIX4_PM.disable_s3=1", + NULL); + + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S3 not supported")); + goto error; + } + } + + if (def->pm.s4) { + if (qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S4)) { + if (def->pm.s4 == VIR_DOMAIN_PM_STATE_ON) + virCommandAddArgList(cmd, + "-global", + "PIIX4_PM.disable_s4=0", + NULL); + + else if (def->pm.s4 == VIR_DOMAIN_PM_STATE_OFF)
ditto
+ virCommandAddArgList(cmd, + "-global", + "PIIX4_PM.disable_s4=1", + NULL); + + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S4 not supported")); + goto error; + } + } + if (!def->os.bootloader) { /* * We prefer using explicit bootindex=N parameters for predictable @@ -8279,6 +8321,67 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
*monConfig = chr; } + } else if (STREQ(arg, "-global") && + STRPREFIX(progargv[i + 1], "PIIX4_PM.disable_s3=")) { + /* We want to parse only the known "-global" parameters, + * so the ones that we don't know are still added to the + * namespace */ + WANT_VALUE(); + + val += strlen("PIIX4_PM.disable_s3="); + + if (strlen(val) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR,
it's not an internal error really. s/VIR_ERR_INTERNAL_ERROR/VIR_ERR_CONFIG_UNSUPPORTED/ I know we misuse VIR_INTERNAL_ERROR in much places, but that should be fixed too.
+ _("invalid value for disable_s3 parameter: " + "'%s'"), val); + goto error; + } + + switch (*val) { + case '0': + def->pm.s3 = VIR_DOMAIN_PM_STATE_ON; + break; + + case '1': + def->pm.s3 = VIR_DOMAIN_PM_STATE_OFF; + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR,
s/VIR_ERR_INTERNAL_ERROR/VIR_ERR_CONFIG_UNSUPPORTED/ btw: what about optimizing this a bit? My intent is to join this and previous virReportError() into one: val += strlen("PIIX4_PM.disable_s3="); if (STREQ(val, "0")) def->pm.s3 = VIR_DOMAIN_PM_STATE_ON; else if (STREQ(val, "1")) def->pm.s3 = VIR_DOMAIN_PM_STATE_OFF; else virReportError(); This apply for s4 as well.
+ _("invalid value for disable_s3 parameter: " + "'%s'"), val); + goto error; + } + + } else if (STREQ(arg, "-global") && + STRPREFIX(progargv[i + 1], "PIIX4_PM.disable_s4=")) { + WANT_VALUE(); + + val += strlen("PIIX4_PM.disable_s4="); + + if (strlen(val) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid value for disable_s4 parameter: " + "'%s'"), val); + goto error; + } + + switch (*val) { + case '0': + def->pm.s4 = VIR_DOMAIN_PM_STATE_ON; + break; + + case '1': + def->pm.s4 = VIR_DOMAIN_PM_STATE_OFF; + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid value for disable_s4 parameter: " + "'%s'"), val); + goto error; + } + } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dee1268..13d5917 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13097,6 +13097,23 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, goto cleanup; }
+ if (vm->def->pm.s3 || vm->def->pm.s4) { + if (!vm->def->pm.s3 == VIR_DOMAIN_PM_STATE_OFF && + (target == VIR_NODE_SUSPEND_TARGET_MEM || + target == VIR_NODE_SUSPEND_TARGET_HYBRID)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("S3 signals are disabled for this domain"));
s/signals are/state is/
+ goto cleanup; + } + + if (vm->def->pm.s4 == VIR_DOMAIN_PM_STATE_OFF && + target == VIR_NODE_SUSPEND_TARGET_DISK) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("S4 signals are disabled for this domain"));
ditto
+ goto cleanup; + } + } + if (priv->agentError) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("QEMU guest agent is not available due to an error"));
Otherwise looking good. Michal
Few tests were added which are checking whether the parsing of the xml and command-line arguments is working and compatible with each other. --- tests/qemuargv2xmltest.c | 3 ++ .../qemuxml2argv-misc-disable-s3.args | 4 +++ .../qemuxml2argv-misc-disable-s3.xml | 27 ++++++++++++++++++++ .../qemuxml2argv-misc-disable-suspends.args | 4 +++ .../qemuxml2argv-misc-disable-suspends.xml | 27 ++++++++++++++++++++ .../qemuxml2argv-misc-enable-s4.args | 4 +++ .../qemuxml2argv-misc-enable-s4.xml | 27 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ tests/qemuxml2xmltest.c | 3 ++ 9 files changed, 103 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.xml diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 439218e..ad5f45b 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -205,6 +205,9 @@ mymain(void) /* Can't rountrip xenner arch */ /*DO_TEST("input-xen");*/ DO_TEST("misc-acpi"); + DO_TEST("misc-disable-s3"); + DO_TEST("misc-disable-suspends"); + DO_TEST("misc-enable-s4"); DO_TEST("misc-no-reboot"); DO_TEST("misc-uuid"); DO_TEST("net-user"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.args b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.args new file mode 100644 index 0000000..dbee64d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S \ +-M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -global PIIX4_PM.disable_s3=1 -boot c -hda /dev/HostVG/QEMUGuest1 \ +-net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.xml b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.xml new file mode 100644 index 0000000..b829321 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>8caaa98c-e7bf-5845-126a-1fc316bd1089</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <pm suspend-to-mem='off'/> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.args b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.args new file mode 100644 index 0000000..6f63d8b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S \ +-M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -global PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot c \ +-hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml new file mode 100644 index 0000000..f61a267 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>8caaa98c-e7bf-5845-126a-1fc316bd1089</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <pm suspend-to-mem='off' suspend-to-disk='off'/> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.args b/tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.args new file mode 100644 index 0000000..b54e7b8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S \ +-M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -global PIIX4_PM.disable_s4=0 -boot c -hda /dev/HostVG/QEMUGuest1 \ +-net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.xml b/tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.xml new file mode 100644 index 0000000..5e14bbc --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>8caaa98c-e7bf-5845-126a-1fc316bd1089</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <pm suspend-to-disk='on'/> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f8d8db5..022792f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -554,6 +554,10 @@ mymain(void) DO_TEST("input-usbtablet", NONE); DO_TEST_ERROR("input-xen", QEMU_CAPS_DOMID); DO_TEST("misc-acpi", NONE); + DO_TEST("misc-disable-s3", QEMU_CAPS_DISABLE_S3); + DO_TEST("misc-disable-suspends", QEMU_CAPS_DISABLE_S3, QEMU_CAPS_DISABLE_S4); + DO_TEST("misc-enable-s4", QEMU_CAPS_DISABLE_S4); + DO_TEST_FAILURE("misc-enable-s4", NONE); DO_TEST("misc-no-reboot", NONE); DO_TEST("misc-uuid", QEMU_CAPS_NAME, QEMU_CAPS_UUID); DO_TEST("net-user", NONE); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index dcdba4f..b60736f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -170,6 +170,9 @@ mymain(void) DO_TEST("input-usbtablet"); DO_TEST("input-xen"); DO_TEST("misc-acpi"); + DO_TEST("misc-disable-s3"); + DO_TEST("misc-disable-suspends"); + DO_TEST("misc-enable-s4"); DO_TEST("misc-no-reboot"); DO_TEST("net-user"); DO_TEST("net-virtio"); -- 1.7.8.6
On 09.08.2012 10:26, Martin Kletzander wrote:
Few tests were added which are checking whether the parsing of the xml and command-line arguments is working and compatible with each other. --- tests/qemuargv2xmltest.c | 3 ++ .../qemuxml2argv-misc-disable-s3.args | 4 +++ .../qemuxml2argv-misc-disable-s3.xml | 27 ++++++++++++++++++++ .../qemuxml2argv-misc-disable-suspends.args | 4 +++ .../qemuxml2argv-misc-disable-suspends.xml | 27 ++++++++++++++++++++ .../qemuxml2argv-misc-enable-s4.args | 4 +++ .../qemuxml2argv-misc-enable-s4.xml | 27 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ tests/qemuxml2xmltest.c | 3 ++ 9 files changed, 103 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.xml
ACK Michal
--- docs/formatdomain.html.in | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f97c630..50c4783 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -922,6 +922,27 @@ domain will be restarted with the same configuration</dd> </dl> + <h3><a name="elementsPowerManagement">Power Management</a></h3> + + <p> + <span class="since">Since 0.10.1</span> it is possible to + forcibly enable or disable BIOS advertisements to the guest + OS. (NB: Only qemu driver support) + </p> + +<pre> + ... + <pm suspend-to-disk='off' suspend-to-ram='on' /> + ...</pre> + + <dl> + <dt><code>pm</code></dt> + <dd>This element enables ('on') or disables ('off') BIOS support + for S3 (suspend-to-disk) and S4 (suspend-to-mem) ACPI sleep + states. If no value is specified, then the hypervison will be + left with its default value.</dd> + </dl> + <h3><a name="elementsFeatures">Hypervisor features</a></h3> <p> -- 1.7.8.6
On 09.08.2012 10:26, Martin Kletzander wrote:
--- docs/formatdomain.html.in | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f97c630..50c4783 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -922,6 +922,27 @@ domain will be restarted with the same configuration</dd> </dl>
+ <h3><a name="elementsPowerManagement">Power Management</a></h3> + + <p> + <span class="since">Since 0.10.1</span> it is possible to + forcibly enable or disable BIOS advertisements to the guest + OS. (NB: Only qemu driver support) + </p> + +<pre> + ... + <pm suspend-to-disk='off' suspend-to-ram='on' /> + ...</pre> + + <dl> + <dt><code>pm</code></dt> + <dd>This element enables ('on') or disables ('off') BIOS support + for S3 (suspend-to-disk) and S4 (suspend-to-mem) ACPI sleep + states. If no value is specified, then the hypervison will be
s/hypervison/hypervisor/
+ left with its default value.</dd> + </dl> + <h3><a name="elementsFeatures">Hypervisor features</a></h3>
<p>
ACK
On 08/22/2012 07:37 AM, Michal Privoznik wrote:
On 09.08.2012 10:26, Martin Kletzander wrote:
--- docs/formatdomain.html.in | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f97c630..50c4783 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -922,6 +922,27 @@ domain will be restarted with the same configuration</dd> </dl>
+ <h3><a name="elementsPowerManagement">Power Management</a></h3> + + <p> + <span class="since">Since 0.10.1</span> it is possible to + forcibly enable or disable BIOS advertisements to the guest + OS. (NB: Only qemu driver support)
Are we trying to get this into 0.10.0 (even though it missed RC1 freeze) or deferring it to 0.10.1? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/22/2012 05:08 PM, Eric Blake wrote:
On 08/22/2012 07:37 AM, Michal Privoznik wrote:
On 09.08.2012 10:26, Martin Kletzander wrote:
--- docs/formatdomain.html.in | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f97c630..50c4783 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -922,6 +922,27 @@ domain will be restarted with the same configuration</dd> </dl>
+ <h3><a name="elementsPowerManagement">Power Management</a></h3> + + <p> + <span class="since">Since 0.10.1</span> it is possible to + forcibly enable or disable BIOS advertisements to the guest + OS. (NB: Only qemu driver support)
Are we trying to get this into 0.10.0 (even though it missed RC1 freeze) or deferring it to 0.10.1?
Nope, we didn't make it => it has to wait, no point in rushing it. I've missed the version number, so at least one thing I don't have to fix ;) I'll fix the other things Michal pointed out and it can wait till the next one. Thanks for considering it, though. Martin
participants (3)
-
Eric Blake -
Martin Kletzander -
Michal Privoznik