[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