[libvirt] [PATCH v2 3/3] qemu: honour parseOpaque instead of refetching caps

Daniel P. Berrangé berrange at redhat.com
Wed Dec 11 15:58:23 UTC 2019


The use of the parseOpaque parameter was mistakenly removed in

  commit 4a4132b4625778cf80acb9c92d06351b44468ac3
  Author: Daniel P. Berrangé <berrange at redhat.com>
  Date:   Tue Dec 3 10:49:49 2019 +0000

    conf: don't use passed in caps in post parse method

causing the method to re-fetch qemuCaps that were already just
fetched and put into parseOpaque.

This is inefficient when parsing incoming XML, but for live
XML this is more serious as it means we use the capabilities
for the current QEMU binary on disk, rather than the running
QEMU.

That commit, however, did have a useful side effect of fixing
a crasher bug in the qemu post parse callback introduced by

  commit 5e939cea896fb3373a6f68f86e325c657429ed3d
  Author: Jiri Denemark <jdenemar at redhat.com>
  Date:   Thu Sep 26 18:42:02 2019 +0200

    qemu: Store default CPU in domain XML

The qemuDomainDefSetDefaultCPU() method in that patch did not
allow for the possibility that qemuCaps would be NULL and thus
resulted in a SEGV.

This shows a risk in letting each check in the post parse
callback look for qemuCaps == NULL. The safer option is to
check once upfront and immediately stop (postpone) further
validation.

Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
---
 src/qemu/qemu_domain.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3e1bd52d9f..cf2e9aebd2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4854,16 +4854,18 @@ static int
 qemuDomainDefPostParse(virDomainDefPtr def,
                        unsigned int parseFlags,
                        void *opaque,
-                       void *parseOpaque G_GNUC_UNUSED)
+                       void *parseOpaque)
 {
     virQEMUDriverPtr driver = opaque;
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-    g_autoptr(virQEMUCaps) qemuCaps = NULL;
+    virQEMUCapsPtr qemuCaps = parseOpaque;
 
-    if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
-                                            def->emulator))) {
+    /* Note that qemuCaps may be NULL when this function is called. This
+     * function shall not fail in that case. It will be re-run on VM startup
+     * with the capabilities populated.
+     */
+    if (!qemuCaps)
         return 1;
-    }
 
     if (def->os.bootloader || def->os.bootloaderArgs) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-- 
2.23.0




More information about the libvir-list mailing list