[libvirt] [PATCH] Add support for disabling S3 and S4 states
There a two new elements in <features> implemented, which control what ACPI sleeping states will be advertised. The default is to have both states enabled, so this means no change for current machines. --- docs/schemas/domaincommon.rng | 10 ++++++ src/conf/domain_conf.c | 4 ++- src/conf/domain_conf.h | 2 + src/qemu/qemu_command.c | 17 +++++++++++ src/qemu/qemu_driver.c | 15 ++++++++++ tests/qemuargv2xmltest.c | 3 ++ .../qemuxml2argv-misc-disable-s3.args | 4 ++ .../qemuxml2argv-misc-disable-s3.xml | 29 +++++++++++++++++++ .../qemuxml2argv-misc-disable-s4.args | 4 ++ .../qemuxml2argv-misc-disable-s4.xml | 29 +++++++++++++++++++ .../qemuxml2argv-misc-disable-suspends.args | 4 ++ .../qemuxml2argv-misc-disable-suspends.xml | 30 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ tests/qemuxml2xmltest.c | 3 ++ 14 files changed, 156 insertions(+), 1 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-s4.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s4.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b7562ad..859cb26 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2788,6 +2788,16 @@ <empty/> </element> </optional> + <optional> + <element name="disable_s3"> + <empty/> + </element> + </optional> + <optional> + <element name="disable_s4"> + <empty/> + </element> + </optional> </interleave> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 143d92e..1bfb8d0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -107,7 +107,9 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "pae", "hap", "viridian", - "privnet") + "privnet", + "disable_s3", + "disable_s4") VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST, "destroy", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bc02caf..b7ee95e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1346,6 +1346,8 @@ enum virDomainFeature { VIR_DOMAIN_FEATURE_HAP, VIR_DOMAIN_FEATURE_VIRIDIAN, VIR_DOMAIN_FEATURE_PRIVNET, + VIR_DOMAIN_FEATURE_DISABLE_S3, + VIR_DOMAIN_FEATURE_DISABLE_S4, VIR_DOMAIN_FEATURE_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4033644..ee9e169 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4691,6 +4691,12 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-no-acpi"); } + if (def->features & (1 << VIR_DOMAIN_FEATURE_DISABLE_S3)) + virCommandAddArgList(cmd, "-global", "PIIX4_PM.disable_s3=1", NULL); + + if (def->features & (1 << VIR_DOMAIN_FEATURE_DISABLE_S4)) + virCommandAddArgList(cmd, "-global", "PIIX4_PM.disable_s4=1", NULL); + if (!def->os.bootloader) { /* * We prefer using explicit bootindex=N parameters for predictable @@ -8191,6 +8197,17 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, *monConfig = chr; } + } else if (STREQ(arg, "-global") && + STREQ(progargv[i + 1], "PIIX4_PM.disable_s3=1")) { + /* We want to parse only the known "-global" parameters, + * so the ones that we don't know are still added to the + * namespace */ + i++; + def->features |= (1 << VIR_DOMAIN_FEATURE_DISABLE_S3); + } else if (STREQ(arg, "-global") && + STREQ(progargv[i + 1], "PIIX4_PM.disable_s4=1")) { + i++; + def->features |= (1 << VIR_DOMAIN_FEATURE_DISABLE_S4); } 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 0da5c5a..76dd374 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13060,6 +13060,21 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, goto cleanup; } + if (vm->def->features & (1 << VIR_DOMAIN_FEATURE_DISABLE_S3) && + (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->features & (1 << VIR_DOMAIN_FEATURE_DISABLE_S4) && + 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")); diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 439218e..7fae39a 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-s4"); + DO_TEST("misc-disable-suspends"); 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..4b2d13e --- /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> + <features> + <disable_s3/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <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-s4.args b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s4.args new file mode 100644 index 0000000..9177904 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-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=1 -boot c -hda /dev/HostVG/QEMUGuest1 \ +-net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s4.xml b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s4.xml new file mode 100644 index 0000000..1a27e9f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-misc-disable-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> + <features> + <disable_s4/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <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..587e80a --- /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> + <features> + <disable_s3/> + <disable_s4/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <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 c4aa7c4..c7a3d50 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -561,6 +561,9 @@ 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", NONE); + DO_TEST("misc-disable-s4", NONE); + DO_TEST("misc-disable-suspends", 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..f7622b7 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-s4"); + DO_TEST("misc-disable-suspends"); DO_TEST("misc-no-reboot"); DO_TEST("net-user"); DO_TEST("net-virtio"); -- 1.7.8.6
On Mon, Jul 30, 2012 at 02:25:04PM +0200, Martin Kletzander wrote:
There a two new elements in <features> implemented, which control what ACPI sleeping states will be advertised. The default is to have both states enabled, so this means no change for current machines.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b7562ad..859cb26 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2788,6 +2788,16 @@ <empty/> </element> </optional> + <optional> + <element name="disable_s3"> + <empty/> + </element> + </optional> + <optional> + <element name="disable_s4"> + <empty/> + </element> + </optional> </interleave> </element> </optional>
This is not very nice design. With this if an app wants to request S3, and have an error if it is not supported, they can't do that. I think we need a tri-state for S3/S4 - Default (hypervisor-specific) - On - Off Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 07/30/2012 02:33 PM, Daniel P. Berrange wrote:
On Mon, Jul 30, 2012 at 02:25:04PM +0200, Martin Kletzander wrote:
There a two new elements in <features> implemented, which control what ACPI sleeping states will be advertised. The default is to have both states enabled, so this means no change for current machines.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b7562ad..859cb26 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2788,6 +2788,16 @@ <empty/> </element> </optional> + <optional> + <element name="disable_s3"> + <empty/> + </element> + </optional> + <optional> + <element name="disable_s4"> + <empty/> + </element> + </optional> </interleave> </element> </optional>
This is not very nice design. With this if an app wants to request S3, and have an error if it is not supported, they can't do that.
I'm sorry, but I don't quite understand what do you mean. If the guest wants to go to S3, there is (of course) no difference between the state not being supported or the support being disabled. If an application wants to request the guest to go to S3 through libvirt, then it will get an appropriate message (disabled/not supported). Having one more option means just setting the QEMU parameter to the default value (0). If you really want it this way, then no problem, I'll create it as a another element (not in the 'features' part) and send v2. Martin
On Mon, Jul 30, 2012 at 02:43:33PM +0200, Martin Kletzander wrote:
On 07/30/2012 02:33 PM, Daniel P. Berrange wrote:
On Mon, Jul 30, 2012 at 02:25:04PM +0200, Martin Kletzander wrote:
There a two new elements in <features> implemented, which control what ACPI sleeping states will be advertised. The default is to have both states enabled, so this means no change for current machines.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b7562ad..859cb26 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2788,6 +2788,16 @@ <empty/> </element> </optional> + <optional> + <element name="disable_s3"> + <empty/> + </element> + </optional> + <optional> + <element name="disable_s4"> + <empty/> + </element> + </optional> </interleave> </element> </optional>
This is not very nice design. With this if an app wants to request S3, and have an error if it is not supported, they can't do that.
I'm sorry, but I don't quite understand what do you mean. If the guest wants to go to S3, there is (of course) no difference between the state not being supported or the support being disabled. If an application wants to request the guest to go to S3 through libvirt, then it will get an appropriate message (disabled/not supported). Having one more option means just setting the QEMU parameter to the default value (0).
I'm talking about the host mgmt's POV. An application creating a guest may want to guarantee that S3 is available to the guest. With your proposal, there is no way they can do that - since there is no explicit element in the XML to request S3. So the mgmt app may get a guest with S3, or it may not - it has no way of being sure. The only thing a mgmt app can do is forcably disable S3 . Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 07/30/2012 04:48 PM, Daniel P. Berrange wrote:
On Mon, Jul 30, 2012 at 02:43:33PM +0200, Martin Kletzander wrote:
On 07/30/2012 02:33 PM, Daniel P. Berrange wrote:
On Mon, Jul 30, 2012 at 02:25:04PM +0200, Martin Kletzander wrote:
There a two new elements in <features> implemented, which control what ACPI sleeping states will be advertised. The default is to have both states enabled, so this means no change for current machines.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b7562ad..859cb26 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2788,6 +2788,16 @@ <empty/> </element> </optional> + <optional> + <element name="disable_s3"> + <empty/> + </element> + </optional> + <optional> + <element name="disable_s4"> + <empty/> + </element> + </optional> </interleave> </element> </optional>
This is not very nice design. With this if an app wants to request S3, and have an error if it is not supported, they can't do that.
I'm sorry, but I don't quite understand what do you mean. If the guest wants to go to S3, there is (of course) no difference between the state not being supported or the support being disabled. If an application wants to request the guest to go to S3 through libvirt, then it will get an appropriate message (disabled/not supported). Having one more option means just setting the QEMU parameter to the default value (0).
I'm talking about the host mgmt's POV. An application creating a guest may want to guarantee that S3 is available to the guest. With your proposal, there is no way they can do that - since there is no explicit element in the XML to request S3. So the mgmt app may get a guest with S3, or it may not - it has no way of being sure. The only thing a mgmt app can do is forcably disable S3 .
Oh, yes, I get it now. Unfortunately, that is true. Sadly, the only thing I see we can do with QEMU is set the disable_s[34] parameter(s) to zero, but that won't guarantee anything, I guess. Global parameters are valid even if they don't make sense or they don't exist, so even "qemu-kvm -global a.s=df" is a valid command-line. Martin
On Mon, Jul 30, 2012 at 04:54:34PM +0200, Martin Kletzander wrote:
On 07/30/2012 04:48 PM, Daniel P. Berrange wrote:
On Mon, Jul 30, 2012 at 02:43:33PM +0200, Martin Kletzander wrote:
On 07/30/2012 02:33 PM, Daniel P. Berrange wrote:
On Mon, Jul 30, 2012 at 02:25:04PM +0200, Martin Kletzander wrote:
There a two new elements in <features> implemented, which control what ACPI sleeping states will be advertised. The default is to have both states enabled, so this means no change for current machines.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b7562ad..859cb26 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2788,6 +2788,16 @@ <empty/> </element> </optional> + <optional> + <element name="disable_s3"> + <empty/> + </element> + </optional> + <optional> + <element name="disable_s4"> + <empty/> + </element> + </optional> </interleave> </element> </optional>
This is not very nice design. With this if an app wants to request S3, and have an error if it is not supported, they can't do that.
I'm sorry, but I don't quite understand what do you mean. If the guest wants to go to S3, there is (of course) no difference between the state not being supported or the support being disabled. If an application wants to request the guest to go to S3 through libvirt, then it will get an appropriate message (disabled/not supported). Having one more option means just setting the QEMU parameter to the default value (0).
I'm talking about the host mgmt's POV. An application creating a guest may want to guarantee that S3 is available to the guest. With your proposal, there is no way they can do that - since there is no explicit element in the XML to request S3. So the mgmt app may get a guest with S3, or it may not - it has no way of being sure. The only thing a mgmt app can do is forcably disable S3 .
Oh, yes, I get it now. Unfortunately, that is true. Sadly, the only thing I see we can do with QEMU is set the disable_s[34] parameter(s) to zero, but that won't guarantee anything, I guess. Global parameters are valid even if they don't make sense or they don't exist, so even "qemu-kvm -global a.s=df" is a valid command-line.
Well regardless, that is a QEMU bug - please report it to the QEMU developer mailing list. libvirt needs to find a way to detct whether S3 is available and report CONFIG_NOT_SUPPORTED, even if we just do this based on version number. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Martin Kletzander