[libvirt] [PATCH v4 0/3] 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. -- Even though the patches were half-acked, I rather send them again with the changes made after Michal's suggestions. v4: - Changed the XML config to subelements - Minor fixes and optimalizations 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 (3): 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/formatdomain.html.in | 24 +++++++++ docs/schemas/domaincommon.rng | 39 ++++++++++++++ src/conf/domain_conf.c | 50 +++++++++++++++++ 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 | 62 ++++++++++++++++++++++ src/qemu/qemu_driver.c | 17 ++++++ tests/qemuargv2xmltest.c | 3 ++ .../qemuxml2argv-misc-disable-s3.args | 4 ++ .../qemuxml2argv-misc-disable-s3.xml | 29 ++++++++++ .../qemuxml2argv-misc-disable-suspends.args | 4 ++ .../qemuxml2argv-misc-disable-suspends.xml | 30 +++++++++++ .../qemuxml2argv-misc-enable-s4.args | 4 ++ .../qemuxml2argv-misc-enable-s4.xml | 29 ++++++++++ tests/qemuxml2argvtest.c | 4 ++ tests/qemuxml2xmltest.c | 3 ++ 18 files changed, 328 insertions(+) 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.12
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. The documentation of the pm element is added as well. --- docs/formatdomain.html.in | 24 +++++++++++++++++++++ docs/schemas/domaincommon.rng | 39 +++++++++++++++++++++++++++++++++ src/conf/domain_conf.c | 50 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 15 +++++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 130 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 671ce17..810c5a6 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -960,6 +960,30 @@ 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.2</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 enabled='no'/> + <suspend-to-ram enabled='yes'/> + </pm> + ...</pre> + + <dl> + <dt><code>pm</code></dt> + <dd>These elements enable ('on') or disable ('off') BIOS support + for S3 (suspend-to-disk) and S4 (suspend-to-mem) ACPI sleep + states. If nothing is specified, then the hypervisor will be + left with its default value.</dd> + </dl> + <h3><a name="elementsFeatures">Hypervisor features</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 145caf7..6b7113c 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> <zeroOrMore> @@ -2255,6 +2258,42 @@ </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> + <element name="suspend-to-mem"> + <ref name="suspendChoices"/> + </element> + </optional> + <optional> + <element name="suspend-to-disk"> + <ref name="suspendChoices"/> + </element> + </optional> + </interleave> + <empty/> + </element> + </define> + <define name="suspendChoices"> + <interleave> + <optional> + <attribute name="enabled"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + </interleave> + </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 49327df..12fb3a9 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", + "yes", + "no") + VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "none", "disk", @@ -7211,6 +7216,28 @@ static int virDomainLifecycleParseXML(xmlXPathContextPtr ctxt, return 0; } +static int +virDomainPMStateParseXML(xmlXPathContextPtr ctxt, + const char *xpath, + int *val) +{ + int ret = -1; + char *tmp = virXPathString(xpath, ctxt); + if (tmp) { + *val = virDomainPMStateTypeFromString(tmp); + if (*val < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown PM state value %s"), tmp); + goto cleanup; + } + } + + ret = 0; + cleanup: + VIR_FREE(tmp); + return ret; +} + virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, virDomainDefPtr def, const char *xmlStr, @@ -8583,6 +8610,16 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainLifecycleCrashTypeFromString) < 0) goto error; + if (virDomainPMStateParseXML(ctxt, + "string(./pm/suspend-to-mem/@enabled)", + &def->pm.s3) < 0) + goto error; + + if (virDomainPMStateParseXML(ctxt, + "string(./pm/suspend-to-disk/@enabled)", + &def->pm.s4) < 0) + goto error; + tmp = virXPathString("string(./clock/@offset)", ctxt); if (tmp) { if ((def->clock.offset = virDomainClockOffsetTypeFromString(tmp)) < 0) { @@ -13375,6 +13412,19 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainLifecycleCrashTypeToString) < 0) goto cleanup; + if (def->pm.s3 || def->pm.s4) { + virBufferAddLit(buf, " <pm>\n"); + if (def->pm.s3) { + virBufferAsprintf(buf, " <suspend-to-mem enabled='%s'/>\n", + virDomainPMStateTypeToString(def->pm.s3)); + } + if (def->pm.s4) { + virBufferAsprintf(buf, " <suspend-to-disk enabled='%s'/>\n", + virDomainPMStateTypeToString(def->pm.s4)); + } + virBufferAddLit(buf, " </pm>\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 034bebf..2f88ccc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1366,6 +1366,14 @@ enum virDomainLifecycleCrashAction { VIR_DOMAIN_LIFECYCLE_CRASH_LAST }; +enum virDomainPMState { + VIR_DOMAIN_PM_STATE_DEFAULT = 0, + VIR_DOMAIN_PM_STATE_ENABLED, + VIR_DOMAIN_PM_STATE_DISABLED, + + VIR_DOMAIN_PM_STATE_LAST, +}; + enum virDomainBIOSUseserial { VIR_DOMAIN_BIOS_USESERIAL_DEFAULT = 0, VIR_DOMAIN_BIOS_USESERIAL_YES, @@ -1620,6 +1628,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; @@ -2087,6 +2101,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(virDomainDisk) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6f14763..65067d6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -453,6 +453,8 @@ virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; virDomainPciRombarModeTypeFromString; virDomainPciRombarModeTypeToString; +virDomainPMStateTypeFromString; +virDomainPMStateTypeToString; virDomainRedirdevBusTypeFromString; virDomainRedirdevBusTypeToString; virDomainRemoveInactive; -- 1.7.12
On 08/31/2012 07:59 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. The documentation of the pm element is added as well. ---
+<pre> + ... + <pm> + <suspend-to-disk enabled='no'/> + <suspend-to-ram enabled='yes'/>
'no' and 'yes' here...
+ </pm> + ...</pre> + + <dl> + <dt><code>pm</code></dt> + <dd>These elements enable ('on') or disable ('off') BIOS support
'on' and 'off' here...
<!-- + 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="suspendChoices"> + <interleave> + <optional> + <attribute name="enabled"> + <choice> + <value>yes</value> + <value>no</value>
...back to 'yes' and 'no' here. Fix the .html.in version to use the right naming.
+ </choice> + </attribute> + </optional> + </interleave>
The <interleave> layer is not necessary here (you only have one sub-entry; furthermore, the sub-entry is an <attribute> which is already auto-interleaved; only <element> entries need interleaving). ACK with those tweaks. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/31/2012 07:42 PM, Eric Blake wrote:
On 08/31/2012 07:59 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. The documentation of the pm element is added as well. ---
+<pre> + ... + <pm> + <suspend-to-disk enabled='no'/> + <suspend-to-ram enabled='yes'/>
'no' and 'yes' here...
+ </pm> + ...</pre> + + <dl> + <dt><code>pm</code></dt> + <dd>These elements enable ('on') or disable ('off') BIOS support
'on' and 'off' here...
<!-- + 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="suspendChoices"> + <interleave> + <optional> + <attribute name="enabled"> + <choice> + <value>yes</value> + <value>no</value>
...back to 'yes' and 'no' here. Fix the .html.in version to use the right naming.
+ </choice> + </attribute> + </optional> + </interleave>
The <interleave> layer is not necessary here (you only have one sub-entry; furthermore, the sub-entry is an <attribute> which is already auto-interleaved; only <element> entries need interleaving).
ACK with those tweaks.
Fixed and pushed, thanks. Martin
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 | 62 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 17 ++++++++++++ 4 files changed, 88 insertions(+) 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 25f2451..d4217fa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4782,6 +4782,32 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-no-acpi"); } + if (def->pm.s3) { + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S3)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S3 not supported")); + goto error; + } + virCommandAddArgList(cmd, + "-global", + (def->pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED ? + "PIIX4_PM.disable_s3=0" : "PIIX4_PM.disable_s3=1"), + NULL); + } + + if (def->pm.s4) { + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S4)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S4 not supported")); + goto error; + } + virCommandAddArgList(cmd, + "-global", + (def->pm.s4 == VIR_DOMAIN_PM_STATE_ENABLED ? + "PIIX4_PM.disable_s4=0" : "PIIX4_PM.disable_s4=1"), + NULL); + } + if (!def->os.bootloader) { /* * We prefer using explicit bootindex=N parameters for predictable @@ -8368,6 +8394,42 @@ 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 (STREQ(val, "0")) + def->pm.s3 = VIR_DOMAIN_PM_STATE_ENABLED; + else if (STREQ(val, "1")) + def->pm.s3 = VIR_DOMAIN_PM_STATE_DISABLED; + else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("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 (STREQ(val, "0")) + def->pm.s4 = VIR_DOMAIN_PM_STATE_ENABLED; + else if (STREQ(val, "1")) + def->pm.s4 = VIR_DOMAIN_PM_STATE_DISABLED; + else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("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 53d6e5b..1f44c69 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13722,6 +13722,23 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, goto cleanup; } + if (vm->def->pm.s3 || vm->def->pm.s4) { + if (!vm->def->pm.s3 == VIR_DOMAIN_PM_STATE_DISABLED && + (target == VIR_NODE_SUSPEND_TARGET_MEM || + target == VIR_NODE_SUSPEND_TARGET_HYBRID)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("S3 state is disabled for this domain")); + goto cleanup; + } + + if (vm->def->pm.s4 == VIR_DOMAIN_PM_STATE_DISABLED && + target == VIR_NODE_SUSPEND_TARGET_DISK) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("S4 state is disabled for this domain")); + goto cleanup; + } + } + if (priv->agentError) { virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", _("QEMU guest agent is not " -- 1.7.12
On 08/31/2012 07:59 AM, 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 | 62 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 17 ++++++++++++ 4 files changed, 88 insertions(+)
+++ b/src/qemu/qemu_command.c @@ -4782,6 +4782,32 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-no-acpi"); }
+ if (def->pm.s3) { + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S3)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S3 not supported")); + goto error; + } + virCommandAddArgList(cmd, + "-global", + (def->pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED ? + "PIIX4_PM.disable_s3=0" : "PIIX4_PM.disable_s3=1"), + NULL);
Fine as is, but I probably would have written: virCommandAddArg(cmd, "-global"); virCommandAddArgFormat(cmd, "PIIX4_PM.disable_s3=%d", def->pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED); for less typing.
+++ b/src/qemu/qemu_driver.c @@ -13722,6 +13722,23 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, goto cleanup; }
+ if (vm->def->pm.s3 || vm->def->pm.s4) { + if (!vm->def->pm.s3 == VIR_DOMAIN_PM_STATE_DISABLED &&
Logic bug. (!vm->def->pm.s3) means that you have flattened an enum into 0 or 1, before comparing it back to an enum value. I think you meant to drop the ! entirely. ACK with that fix. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/31/2012 07:49 PM, Eric Blake wrote:
On 08/31/2012 07:59 AM, 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 | 62 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 17 ++++++++++++ 4 files changed, 88 insertions(+)
+++ b/src/qemu/qemu_command.c @@ -4782,6 +4782,32 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-no-acpi"); }
+ if (def->pm.s3) { + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S3)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S3 not supported")); + goto error; + } + virCommandAddArgList(cmd, + "-global", + (def->pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED ? + "PIIX4_PM.disable_s3=0" : "PIIX4_PM.disable_s3=1"), + NULL);
Fine as is, but I probably would have written:
virCommandAddArg(cmd, "-global"); virCommandAddArgFormat(cmd, "PIIX4_PM.disable_s3=%d", def->pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED);
for less typing.
+++ b/src/qemu/qemu_driver.c @@ -13722,6 +13722,23 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, goto cleanup; }
+ if (vm->def->pm.s3 || vm->def->pm.s4) { + if (!vm->def->pm.s3 == VIR_DOMAIN_PM_STATE_DISABLED &&
Logic bug. (!vm->def->pm.s3) means that you have flattened an enum into 0 or 1, before comparing it back to an enum value. I think you meant to drop the ! entirely.
ACK with that fix.
Yes, I've missed it after one fixing and haven't checked if it works after that. Fixed, checked and pushed now. Thanks for the review. Martin
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 | 29 +++++++++++++++++++++ .../qemuxml2argv-misc-disable-suspends.args | 4 +++ .../qemuxml2argv-misc-disable-suspends.xml | 30 ++++++++++++++++++++++ .../qemuxml2argv-misc-enable-s4.args | 4 +++ .../qemuxml2argv-misc-enable-s4.xml | 29 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ tests/qemuxml2xmltest.c | 3 +++ 9 files changed, 110 insertions(+) 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..b89327d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.xml @@ -0,0 +1,29 @@ +<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 enabled='no'/> + </pm> + <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..fe0cf99 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml @@ -0,0 +1,30 @@ +<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 enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <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..fd65832 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.xml @@ -0,0 +1,29 @@ +<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 enabled='yes'/> + </pm> + <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 71513fb..8f91c48 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 da298b7..87d9e77 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.12
On 08/31/2012 07:59 AM, 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 | 29 +++++++++++++++++++++ .../qemuxml2argv-misc-disable-suspends.args | 4 +++ .../qemuxml2argv-misc-disable-suspends.xml | 30 ++++++++++++++++++++++ .../qemuxml2argv-misc-enable-s4.args | 4 +++ .../qemuxml2argv-misc-enable-s4.xml | 29 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ tests/qemuxml2xmltest.c | 3 +++ 9 files changed, 110 insertions(+)
Always good to see stuff like this. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/31/2012 08:01 PM, Eric Blake wrote:
On 08/31/2012 07:59 AM, 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 | 29 +++++++++++++++++++++ .../qemuxml2argv-misc-disable-suspends.args | 4 +++ .../qemuxml2argv-misc-disable-suspends.xml | 30 ++++++++++++++++++++++ .../qemuxml2argv-misc-enable-s4.args | 4 +++ .../qemuxml2argv-misc-enable-s4.xml | 29 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ tests/qemuxml2xmltest.c | 3 +++ 9 files changed, 110 insertions(+)
Always good to see stuff like this.
ACK.
Thanks, pushed. Martin
participants (2)
-
Eric Blake -
Martin Kletzander