[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]