Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
---
src/bhyve/bhyve_driver.c | 2 +-
src/conf/cpu_conf.c | 28 ++++++++++++++++++++++++----
src/conf/cpu_conf.h | 6 ++++--
src/conf/domain_conf.c | 3 ++-
src/cpu/cpu.c | 5 +++--
src/cpu/cpu.h | 3 ++-
src/libxl/libxl_driver.c | 2 +-
src/qemu/qemu_domain.c | 5 +++--
src/qemu/qemu_driver.c | 6 +++---
src/qemu/qemu_migration_cookie.c | 3 ++-
tests/cputest.c | 5 +++--
11 files changed, 48 insertions(+), 20 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index daa20bad40..fc57ccd504 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1465,7 +1465,7 @@ bhyveConnectCompareCPU(virConnectPtr conn,
}
} else {
ret = virCPUCompareXML(caps->host.arch, caps->host.cpu,
- xmlDesc, failIncompatible);
+ xmlDesc, failIncompatible, false);
}
cleanup:
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index dea950ce68..40d8da4a8e 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -20,9 +20,11 @@
#include <config.h>
+#include "configmake.h"
#include "virerror.h"
#include "viralloc.h"
#include "virbuffer.h"
+#include "virfile.h"
#include "cpu_conf.h"
#include "domain_conf.h"
#include "virstring.h"
@@ -281,7 +283,8 @@ virCPUDefCopy(const virCPUDef *cpu)
int
virCPUDefParseXMLString(const char *xml,
virCPUType type,
- virCPUDefPtr *cpu)
+ virCPUDefPtr *cpu,
+ bool validateXML)
{
xmlDocPtr doc = NULL;
xmlXPathContextPtr ctxt = NULL;
@@ -295,7 +298,7 @@ virCPUDefParseXMLString(const char *xml,
if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt)))
goto cleanup;
- if (virCPUDefParseXML(ctxt, NULL, type, cpu) < 0)
+ if (virCPUDefParseXML(ctxt, NULL, type, cpu, validateXML) < 0)
goto cleanup;
ret = 0;
@@ -323,7 +326,8 @@ int
virCPUDefParseXML(xmlXPathContextPtr ctxt,
const char *xpath,
virCPUType type,
- virCPUDefPtr *cpu)
+ virCPUDefPtr *cpu,
+ bool validateXML)
{
g_autoptr(virCPUDef) def = NULL;
g_autofree xmlNodePtr *nodes = NULL;
@@ -348,6 +352,22 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
return -1;
}
+ if (validateXML) {
+ g_autofree char *schemafile = NULL;
+
+ if (!(schemafile = virFileFindResource("cpu.rng",
+ abs_top_srcdir "/docs/schemas",
+ PKGDATADIR "/schemas")))
+ return -1;
+
+ if (virXMLValidateNodeAgainstSchema(schemafile, ctxt->doc,
+ ctxt->node) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("'cpu' element is not valid"));
+ return -1;
+ }
+ }
+
def = virCPUDefNew();
if (type == VIR_CPU_TYPE_AUTO) {
@@ -1146,7 +1166,7 @@ virCPUDefListParse(const char **xmlCPUs,
if (!(doc = virXMLParseStringCtxt(xmlCPUs[i], _("(CPU_definition)"), &ctxt)))
goto error;
- if (virCPUDefParseXML(ctxt, NULL, cpuType, &cpus[i]) < 0)
+ if (virCPUDefParseXML(ctxt, NULL, cpuType, &cpus[i], false) < 0)
goto error;
xmlXPathFreeContext(ctxt);
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 24c51e3a63..3ef14b7932 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -192,13 +192,15 @@ virCPUDefCopyWithoutModel(const virCPUDef *cpu);
int
virCPUDefParseXMLString(const char *xml,
virCPUType type,
- virCPUDefPtr *cpu);
+ virCPUDefPtr *cpu,
+ bool validateXML);
int
virCPUDefParseXML(xmlXPathContextPtr ctxt,
const char *xpath,
virCPUType mode,
- virCPUDefPtr *cpu);
+ virCPUDefPtr *cpu,
+ bool validateXML);
bool
virCPUDefIsEqual(virCPUDefPtr src,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4d296f7bcb..2ae5329938 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21583,7 +21583,8 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(nodes);
- if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &def->cpu) < 0)
+ if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &def->cpu,
+ false) < 0)
goto error;
if (virDomainNumaDefParseXML(def->numa, ctxt) < 0)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 188c5d86b5..bf94811960 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -109,14 +109,15 @@ virCPUCompareResult
virCPUCompareXML(virArch arch,
virCPUDefPtr host,
const char *xml,
- bool failIncompatible)
+ bool failIncompatible,
+ bool validateXML)
{
g_autoptr(virCPUDef) cpu = NULL;
VIR_DEBUG("arch=%s, host=%p, xml=%s",
virArchToString(arch), host, NULLSTR(xml));
- if (virCPUDefParseXMLString(xml, VIR_CPU_TYPE_AUTO, &cpu) < 0)
+ if (virCPUDefParseXMLString(xml, VIR_CPU_TYPE_AUTO, &cpu, validateXML) < 0)
return VIR_CPU_COMPARE_ERROR;
return virCPUCompare(arch, host, cpu, failIncompatible);
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index ba8fdd07ba..cc2d132275 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -153,7 +153,8 @@ virCPUCompareResult
virCPUCompareXML(virArch arch,
virCPUDefPtr host,
const char *xml,
- bool failIncompatible);
+ bool failIncompatible,
+ bool validateXML);
virCPUCompareResult
virCPUCompare(virArch arch,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 161a6882f3..72864c2dc9 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -6541,7 +6541,7 @@ libxlConnectCompareCPU(virConnectPtr conn,
cfg = libxlDriverConfigGet(driver);
ret = virCPUCompareXML(cfg->caps->host.arch, cfg->caps->host.cpu,
- xmlDesc, failIncompatible);
+ xmlDesc, failIncompatible, false);
virObjectUnref(cfg);
return ret;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ac443c5ddc..887a88936d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3135,7 +3135,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
qemuDomainSetPrivatePathsOld(driver, vm);
- if (virCPUDefParseXML(ctxt, "./cpu", VIR_CPU_TYPE_GUEST, &priv->origCPU) < 0)
+ if (virCPUDefParseXML(ctxt, "./cpu", VIR_CPU_TYPE_GUEST, &priv->origCPU,
+ false) < 0)
goto error;
priv->chardevStdioLogd = virXPathBoolean("boolean(./chardevStdioLogd)",
@@ -10003,7 +10004,7 @@ qemuDomainSaveCookieParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED,
return -1;
if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST,
- &cookie->cpu) < 0)
+ &cookie->cpu, false) < 0)
return -1;
cookie->slirpHelper = virXPathBoolean("boolean(./slirpHelper)", ctxt) > 0;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae715c01d7..a7961ad3f0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12240,7 +12240,7 @@ qemuConnectCompareCPU(virConnectPtr conn,
return VIR_CPU_COMPARE_ERROR;
return virCPUCompareXML(driver->hostarch, cpu,
- xmlDesc, failIncompatible);
+ xmlDesc, failIncompatible, false);
}
@@ -12330,10 +12330,10 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
}
if (ARCH_IS_X86(arch)) {
- ret = virCPUCompareXML(arch, hvCPU, xmlCPU, failIncompatible);
+ ret = virCPUCompareXML(arch, hvCPU, xmlCPU, failIncompatible, false);
} else if (ARCH_IS_S390(arch) &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON)) {
- if (virCPUDefParseXMLString(xmlCPU, VIR_CPU_TYPE_AUTO, &cpu) < 0)
+ if (virCPUDefParseXMLString(xmlCPU, VIR_CPU_TYPE_AUTO, &cpu, false) < 0)
goto cleanup;
ret = qemuConnectCPUModelComparison(qemuCaps, cfg->libDir,
diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index 3ea46e1527..aef0b042e5 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -1342,7 +1342,8 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
goto error;
if (flags & QEMU_MIGRATION_COOKIE_CPU &&
- virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &mig->cpu) < 0)
+ virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &mig->cpu,
+ false) < 0)
goto error;
if (flags & QEMU_MIGRATION_COOKIE_ALLOW_REBOOT &&
diff --git a/tests/cputest.c b/tests/cputest.c
index 83d63bf495..b40fd7f7f2 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -81,7 +81,7 @@ cpuTestLoadXML(virArch arch, const char *name)
if (!(doc = virXMLParseFileCtxt(xml, &ctxt)))
goto cleanup;
- virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_AUTO, &cpu);
+ virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_AUTO, &cpu, false);
cleanup:
xmlXPathFreeContext(ctxt);
@@ -118,7 +118,8 @@ cpuTestLoadMultiXML(virArch arch,
for (i = 0; i < n; i++) {
ctxt->node = nodes[i];
- if (virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_HOST, &cpus[i]) < 0)
+ if (virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_HOST, &cpus[i],
+ false) < 0)
goto cleanup_cpus;
}
--
2.26.2
On Mon, Sep 21, 2020 at 15:07:29 +0200, Tim Wiederhake wrote:
The summary of the commit is misleading. This patch doesn't do any
validation but rather wires in the validation code to various places
without enabling it.
> Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> ---
[...]
> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> index dea950ce68..40d8da4a8e 100644
> --- a/src/conf/cpu_conf.c
> +++ b/src/conf/cpu_conf.c
> @@ -20,9 +20,11 @@
>
> #include <config.h>
>
> +#include "configmake.h"
This is suspicious. Why did you add this line?
> #include "virerror.h"
> #include "viralloc.h"
> #include "virbuffer.h"
> +#include "virfile.h"
> #include "cpu_conf.h"
> #include "domain_conf.h"
> #include "virstring.h"
[...]
> @@ -348,6 +352,22 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
> return -1;
> }
>
> + if (validateXML) {
> + g_autofree char *schemafile = NULL;
> +
> + if (!(schemafile = virFileFindResource("cpu.rng",
> + abs_top_srcdir "/docs/schemas",
> + PKGDATADIR "/schemas")))
> + return -1;
> +
> + if (virXMLValidateNodeAgainstSchema(schemafile, ctxt->doc,
> + ctxt->node) < 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("'cpu' element is not valid"));
virXMLValidateNodeAgainstSchema calls virXMLValidateAgainstSchema which
calls virXMLValidatorValidate which already reports an error.
> + return -1;
> + }
> + }
> +
> def = virCPUDefNew();
>
> if (type == VIR_CPU_TYPE_AUTO) {
On Tue, 2020-09-29 at 11:12 +0200, Peter Krempa wrote: > (...) > > > diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c > > index dea950ce68..40d8da4a8e 100644 > > --- a/src/conf/cpu_conf.c > > +++ b/src/conf/cpu_conf.c > > @@ -20,9 +20,11 @@ > > > > #include <config.h> > > > > +#include "configmake.h" > > This is suspicious. Why did you add this line? PKGDATADIR is defined there and used in the call to virFileFindResource in virCPUDefParseXML to locate the schema. Tim
On Wed, Sep 30, 2020 at 13:54:41 +0200, Tim Wiederhake wrote: > On Tue, 2020-09-29 at 11:12 +0200, Peter Krempa wrote: > > (...) > > > > > diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c > > > index dea950ce68..40d8da4a8e 100644 > > > --- a/src/conf/cpu_conf.c > > > +++ b/src/conf/cpu_conf.c > > > @@ -20,9 +20,11 @@ > > > > > > #include <config.h> > > > > > > +#include "configmake.h" > > > > This is suspicious. Why did you add this line? > > PKGDATADIR is defined there and used in the call to virFileFindResource > in virCPUDefParseXML to locate the schema. Ah, right, please do mention that in the commit message as it seems to be an unrelated change.
© 2016 - 2026 Red Hat, Inc.