[libvirt] [PATCH v3 3/4] qemu_capabilities: Introduce QEMU_CAPS_PM_WAKEUP_SUPPORT

Daniel Henrique Barboza danielhb413 at gmail.com
Thu Apr 11 10:12:26 UTC 2019



On 4/11/19 6:58 AM, Michal Privoznik wrote:
> On 4/11/19 10:39 AM, Peter Krempa wrote:
>> On Thu, Apr 11, 2019 at 10:25:11 +0200, Michal Privoznik wrote:
>>> This capability tells whether qemu is capable of waking up the
>>> guest from PM suspend.
>>>
>>> Based-on-work-of: Daniel Henrique Barboza <danielhb413 at gmail.com>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>   src/qemu/qemu_capabilities.c                  | 24 
>>> +++++++++++++++++++
>>>   src/qemu/qemu_capabilities.h                  |  3 +++
>>>   .../caps_4.0.0.riscv32.replies                | 11 +++++++++
>>>   .../caps_4.0.0.riscv32.xml                    |  1 +
>>>   .../caps_4.0.0.riscv64.replies                | 11 +++++++++
>>>   .../caps_4.0.0.riscv64.xml                    |  1 +
>>>   .../caps_4.0.0.x86_64.replies                 | 11 +++++++++
>>>   .../caps_4.0.0.x86_64.xml                     |  1 +
>>>   8 files changed, 63 insertions(+)
>>>
>>
>> [...]
>>
>>
>>>   bool
>>>   virQEMUCapsCPUFilterFeatures(const char *name,
>>>                                void *opaque)
>>> @@ -4373,6 +4395,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr 
>>> qemuCaps,
>>>           return -1;
>>>       if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0)
>>>           return -1;
>>> +    if (virQEMUCapsProbeQMPCurrentMachine(qemuCaps, mon) < 0)
>>> +        return -1;
>>
>> This seems wrong by definition. The function is supposed to query
>> current machine, but the capability lookup code uses 'none' machine
>> type. IIUC the support for wakeup in some cases depends on the presence
>> of ACPI in the guest and thus really can't be cached this way.
>
> Yep, I was just about to write this but then got sidetracked.
>
> So I see two options here:
> 1) Query if guest is capable of PM suspend at runtime. However, this 
> has to be done in a clever way. It would need to be done not only in 
> qemuProcessLaunch() (or some subordinate functions), it would need to 
> be done in qemuProcessReconnect() too (to cover case when user is 
> upgrading libvirt). For this, qemuMonitorConnect() looks like the best 
> place - it's called from both locations and it also already contains 
> some 'runtime capabilities' querying. Except not really - the 
> migration capabilities queried there are stored in 
> @priv->migrationCaps rather than priv->qemuCaps.
> Problem with storing runtime capabilities in priv->qemuCaps is that 
> they would get formatted into status XML. And this opens whole 
> different can of worms. For instance, this feature relies on 
> 'query-commands' to see if 'query-current-machine' command is 
> available. Well, we can't re-run 'query-commands' in runtime because 
> that might change the set of qemuCaps used when constructing cmd line.
> Way around this is to have 'runtime only' capabilities, that would not 
> be stored in status XML. At some point we will need those, but writing 
> that infrastructure is not easy and a bit too much to ask for this 
> small fix. Especially, when we have another option:
>
> 2) Issue 'query-current-machine' from 
> qemuDomainPMSuspendForDuration(). In every run. This is a bit 
> suboptimal, but how frequently do we expect this API to be called? 
> Caching qemu capabilities is there to speed up decission process which 
> makes sense if we'd be making the decission 1000 times a second (e.g. 
> building cmd line). With this approach the slowdown would be 
> negligible. Of course, we would need to run the command in 'silent' 
> mode, i.e. not report any error if qemu doesn't know it.

I think the root issue in this series is that I made a mistake by calling
wake-up support a capability. In Libvirt terms, it really doesn't feel like
it - it can't be polled inside qemu_caps because runtime parameters
such as '--no-acpi' can change its value, but at the same time it feels
weird to populate qemuCaps in qemuProcess time.

All that said, I think a good solution would be (2). dompmsuspend isn't a
performance sensitive command, thus the extra time to execute the API
can be ignored. Also, we can still check for QEMU_CAPS_QUERY_CURRENT_MACHINE
inside qemuDomainPMSuspendForDuration to avoid firing up an error in a
QEMU version that doesn't know the API.


Thanks,


DHB



>
> Michal




More information about the libvir-list mailing list