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@xxxxxxxxx>
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
