9:59 a.m.
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
9:59 a.m.
New subject: [libvirt] [PATCH v4 1/3] Add per-guest S3/S4 state configuration
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
12:42 p.m.
New subject: [libvirt] [PATCH v4 1/3] Add per-guest S3/S4 state configuration
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:23 a.m.
New subject: [libvirt] [PATCH v4 1/3] Add per-guest S3/S4 state configuration
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
9:59 a.m.
New subject: [libvirt] [PATCH v4 2/3] qemu: Add support for S3/S4 state configuration
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
12:49 p.m.
New subject: [libvirt] [PATCH v4 2/3] qemu: Add support for S3/S4 state configuration
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:34 a.m.
New subject: [libvirt] [PATCH v4 2/3] qemu: Add support for S3/S4 state configuration
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
9:59 a.m.
New subject: [libvirt] [PATCH v4 3/3] tests: Add tests for qemu S3/S4 state configuration
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
1:01 p.m.
New subject: [libvirt] [PATCH v4 3/3] tests: Add tests for qemu S3/S4 state configuration
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:35 a.m.
New subject: [libvirt] [PATCH v4 3/3] tests: Add tests for qemu S3/S4 state configuration
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
4290
days inactive
4293
days old
9 comments
2 participants
participants (2)
-
Eric Blake -
Martin Kletzander