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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list