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

[Michal Privoznik]

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.

Michal