[libvirt] [PATCH 06/30] 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. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/bhyve/bhyve_domain.c | 7 ++++++- src/conf/capabilities.c | 17 +++++++++++++++++ src/conf/capabilities.h | 7 +++++++ src/conf/domain_conf.c | 19 ++----------------- src/libvirt_private.syms | 1 + src/libxl/libxl_domain.c | 7 ++++++- src/lxc/lxc_domain.c | 5 +++++ src/openvz/openvz_conf.c | 7 ++++++- src/phyp/phyp_driver.c | 9 +++++++-- src/qemu/qemu_domain.c | 19 +++++++++++++++---- src/vmware/vmware_driver.c | 9 +++++++-- src/vmx/vmx.c | 9 +++++++-- src/vz/vz_driver.c | 7 ++++++- 13 files changed, 92 insertions(+), 31 deletions(-) diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 7d24bb602f..575f141b53 100644 --- 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 ff7d265621..748dd64273 100644 --- 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 8a7137d7eb..c39fe0de08 100644 --- 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 1c2b8f26ed..6abe15f721 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19565,14 +19565,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); @@ -19633,18 +19630,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; } @@ -19798,7 +19783,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 7bacf02f0c..74ca8585ee 100644 --- 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 19905442c1..ad9424155a 100644 --- 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 4339d305a9..b505a91c1c 100644 --- 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 8a05aa0504..de8be1ed7d 100644 --- 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 5e00ef6448..218d6f5b5c 100644 --- 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 91d5e796bb..b0e87a05c2 100644 --- 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 bab4fdb82b..be0adb1e45 100644 --- 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 c4af7b1ce9..c2a06daecb 100644 --- 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 1166b77b2c..66b15737a2 100644 --- 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; -- 2.23.0