[PATCH 0/3] interface define: add support for validation against schema
This is just the first series of patches of many more. I will send them soon, I just wanted to know if I am on the right path as they will follow the same pattern. Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1972586 Kristina Hanicova (3): conf: propagate xmlDocPtr and flags into virInterfaceDefParseXML() virsh: add support for '--validate' option in define interface interface_conf: add validation against schema in define include/libvirt/libvirt-interface.h | 4 +++ src/conf/interface_conf.c | 38 ++++++++++++++++++------- src/conf/interface_conf.h | 6 ++-- src/conf/virinterfaceobj.c | 2 +- src/interface/interface_backend_netcf.c | 6 ++-- src/test/test_driver.c | 4 +-- tests/interfacexml2xmltest.c | 2 +- tools/virsh-interface.c | 10 ++++++- 8 files changed, 51 insertions(+), 21 deletions(-) -- 2.31.1
We need to know if validation flag is present in order to validate given XML against schema in virInterfaceDefParseXML(). Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/interface_conf.c | 29 +++++++++++++++---------- src/conf/interface_conf.h | 6 +++-- src/conf/virinterfaceobj.c | 2 +- src/interface/interface_backend_netcf.c | 4 ++-- src/test/test_driver.c | 4 ++-- tests/interfacexml2xmltest.c | 2 +- 6 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index d84ec66def..7b0cfa6562 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -38,7 +38,8 @@ VIR_ENUM_IMPL(virInterface, ); static virInterfaceDef * -virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType); +virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType, + xmlDocPtr xml, unsigned int flags); static int virInterfaceDefDevFormat(virBuffer *buf, const virInterfaceDef *def, @@ -521,7 +522,7 @@ virInterfaceDefParseBridge(virInterfaceDef *def, for (i = 0; i < nbItf; i++) { ctxt->node = interfaces[i]; - itf = virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_BRIDGE); + itf = virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_BRIDGE, NULL, 0); if (itf == NULL) { ret = -1; def->data.bridge.nbItf = i; @@ -565,7 +566,7 @@ virInterfaceDefParseBondItfs(virInterfaceDef *def, for (i = 0; i < nbItf; i++) { ctxt->node = interfaces[i]; - itf = virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_BOND); + itf = virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_BOND, NULL, 0); if (itf == NULL) { def->data.bond.nbItf = i; goto cleanup; @@ -677,7 +678,9 @@ virInterfaceDefParseVlan(virInterfaceDef *def, static virInterfaceDef * virInterfaceDefParseXML(xmlXPathContextPtr ctxt, - int parentIfType) + int parentIfType, + xmlDocPtr xml G_GNUC_UNUSED, + unsigned int flags) { virInterfaceDef *def; int type; @@ -685,6 +688,7 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr lnk; + virCheckFlags(0, NULL); /* check @type */ tmp = virXPathString("string(./@type)", ctxt); @@ -797,7 +801,8 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, virInterfaceDef * virInterfaceDefParseNode(xmlDocPtr xml, - xmlNodePtr root) + xmlNodePtr root, + unsigned int flags) { g_autoptr(xmlXPathContext) ctxt = NULL; @@ -813,19 +818,20 @@ virInterfaceDefParseNode(xmlDocPtr xml, return NULL; ctxt->node = root; - return virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_LAST); + return virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_LAST, xml, flags); } static virInterfaceDef * virInterfaceDefParse(const char *xmlStr, - const char *filename) + const char *filename, + unsigned int flags) { xmlDocPtr xml; virInterfaceDef *def = NULL; if ((xml = virXMLParse(filename, xmlStr, _("(interface_definition)")))) { - def = virInterfaceDefParseNode(xml, xmlDocGetRootElement(xml)); + def = virInterfaceDefParseNode(xml, xmlDocGetRootElement(xml), flags); xmlFreeDoc(xml); } @@ -834,16 +840,17 @@ virInterfaceDefParse(const char *xmlStr, virInterfaceDef * -virInterfaceDefParseString(const char *xmlStr) +virInterfaceDefParseString(const char *xmlStr, + unsigned int flags) { - return virInterfaceDefParse(xmlStr, NULL); + return virInterfaceDefParse(xmlStr, NULL, flags); } virInterfaceDef * virInterfaceDefParseFile(const char *filename) { - return virInterfaceDefParse(NULL, filename); + return virInterfaceDefParse(NULL, filename, 0); } diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index f5e802736b..15819b000f 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -155,14 +155,16 @@ void virInterfaceDefFree(virInterfaceDef *def); virInterfaceDef * -virInterfaceDefParseString(const char *xmlStr); +virInterfaceDefParseString(const char *xmlStr, + unsigned int flags); virInterfaceDef * virInterfaceDefParseFile(const char *filename); virInterfaceDef * virInterfaceDefParseNode(xmlDocPtr xml, - xmlNodePtr root); + xmlNodePtr root, + unsigned int flags); char * virInterfaceDefFormat(const virInterfaceDef *def); diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index a73208f1fc..9439bb3d0b 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -373,7 +373,7 @@ virInterfaceObjListCloneCb(void *payload, if (!(xml = virInterfaceDefFormat(srcObj->def))) goto error; - if (!(backup = virInterfaceDefParseString(xml))) + if (!(backup = virInterfaceDefParseString(xml, 0))) goto error; VIR_FREE(xml); diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index 416e0af36f..9f93cdd657 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -862,7 +862,7 @@ static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo, goto cleanup; } - ifacedef = virInterfaceDefParseString(xmlstr); + ifacedef = virInterfaceDefParseString(xmlstr, 0); if (!ifacedef) { /* error was already reported */ goto cleanup; @@ -898,7 +898,7 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn, virObjectLock(driver); - ifacedef = virInterfaceDefParseString(xml); + ifacedef = virInterfaceDefParseString(xml, 0); if (!ifacedef) { /* error was already reported */ goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 16d70d9025..149d1afdad 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1113,7 +1113,7 @@ testParseInterfaces(testDriver *privconn, if (!node) return -1; - def = virInterfaceDefParseNode(ctxt->doc, node); + def = virInterfaceDefParseNode(ctxt->doc, node, 0); if (!def) return -1; @@ -6074,7 +6074,7 @@ testInterfaceDefineXML(virConnectPtr conn, virCheckFlags(0, NULL); virObjectLock(privconn); - if ((def = virInterfaceDefParseString(xmlStr)) == NULL) + if ((def = virInterfaceDefParseString(xmlStr, flags)) == NULL) goto cleanup; if ((obj = virInterfaceObjListAssignDef(privconn->ifaces, def)) == NULL) diff --git a/tests/interfacexml2xmltest.c b/tests/interfacexml2xmltest.c index 07d179e3a3..3785467f84 100644 --- a/tests/interfacexml2xmltest.c +++ b/tests/interfacexml2xmltest.c @@ -24,7 +24,7 @@ testCompareXMLToXMLFiles(const char *xml) if (virTestLoadFile(xml, &xmlData) < 0) goto fail; - if (!(dev = virInterfaceDefParseString(xmlData))) + if (!(dev = virInterfaceDefParseString(xmlData, 0))) goto fail; if (!(actual = virInterfaceDefFormat(dev))) -- 2.31.1
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- include/libvirt/libvirt-interface.h | 4 ++++ tools/virsh-interface.c | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-interface.h b/include/libvirt/libvirt-interface.h index 7591c6c7fb..e849699230 100644 --- a/include/libvirt/libvirt-interface.h +++ b/include/libvirt/libvirt-interface.h @@ -79,6 +79,10 @@ typedef enum { VIR_INTERFACE_XML_INACTIVE = 1 << 0 /* dump inactive interface information */ } virInterfaceXMLFlags; +typedef enum { + VIR_INTERFACE_DEFINE_VALIDATE = (1 << 0), /* Validate the XML document against schema */ +} virInterfaceDefineFlags; + char * virInterfaceGetXMLDesc (virInterfacePtr iface, unsigned int flags); virInterfacePtr virInterfaceDefineXML (virConnectPtr conn, diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 41acae5dcb..f72d40baab 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -525,6 +525,10 @@ static const vshCmdInfo info_interface_define[] = { static const vshCmdOptDef opts_interface_define[] = { VIRSH_COMMON_OPT_FILE(N_("file containing an XML interface description")), + {.name = "validate", + .type = VSH_OT_BOOL, + .help = N_("validate the XML against the schema") + }, {.name = NULL} }; @@ -535,15 +539,19 @@ cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd) const char *from = NULL; bool ret = true; char *buffer; + unsigned int flags = 0; virshControl *priv = ctl->privData; if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; + if (vshCommandOptBool(cmd, "validate")) + flags |= VIR_INTERFACE_DEFINE_VALIDATE; + if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) return false; - iface = virInterfaceDefineXML(priv->conn, buffer, 0); + iface = virInterfaceDefineXML(priv->conn, buffer, flags); VIR_FREE(buffer); if (iface != NULL) { -- 2.31.1
On Fri, Aug 06, 2021 at 15:08:15 +0200, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- include/libvirt/libvirt-interface.h | 4 ++++
The patch summary just mentions "virsh" but this is really modifying the API. In general API modifications are more important ...
tools/virsh-interface.c | 10 +++++++++-
.. perhaps even to a point where the virsh change should be separate.
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-interface.h b/include/libvirt/libvirt-interface.h index 7591c6c7fb..e849699230 100644 --- a/include/libvirt/libvirt-interface.h +++ b/include/libvirt/libvirt-interface.h @@ -79,6 +79,10 @@ typedef enum { VIR_INTERFACE_XML_INACTIVE = 1 << 0 /* dump inactive interface information */ } virInterfaceXMLFlags;
+typedef enum { + VIR_INTERFACE_DEFINE_VALIDATE = (1 << 0), /* Validate the XML document against schema */ +} virInterfaceDefineFlags; + char * virInterfaceGetXMLDesc (virInterfacePtr iface, unsigned int flags); virInterfacePtr virInterfaceDefineXML (virConnectPtr conn,
Note that the comment for this function in src/libvirt-interface.c needs to be updated as well. For now it notes that: * @flags: extra flags; not used yet, so callers should always pass 0
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 41acae5dcb..f72d40baab 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -525,6 +525,10 @@ static const vshCmdInfo info_interface_define[] = {
static const vshCmdOptDef opts_interface_define[] = { VIRSH_COMMON_OPT_FILE(N_("file containing an XML interface description")), + {.name = "validate", + .type = VSH_OT_BOOL, + .help = N_("validate the XML against the schema") + },
You also need to add this new argument to docs/manpages/virsh.rst
We need to validate the XML against schema if option '--validate' was passed to the virsh 'iface-define' command. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/interface_conf.c | 11 ++++++++++- src/interface/interface_backend_netcf.c | 4 ++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 7b0cfa6562..ddecd96c8b 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -29,6 +29,8 @@ #include "viruuid.h" #include "virbuffer.h" #include "virstring.h" +#include "virfile.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_INTERFACE @@ -688,7 +690,14 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr lnk; - virCheckFlags(0, NULL); + if (flags & VIR_INTERFACE_DEFINE_VALIDATE) { + g_autofree char *schema = virFileFindResource("interface.rng", + abs_top_srcdir "/docs/schemas", + PKGDATADIR "/schemas"); + if (!schema || + virXMLValidateAgainstSchema(schema, xml) < 0) + return NULL; + } /* check @type */ tmp = virXPathString("string(./@type)", ctxt); diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index 9f93cdd657..78fd4f9bc7 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -894,11 +894,11 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn, virInterfaceDef *ifacedef = NULL; virInterfacePtr ret = NULL; - virCheckFlags(0, NULL); + virCheckFlags(VIR_INTERFACE_DEFINE_VALIDATE, NULL); virObjectLock(driver); - ifacedef = virInterfaceDefParseString(xml, 0); + ifacedef = virInterfaceDefParseString(xml, flags); if (!ifacedef) { /* error was already reported */ goto cleanup; -- 2.31.1
participants (2)
-
Kristina Hanicova -
Peter Krempa