conf: move virt type / os type / arch validation to post-parse

The XML parser currently calls virCapabilitiesDomainDataLookup during
parsing to find the domain capabilities matching the triple

  (virt type, os type, arch)

This is, however, bogus with the QEMU driver as it assumes that there
is an emulator known to the default driver capabilities that matches
this triple. It is entirely possible for the driver to be parsing an
XML file with a custom emulator path specified pointing to a binary
that doesn't exist in the default driver capabilities.  This will,
for example be the case on a RHEL host which only installs the host
native emulator to /usr/bin. The user can have built a custom QEMU
for non-native arches into $HOME and wish to use that.

Aside from validation, this call is also used to fill in a machine type
for the guest if not otherwise specified. Again, this data may be
incorrect for the QEMU driver because it is not taking account of
the emulator binary that is referenced.

To start fixing this, move the validation to the post-parse callbacks
where more intelligent driver specific logic can be applied.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
index 7d24bb602f2b177f7e484854a4e9869a83d4fd2d..575f141b53728b409cb42b4846bece4cb727e09b 100644 (file)
--- a/src/bhyve/bhyve_domain.c
+++ b/src/bhyve/bhyve_domain.c
@@ -74,11 +74,16 @@ bhyveDomainDefNeedsISAController(virDomainDefPtr def)
 
 static int
 bhyveDomainDefPostParse(virDomainDefPtr def,
-                        virCapsPtr caps G_GNUC_UNUSED,
+                        virCapsPtr caps,
                         unsigned int parseFlags G_GNUC_UNUSED,
                         void *opaque G_GNUC_UNUSED,
                         void *parseOpaque G_GNUC_UNUSED)
 {
+    if (!virCapabilitiesDomainSupported(caps, def->os.type,
+                                        def->os.arch,
+                                        def->virtType))
+        return -1;
+
     /* Add an implicit PCI root controller */
     if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
                                        VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index ff7d2656216c60e90347c9c2d7f4a47e651e79a5..748dd642733d93c67af3afc99d3a483b9cb2298e 100644 (file)
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -816,6 +816,23 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
 }
 
 
+bool
+virCapabilitiesDomainSupported(virCapsPtr caps,
+                               int ostype,
+                               virArch arch,
+                               int virttype)
+{
+    g_autofree virCapsDomainDataPtr capsdata = NULL;
+
+    capsdata = virCapabilitiesDomainDataLookup(caps, ostype,
+                                               arch,
+                                               virttype,
+                                               NULL, NULL);
+
+    return capsdata != NULL;
+}
+
+
 int
 virCapabilitiesAddStoragePool(virCapsPtr caps,
                               int poolType)

diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 8a7137d7eb3494a100cfe28eb4707a8c749b1189..c39fe0de08457fee197b0c2f4c81908413134756 100644 (file)
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -309,6 +309,13 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
                                 const char *emulator,
                                 const char *machinetype);
 
+bool
+virCapabilitiesDomainSupported(virCapsPtr caps,
+                               int ostype,
+                               virArch arch,
+                               int domaintype);
+
+
 void
 virCapabilitiesClearHostNUMACellCPUTopology(virCapsHostNUMACellCPUPtr cpu,
                                             size_t ncpus);

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3d36bb9ac90c6dddc6f3e6c4bddf287f7ac980ee..9be3ef7651636c58953dfd6193e7a1a9e2407f7e 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19613,14 +19613,11 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
 static int
 virDomainDefParseCaps(virDomainDefPtr def,
                       xmlXPathContextPtr ctxt,
-                      virDomainXMLOptionPtr xmlopt,
-                      virCapsPtr caps,
-                      unsigned int flags)
+                      virDomainXMLOptionPtr xmlopt)
 {
     g_autofree char *virttype = NULL;
     g_autofree char *arch = NULL;
     g_autofree char *ostype = NULL;
-    g_autofree virCapsDomainDataPtr capsdata = NULL;
 
     virttype = virXPathString("string(./@type)", ctxt);
     ostype = virXPathString("string(./os/type[1])", ctxt);
@@ -19681,18 +19678,6 @@ virDomainDefParseCaps(virDomainDefPtr def,
             def->os.arch = virArchFromHost();
     }
 
-    if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type,
-                                                     def->os.arch,
-                                                     def->virtType,
-                                                     NULL, NULL))) {
-        if (!(flags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))
-            return -1;
-        virResetLastError();
-    } else {
-        if (!def->os.machine)
-            def->os.machine = g_strdup(capsdata->machinetype);
-    }
-
     return 0;
 }
 
