7:05 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.
--
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/schemas/domaincommon.rng | 33 ++++++
src/conf/domain_conf.c | 44 +++++++++
src/conf/domain_conf.h | 14 +++
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 +
17 files changed, 325 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
7:05 a.m.
New subject: [libvirt] [PATCH v2 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.
---
docs/schemas/domaincommon.rng | 33 ++++++++++++++++++++++++++++++
src/conf/domain_conf.c | 44 +++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 14 +++++++++++++
src/libvirt_private.syms | 2 +
4 files changed, 93 insertions(+), 0 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c85d763..657d723 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="s3">
+ <ref name="suspendChoices"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="s4">
+ <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 43b3f80..7a8279b 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",
@@ -8381,6 +8386,19 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
virDomainLifecycleCrashTypeFromString) < 0)
goto error;
+ if (virXPathNode("./pm", ctxt)) {
+ tmp = virXPathString("string(./pm/@s3)", ctxt);
+ if (tmp) {
+ def->pm.s3 = virDomainPMStateTypeFromString(tmp);
+ VIR_FREE(tmp);
+ }
+ tmp = virXPathString("string(./pm/@s4)", 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) {
@@ -13061,6 +13079,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, " s3='%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, " s4='%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 9fdda78..3eddd5f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1372,6 +1372,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,
@@ -1618,6 +1626,11 @@ struct _virDomainDef {
int onPoweroff;
int onCrash;
+ struct {
+ int s3;
+ int s4;
+ } pm;
+
virDomainOSDef os;
char *emulator;
int features;
@@ -2165,6 +2178,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 bb8849b..98bcb98 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -431,6 +431,8 @@ virDomainPausedReasonTypeFromString;
virDomainPausedReasonTypeToString;
virDomainPciRombarModeTypeFromString;
virDomainPciRombarModeTypeToString;
+virDomainPMStateTypeFromString;
+virDomainPMStateTypeToString;
virDomainRedirdevBusTypeFromString;
virDomainRedirdevBusTypeToString;
virDomainRemoveInactive;
--
1.7.8.6
3:36 p.m.
New subject: [libvirt] [PATCH v2 1/3] Add per-guest S3/S4 state configuration
On 08/02/2012 06:05 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.
You are proposing /domain/pm; but we also have /domain/os/bios, would
this be better as a subelement /domain/os/bios/pm, since it is related
to bios options?
---
docs/schemas/domaincommon.rng | 33 ++++++++++++++++++++++++++++++
src/conf/domain_conf.c | 44 +++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 14 +++++++++++++
src/libvirt_private.syms | 2 +
4 files changed, 93 insertions(+), 0 deletions(-)
@@ -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="s3">
Is the name 's3' too x86-centric? Remember, for
virNodeSuspendForDuration, we went with the names 'mem' (s3) and 'disk'
(s4) to give a more descriptive naming for where we were suspending. So
controlling whether s3 is advertised is really controlling whether
'suspend-to-memory' is advertised.
+++ b/src/conf/domain_conf.h
@@ -1372,6 +1372,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,
@@ -1618,6 +1626,11 @@ struct _virDomainDef {
int onPoweroff;
int onCrash;
+ struct {
+ int s3;
+ int s4;
Add a comment that these values are type enum virDomainPMState.
The code itself looks decent (and thanks for the test cases), but I'd
like a second opinion on whether the choice of XML naming is the best;
if we choose a different naming scheme, I think you should be able to
adapt the rest of the patch pretty easily.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:16 a.m.
New subject: [libvirt] [PATCH v2 1/3] Add per-guest S3/S4 state configuration
On Thu, Aug 2, 2012 at 3:36 PM, Eric Blake <eblake(a)redhat.com> wrote:
On 08/02/2012 06:05 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.
You are proposing /domain/pm; but we also have /domain/os/bios, would
this be better as a subelement /domain/os/bios/pm, since it is related
to bios options?
I would say that /domain/os/bios/pm isn't the correct place because
/domain/os relates only to OS booting while Power Management is
outside of the scope of booting. In fact /domain/os/bios simply
controls whether the BIOS output will be over VGA or over serial.
Features specific to the system are typically kept as top level items,
which matches /domain/pm. I would however say that using the terms
ACPI, s3 and s4 isn't good design for the XML. You can leave that to
documentation but /domain/pm/suspend-to-mem and
/domain/pm/suspend-to-disk are better so that the same XML structure
can be used for PPC. They have concepts similar to s3 and s4 (I only
briefly Googled this) but they're named completely differently. The
only under place I could see this would be /domain/features/acpi. If
its there then s3 and s4 potentially make sense, but then we're left
without a place for PPC to tie in.
--
Doug Goldstein
2:45 a.m.
New subject: [libvirt] [PATCH v2 1/3] Add per-guest S3/S4 state configuration
On 08/03/2012 07:16 AM, Doug Goldstein wrote:
On Thu, Aug 2, 2012 at 3:36 PM, Eric Blake <eblake(a)redhat.com>
wrote:
> On 08/02/2012 06:05 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.
>
> You are proposing /domain/pm; but we also have /domain/os/bios, would
> this be better as a subelement /domain/os/bios/pm, since it is related
> to bios options?
I would say that /domain/os/bios/pm isn't the correct place because
/domain/os relates only to OS booting while Power Management is
outside of the scope of booting. In fact /domain/os/bios simply
controls whether the BIOS output will be over VGA or over serial.
Features specific to the system are typically kept as top level items,
which matches /domain/pm. I would however say that using the terms
ACPI, s3 and s4 isn't good design for the XML. You can leave that to
documentation but /domain/pm/suspend-to-mem and
/domain/pm/suspend-to-disk are better so that the same XML structure
can be used for PPC. They have concepts similar to s3 and s4 (I only
briefly Googled this) but they're named completely differently. The
only under place I could see this would be /domain/features/acpi. If
its there then s3 and s4 potentially make sense, but then we're left
without a place for PPC to tie in.
Thanks both of you for the tips. I also see I haven't added this to the
documentation, which I have to fix in next version of the patch. I have
no problem adapting the naming in XML to whatever will suit you. I was
thinking to put this into /domain/sysinfo, but it looked like it doesn't
fit there and I couldn't make it tristate in /domain/features, so that's
why I've chosen this.
2:46 a.m.
New subject: [libvirt] [PATCH v2 1/3] Add per-guest S3/S4 state configuration
Il 03/08/2012 07:16, Doug Goldstein ha scritto:
> > You are proposing /domain/pm; but we also have
/domain/os/bios, would
> > this be better as a subelement /domain/os/bios/pm, since it is related
> > to bios options?
I would say that /domain/os/bios/pm isn't the correct place because
/domain/os relates only to OS booting while Power Management is
outside of the scope of booting. In fact /domain/os/bios simply
controls whether the BIOS output will be over VGA or over serial.
Features specific to the system are typically kept as top level items,
which matches /domain/pm. I would however say that using the terms
ACPI, s3 and s4 isn't good design for the XML. You can leave that to
documentation but /domain/pm/suspend-to-mem and
/domain/pm/suspend-to-disk are better so that the same XML structure
can be used for PPC. They have concepts similar to s3 and s4 (I only
briefly Googled this) but they're named completely differently. The
only under place I could see this would be /domain/features/acpi. If
its there then s3 and s4 potentially make sense, but then we're left
without a place for PPC to tie in.
I would put it under /domain/features/pm.
Paolo
2:47 a.m.
New subject: [libvirt] [PATCH v2 1/3] Add per-guest S3/S4 state configuration
On 08/03/2012 09:46 AM, Paolo Bonzini wrote:
Il 03/08/2012 07:16, Doug Goldstein ha scritto:
>>> You are proposing /domain/pm; but we also have /domain/os/bios, would
>>> this be better as a subelement /domain/os/bios/pm, since it is related
>>> to bios options?
> I would say that /domain/os/bios/pm isn't the correct place because
> /domain/os relates only to OS booting while Power Management is
> outside of the scope of booting. In fact /domain/os/bios simply
> controls whether the BIOS output will be over VGA or over serial.
> Features specific to the system are typically kept as top level items,
> which matches /domain/pm. I would however say that using the terms
> ACPI, s3 and s4 isn't good design for the XML. You can leave that to
> documentation but /domain/pm/suspend-to-mem and
> /domain/pm/suspend-to-disk are better so that the same XML structure
> can be used for PPC. They have concepts similar to s3 and s4 (I only
> briefly Googled this) but they're named completely differently. The
> only under place I could see this would be /domain/features/acpi. If
> its there then s3 and s4 potentially make sense, but then we're left
> without a place for PPC to tie in.
I would put it under /domain/features/pm.
Paolo
In v1 I had this in /domain/features, but due to the way the features
are parsed (they are just flags) I couldn't make it tristate as danpb
suggested (and he was right), so I had to choose a different place, but
otherwise it would fit there logically, yes.
Martin
7:05 a.m.
New subject: [libvirt] [PATCH v2 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 | 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 85c49a2..b4a662f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -169,6 +169,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"virtio-s390",
"balloon-event",
+ "disable-s3", /* 100 */
+ "disable-s4",
);
struct qemu_feature_flags {
@@ -1396,6 +1398,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);
@@ -1487,6 +1490,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 e8251dc..a909f67 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -135,6 +135,8 @@ enum qemuCapsFlags {
QEMU_CAPS_NEC_USB_XHCI = 97, /* -device nec-usb-xhci */
QEMU_CAPS_VIRTIO_S390 = 98, /* -device virtio-*-s390 */
QEMU_CAPS_BALLOON_EVENT = 99, /* Async event for balloon changes */
+ QEMU_CAPS_DISABLE_S3 = 100, /* S3 BIOS Advertisement on/off */
+ QEMU_CAPS_DISABLE_S4 = 101, /* 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 6ad65a6..a30c2f1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4692,6 +4692,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
@@ -8199,6 +8241,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 b3f946c..24712ce 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13066,6 +13066,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
3:48 p.m.
New subject: [libvirt] [PATCH v2 2/3] qemu: Add support for S3/S4 state configuration
On 08/02/2012 06:05 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 | 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 85c49a2..b4a662f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -169,6 +169,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"virtio-s390",
"balloon-event",
+ "disable-s3", /* 100 */
+ "disable-s4",
Do we need two capability bits here, or are we guaranteed that both
command line options are always present or absent together and one bit
sufficient? If we use just one bit, maybe naming it 'disable-pm' would
be reasonable. Then again, the qemu folks might say that it really
should be two independent bits, such as if it is easier to backport just
one of the two controls to an older qemu.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:55 a.m.
New subject: [libvirt] [PATCH v2 2/3] qemu: Add support for S3/S4 state configuration
On 08/02/2012 10:48 PM, Eric Blake wrote:
On 08/02/2012 06:05 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 | 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 85c49a2..b4a662f 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -169,6 +169,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
> "virtio-s390",
> "balloon-event",
>
> + "disable-s3", /* 100 */
> + "disable-s4",
Do we need two capability bits here, or are we guaranteed that both
command line options are always present or absent together and one bit
sufficient? If we use just one bit, maybe naming it 'disable-pm' would
be reasonable. Then again, the qemu folks might say that it really
should be two independent bits, such as if it is easier to backport just
one of the two controls to an older qemu.
It depends. We can make it as one bit and check only for one of those
(or both). I don't think this will be dropped any time nor that someone
has qemu old enough not to support this. The level to which these checks
should be done is very hard to guess for me and can vary a lot. For
example check that qemu will use PIIX4_PM device, because otherwise
these values mean nothing. OTOH qemu uses this one by default even
without specifying that, because there has to be some chipset and this
is the only one that exists there (if I understood that correctly). Hard
to say, but I'll gladly listen to more experienced colleague ;)
Martin
7:05 a.m.
New subject: [libvirt] [PATCH v2 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 | 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..7938245
--- /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 s3='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..0b28584
--- /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 s3='off' s4='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..61b8299
--- /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 s4='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 39fcd9f..a3f52a6 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -561,6 +561,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
4529
days inactive
4530
days old
10 comments
4 participants
participants (4)
-
Doug Goldstein -
Eric Blake -
Martin Kletzander -
Paolo Bonzini