Re: [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.