@@ -19846,7 +19831,7 @@ virDomainDefParseXML(xmlDocPtr xml,
             id = -1;
     def->id = (int)id;
 
-    if (virDomainDefParseCaps(def, ctxt, xmlopt, caps, flags) < 0)
+    if (virDomainDefParseCaps(def, ctxt, xmlopt) < 0)
         goto error;
 
     /* Extract domain name */

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index be55438165f0e45f045760b7010bfdc2c6871e90..a347f37fe226a8979c8975f4a1d3151b091ca6a6 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -54,6 +54,7 @@ virCapabilitiesAddStoragePool;
 virCapabilitiesAllocMachines;
 virCapabilitiesClearHostNUMACellCPUTopology;
 virCapabilitiesDomainDataLookup;
+virCapabilitiesDomainSupported;
 virCapabilitiesFormatXML;
 virCapabilitiesFreeGuest;
 virCapabilitiesFreeMachines;

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 19905442c18af5499cb33f71dc2e8953e47d0c81..ad9424155a5f762c577d896ae62f928be7a75036 100644 (file)
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -367,11 +367,16 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 
 static int
 libxlDomainDefPostParse(virDomainDefPtr def,
-                        virCapsPtr caps G_GNUC_UNUSED,
+                        virCapsPtr caps,
                         unsigned int parseFlags G_GNUC_UNUSED,
                         void *opaque G_GNUC_UNUSED,
                         void *parseOpaque G_GNUC_UNUSED)
 {
+    if (!virCapabilitiesDomainSupported(caps, def->os.type,
+                                        def->os.arch,
+                                        def->virtType))
+        return -1;
+
     /* Xen PV domains always have a PV console, so add one to the domain config
      * via post-parse callback if not explicitly specified in the XML. */
     if (def->os.type != VIR_DOMAIN_OSTYPE_HVM && def->nconsoles == 0) {

diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
index 4339d305a9813921e5c85657469f3ca5808519d6..b505a91c1c8b75b260d841043c1fc23d704a3c89 100644 (file)
--- a/src/lxc/lxc_domain.c
+++ b/src/lxc/lxc_domain.c
@@ -356,6 +356,11 @@ virLXCDomainDefPostParse(virDomainDefPtr def,
                          void *opaque G_GNUC_UNUSED,
                          void *parseOpaque G_GNUC_UNUSED)
 {
+    if (!virCapabilitiesDomainSupported(caps, def->os.type,
+                                        def->os.arch,
+                                        def->virtType))
+        return -1;
+
     /* check for emulator and create a default one if needed */
     if (!def->emulator &&
         !(def->emulator = virDomainDefGetDefaultEmulator(def, caps)))

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 8a05aa05044dc5b2887ac9a461a9bf8ba5fe3255..de8be1ed7d1765d82715424881cf4be502dacdc3 100644 (file)
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -1082,11 +1082,16 @@ int openvzGetVEID(const char *name)
 
 static int
 openvzDomainDefPostParse(virDomainDefPtr def,
-                         virCapsPtr caps G_GNUC_UNUSED,
+                         virCapsPtr caps,
                          unsigned int parseFlags G_GNUC_UNUSED,
                          void *opaque G_GNUC_UNUSED,
                          void *parseOpaque G_GNUC_UNUSED)
 {
+    if (!virCapabilitiesDomainSupported(caps, def->os.type,
+                                        def->os.arch,
+                                        def->virtType))
+        return -1;
+
     /* fill the init path */
     if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init)
         def->os.init = g_strdup("/sbin/init");

diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 5e00ef6448701d4961f24d47fd1f112c7714ca39..218d6f5b5cdf486e7237da94c0a7c1c608d15c66 100644 (file)
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1061,12 +1061,17 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
 
 
 static int
-phypDomainDefPostParse(virDomainDefPtr def G_GNUC_UNUSED,
-                       virCapsPtr caps G_GNUC_UNUSED,
+phypDomainDefPostParse(virDomainDefPtr def,
+                       virCapsPtr caps,
                        unsigned int parseFlags G_GNUC_UNUSED,
                        void *opaque G_GNUC_UNUSED,
                        void *parseOpaque G_GNUC_UNUSED)
 {
+    if (!virCapabilitiesDomainSupported(caps, def->os.type,
+                                        def->os.arch,
+                                        def->virtType))
+        return -1;
+
     return 0;
 }
 

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c6f34dbc4eaa03608d3260aa83854e19aa7a2bab..a342af334836c60a3603c1a3023b61059b424222 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4691,7 +4691,7 @@ qemuDomainDefPostParseBasic(virDomainDefPtr def,
 
 static int
 qemuDomainDefPostParse(virDomainDefPtr def,
-                       virCapsPtr caps G_GNUC_UNUSED,
+                       virCapsPtr caps,
                        unsigned int parseFlags,
                        void *opaque,
                        void *parseOpaque)
@@ -4703,6 +4703,11 @@ qemuDomainDefPostParse(virDomainDefPtr def,
      * with the capabilities populated. */
     virQEMUCapsPtr qemuCaps = parseOpaque;
 
+    if (!virCapabilitiesDomainSupported(caps, def->os.type,
+                                        def->os.arch,
+                                        def->virtType))
+        return -1;
+
     if (def->os.bootloader || def->os.bootloaderArgs) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("bootloader is not supported by QEMU"));
@@ -4710,9 +4715,15 @@ qemuDomainDefPostParse(virDomainDefPtr def,
     }
 
     if (!def->os.machine) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("missing machine type"));
-        return -1;
+        g_autofree virCapsDomainDataPtr capsdata = NULL;
+
+        if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type,
+                                                         def->os.arch,
+                                                         def->virtType,
+                                                         NULL, NULL))) {
+            return -1;
+        }
+        def->os.machine = g_strdup(capsdata->machinetype);
     }
 
     qemuDomainNVRAMPathGenerate(cfg, def);

diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index bab4fdb82bdffdeca435fccbd119f513acf9264d..be0adb1e45bbde3eabaa3598968c82a3a99665b7 100644 (file)
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -116,12 +116,17 @@ vmwareDataFreeFunc(void *data)
 }
 
 static int
