[PATCH] qemu: Audit VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE flag usage

Michal Privoznik mprivozn at redhat.com
Mon Jan 31 09:42:28 UTC 2022


There is plenty of places where a domain XML is parsed using
VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE flag, but not all are
warranted. The flag usage is okay when parsing an XML produced by
us (e.g. when copying virDomainDef). In the rest of places
(especially when the XML might come from user) we need to
validate the XML, otherwise we may start QEMU assuming it has
certain capabilities while in fact it doesn't. For instance, in
this specific case when migrating a guest with virtio-mem to a
QEMU that has virtio-mem disabled, loading migration fails with:

  qemu-kvm: ... 'virtio-mem-pci' is not a valid device model name

This bug is more visible the more we transfer validation from
qemu_command.c into qemu_validate.c.

There is a possibility that we might prevent migration because of
a bug in our validator, but that's better than starting a QEMU
with features it doesn't support.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2048435
Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/qemu/qemu_driver.c    | 5 ++---
 src/qemu/qemu_migration.c | 9 +++------
 src/qemu/qemu_saveimage.c | 3 +--
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 370d223198..7d12b187ba 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2681,8 +2681,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
 
         if (!(def = virDomainDefParseString(xmlin, driver->xmlopt,
                                             priv->qemuCaps,
-                                            VIR_DOMAIN_DEF_PARSE_INACTIVE |
-                                            VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) {
+                                            VIR_DOMAIN_DEF_PARSE_INACTIVE))) {
             goto endjob;
         }
         if (!qemuDomainCheckABIStability(driver, vm, def))
@@ -7944,7 +7943,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver,
     g_autoptr(virQEMUDriverConfig) cfg = NULL;
     g_autoptr(virDomainDeviceDef) dev = NULL;
     virDomainDeviceDef *dev_copy = NULL;
-    unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
+    unsigned int parse_flags = 0;
     g_autoptr(virDomainDef) vmdef = NULL;
     int ret = -1;
 
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 2635ef1162..c83eb41693 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2469,8 +2469,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver,
 
     if (xmlin) {
         if (!(def = virDomainDefParseString(xmlin, driver->xmlopt, priv->qemuCaps,
-                                            VIR_DOMAIN_DEF_PARSE_INACTIVE |
-                                            VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
+                                            VIR_DOMAIN_DEF_PARSE_INACTIVE)))
             return NULL;
 
         if (!qemuDomainCheckABIStability(driver, vm, def))
@@ -2858,8 +2857,7 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver,
 
                 VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout);
                 newdef = virDomainDefParseString(xmlout, driver->xmlopt, NULL,
-                                                 VIR_DOMAIN_DEF_PARSE_INACTIVE |
-                                                 VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);
+                                                 VIR_DOMAIN_DEF_PARSE_INACTIVE);
                 if (!newdef)
                     goto cleanup;
 
@@ -3355,8 +3353,7 @@ qemuMigrationAnyPrepareDef(virQEMUDriver *driver,
 
     if (!(def = virDomainDefParseString(dom_xml, driver->xmlopt,
                                         qemuCaps,
-                                        VIR_DOMAIN_DEF_PARSE_INACTIVE |
-                                        VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
+                                        VIR_DOMAIN_DEF_PARSE_INACTIVE)))
         goto cleanup;
 
     if (dname) {
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 557ee2cd21..b106e5b299 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -556,8 +556,7 @@ qemuSaveImageOpen(virQEMUDriver *driver,
 
     /* Create a domain from this XML */
     if (!(def = virDomainDefParseString(data->xml, driver->xmlopt, qemuCaps,
-                                        VIR_DOMAIN_DEF_PARSE_INACTIVE |
-                                        VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
+                                        VIR_DOMAIN_DEF_PARSE_INACTIVE)))
         return -1;
 
     *ret_def = g_steal_pointer(&def);
-- 
2.34.1




More information about the libvir-list mailing list