From: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; QEMU commit 46ea94ca9cf ("qmp: query-current-machine with wakeup-suspend-support") added a new QMP command called 'query-current-machine' that retrieves guest parameters that can vary in the same machine model (e.g. ACPI support for x86 VMs depends on the '--no-acpi' option). Currently, this API has a single flag, 'wakeup-suspend-support', that indicates whether the guest has the capability of waking up from suspended state. Introduce a libvirt capability that reflects whether qemu has the monitor command. Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 + 5 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a2de8630cd..5b3a2d0c33 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -524,6 +524,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "nvdimm.unarmed", "scsi-disk.device_id", "virtio-pci-non-transitional", + "query-current-machine", ); @@ -969,6 +970,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "query-cpus-fast", QEMU_CAPS_QUERY_CPUS_FAST }, { "qom-list-properties", QEMU_CAPS_QOM_LIST_PROPERTIES }, { "blockdev-del", QEMU_CAPS_BLOCKDEV_DEL }, + { "query-current-machine", QEMU_CAPS_QUERY_CURRENT_MACHINE }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9abeed9014..d3f90e82a8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -507,6 +507,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DEVICE_NVDIMM_UNARMED, /* -device nvdimm,unarmed= */ QEMU_CAPS_SCSI_DISK_DEVICE_ID, /* 'device_id' property of scsi disk */ QEMU_CAPS_VIRTIO_PCI_TRANSITIONAL, /* virtio *-pci-{non-}transitional devices */ + QEMU_CAPS_QUERY_CURRENT_MACHINE, /* query-current-machine command */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml index a8b5eb61c5..9d20be128a 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml @@ -163,6 +163,7 @@ <flag name='memory-backend-file.pmem'/> <flag name='scsi-disk.device_id'/> <flag name='virtio-pci-non-transitional'/> + <flag name='query-current-machine'/> <version>3001091</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml index 311740f34f..a1a59e14cb 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml @@ -163,6 +163,7 @@ <flag name='memory-backend-file.pmem'/> <flag name='scsi-disk.device_id'/> <flag name='virtio-pci-non-transitional'/> + <flag name='query-current-machine'/> <version>3001091</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml index 4585ff9e07..899c0cf7c8 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml @@ -201,6 +201,7 @@ <flag name='nvdimm.unarmed'/> <flag name='scsi-disk.device_id'/> <flag name='virtio-pci-non-transitional'/> + <flag name='query-current-machine'/> <version>3001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100758</microcodeVersion> -- 2.21.0

This capability tells whether qemu is capable of waking up the guest from PM suspend. Based-on-work-of: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- 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(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5b3a2d0c33..389986d924 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -525,6 +525,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "scsi-disk.device_id", "virtio-pci-non-transitional", "query-current-machine", + + /* 330 */ + "wakeup-suspend-support", ); @@ -2769,6 +2772,25 @@ virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps, } +static int +virQEMUCapsProbeQMPCurrentMachine(virQEMUCapsPtr qemuCaps, + qemuMonitorPtr mon) +{ + qemuMonitorCurrentMachineInfo info = { 0 }; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE)) + return 0; + + if (qemuMonitorGetCurrentMachineInfo(mon, &info) < 0) + return -1; + + if (info.wakeupSuspendSupport) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_PM_WAKEUP_SUPPORT); + + return 0; +} + + 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; virQEMUCapsInitProcessCaps(qemuCaps); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d3f90e82a8..721092b91b 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -509,6 +509,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_VIRTIO_PCI_TRANSITIONAL, /* virtio *-pci-{non-}transitional devices */ QEMU_CAPS_QUERY_CURRENT_MACHINE, /* query-current-machine command */ + /* 330 */ + QEMU_CAPS_PM_WAKEUP_SUPPORT, /* domain has wake-up from suspend support */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies index c7dac44289..a4d5786301 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies @@ -17756,3 +17756,14 @@ ], "id": "libvirt-39" } + +{ + "execute": "query-current-machine", + "id": "libvirt-40" +} + +{ + "return": { + "wakeup-suspend-support": true + } +} diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml index 9d20be128a..c82b0d5e74 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml @@ -164,6 +164,7 @@ <flag name='scsi-disk.device_id'/> <flag name='virtio-pci-non-transitional'/> <flag name='query-current-machine'/> + <flag name='wakeup-suspend-support'/> <version>3001091</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies index 6fda8ad2d2..f2d209cd91 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies @@ -17756,3 +17756,14 @@ ], "id": "libvirt-39" } + +{ + "execute": "query-current-machine", + "id": "libvirt-40" +} + +{ + "return": { + "wakeup-suspend-support": true + } +} diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml index a1a59e14cb..bf7f95e3b1 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml @@ -164,6 +164,7 @@ <flag name='scsi-disk.device_id'/> <flag name='virtio-pci-non-transitional'/> <flag name='query-current-machine'/> + <flag name='wakeup-suspend-support'/> <version>3001091</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.replies index aa9ee38c80..47ed75d359 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.replies @@ -21052,6 +21052,17 @@ } } +{ + "execute": "query-current-machine", + "id": "libvirt-50" +} + +{ + "return": { + "wakeup-suspend-support": true + } +} + { "execute": "qmp_capabilities", "id": "libvirt-1" diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml index 899c0cf7c8..df1b0da731 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml @@ -202,6 +202,7 @@ <flag name='scsi-disk.device_id'/> <flag name='virtio-pci-non-transitional'/> <flag name='query-current-machine'/> + <flag name='wakeup-suspend-support'/> <version>3001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100758</microcodeVersion> -- 2.21.0

On 4/11/19 10:39 AM, Peter Krempa wrote: 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

On Thu, Apr 11, 2019 at 11:58:04 +0200, Michal Privoznik wrote: If you are upgrading the capabilities parsed from the status XML won't have the bit representing the presence of the query command. This means that new libvirt would still attempt the suspend ignoring whether it is supported or not. I don't think it makes sense to re-detect presence of stuff which was done prior to upgrading. I was considering whether it's a good idea to keep runtime only capabilities in the qemuCaps array. I can find both arguments for it and against it. The convenient part is that it's automatically stored in the status XML. It is strongly desired to store it in the status XML. That way you don't have to re-query. Why bother? If qemu was started prior to the support being added I don't see a compelling reason to requery everything. In most cases it would be also wrong to do so. I don't think it's suboptimal. Normally users don't use PM suspend for the VM so you can argue that this saves one qemu call in all the cases where PM suspend is never used. Or you can still use the capability bit whether the query command is available.