-vmwareDomainDefPostParse(virDomainDefPtr def G_GNUC_UNUSED,
-                         virCapsPtr caps G_GNUC_UNUSED,
+vmwareDomainDefPostParse(virDomainDefPtr def,
+                         virCapsPtr caps,
                          unsigned int parseFlags G_GNUC_UNUSED,
                          void *opaque G_GNUC_UNUSED,
                          void *parseOpaque G_GNUC_UNUSED)
 {
+    if (!virCapabilitiesDomainSupported(caps, def->os.type,
+                                        def->os.arch,
+                                        def->virtType))
+        return -1;
+
     return 0;
 }
 

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index c4af7b1ce97763cec066e47eed3260a4968f093e..c2a06daecb4b9bf81ca90c6b17a53bb56955b072 100644 (file)
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -529,12 +529,17 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI,
  */
 
 static int
-virVMXDomainDefPostParse(virDomainDefPtr def G_GNUC_UNUSED,
-                         virCapsPtr caps G_GNUC_UNUSED,
+virVMXDomainDefPostParse(virDomainDefPtr def,
+                         virCapsPtr caps,
                          unsigned int parseFlags G_GNUC_UNUSED,
                          void *opaque G_GNUC_UNUSED,
                          void *parseOpaque G_GNUC_UNUSED)
 {
+    if (!virCapabilitiesDomainSupported(caps, def->os.type,
+                                        def->os.arch,
+                                        def->virtType))
+        return -1;
+
     return 0;
 }
 

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 1166b77b2ce4cc41733ef1e9f83379a223d85c54..66b15737a2c78b7f67993c282afc695140d23a50 100644 (file)
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -241,11 +241,16 @@ vzDomainDefAddDefaultInputDevices(virDomainDefPtr def)
 
 static int
 vzDomainDefPostParse(virDomainDefPtr def,
-                     virCapsPtr caps G_GNUC_UNUSED,
+                     virCapsPtr caps,
                      unsigned int parseFlags G_GNUC_UNUSED,
                      void *opaque G_GNUC_UNUSED,
                      void *parseOpaque G_GNUC_UNUSED)
 {
+    if (!virCapabilitiesDomainSupported(caps, def->os.type,
+                                        def->os.arch,
+                                        def->virtType))
+        return -1;
+
     if (vzDomainDefAddDefaultInputDevices(def) < 0)
         return -1;