[PATCH 0/4] storage pool define: add support for validation against schema

Kristina Hanicova (4): api: add virStoragePoolDefineFlags storage_conf: add validation against schema in pool define storage_driver & test_driver: allow VIR_STORAGE_POOL_DEFINE_VALIDATE flag virsh: add support for '--validate' option in define storage pool docs/manpages/virsh.rst | 4 +++- include/libvirt/libvirt-storage.h | 4 ++++ src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 13 ++++++++----- src/conf/storage_conf.h | 3 ++- src/libvirt-storage.c | 2 +- src/storage/storage_driver.c | 6 +++--- src/test/test_driver.c | 6 +++--- tools/virsh-pool.c | 11 +++++++++-- 9 files changed, 34 insertions(+), 17 deletions(-) -- 2.31.1

Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- include/libvirt/libvirt-storage.h | 4 ++++ src/libvirt-storage.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 089e1e0bd1..b459fe3e8e 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -271,6 +271,10 @@ virStoragePoolPtr virStoragePoolLookupByVolume (virStorageVolPtr vol); virStoragePoolPtr virStoragePoolLookupByTargetPath(virConnectPtr conn, const char *path); +typedef enum { + VIR_STORAGE_POOL_DEFINE_VALIDATE = 1 << 0, /* Validate the XML document against schema */ +} virStoragePoolDefineFlags; + /* * Creating/destroying pools */ diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c index 2a7cdca234..5ec61c7d99 100644 --- a/src/libvirt-storage.c +++ b/src/libvirt-storage.c @@ -589,7 +589,7 @@ virStoragePoolCreateXML(virConnectPtr conn, * virStoragePoolDefineXML: * @conn: pointer to hypervisor connection * @xml: XML description for new pool - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virStoragePoolDefineFlags * * Define an inactive persistent storage pool or modify an existing persistent * one from the XML description. -- 2.31.1

We need to validate the XML against schema if option '--validate' was passed to the virsh command. This patch also includes propagation of flags into the virStoragePoolDefParse() function. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 13 ++++++++----- src/conf/storage_conf.h | 3 ++- src/storage/storage_driver.c | 4 ++-- src/test/test_driver.c | 4 ++-- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0328447f87..6127513117 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30707,7 +30707,7 @@ virDomainStorageSourceTranslateSourcePool(virStorageSource *src, if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0))) return -1; - if (!(pooldef = virStoragePoolDefParseString(poolxml))) + if (!(pooldef = virStoragePoolDefParseString(poolxml, 0))) return -1; src->srcpool->pooltype = pooldef->type; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 040a413d0f..c78456695c 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1004,12 +1004,14 @@ virStoragePoolDefParseNode(xmlDocPtr xml, static virStoragePoolDef * virStoragePoolDefParse(const char *xmlStr, - const char *filename) + const char *filename, + unsigned int flags) { virStoragePoolDef *ret = NULL; g_autoptr(xmlDoc) xml = NULL; - if ((xml = virXMLParse(filename, xmlStr, _("(storage_pool_definition)"), NULL, false))) { + if ((xml = virXMLParse(filename, xmlStr, _("(storage_pool_definition)"), + "storagepool.rng", flags & VIR_STORAGE_POOL_DEFINE_VALIDATE))) { ret = virStoragePoolDefParseNode(xml, xmlDocGetRootElement(xml)); } @@ -1018,16 +1020,17 @@ virStoragePoolDefParse(const char *xmlStr, virStoragePoolDef * -virStoragePoolDefParseString(const char *xmlStr) +virStoragePoolDefParseString(const char *xmlStr, + unsigned int flags) { - return virStoragePoolDefParse(xmlStr, NULL); + return virStoragePoolDefParse(xmlStr, NULL, flags); } virStoragePoolDef * virStoragePoolDefParseFile(const char *filename) { - return virStoragePoolDefParse(NULL, filename); + return virStoragePoolDefParse(NULL, filename, 0); } diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 76efaac531..aaecf138d6 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -275,7 +275,8 @@ virStoragePoolDef * virStoragePoolDefParseXML(xmlXPathContextPtr ctxt); virStoragePoolDef * -virStoragePoolDefParseString(const char *xml); +virStoragePoolDefParseString(const char *xml, + unsigned int flags); virStoragePoolDef * virStoragePoolDefParseFile(const char *filename); diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6aa10d89f6..37b84d038a 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -737,7 +737,7 @@ storagePoolCreateXML(virConnectPtr conn, VIR_EXCLUSIVE_FLAGS_RET(VIR_STORAGE_POOL_BUILD_OVERWRITE, VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, NULL); - if (!(newDef = virStoragePoolDefParseString(xml))) + if (!(newDef = virStoragePoolDefParseString(xml, 0))) goto cleanup; if (virStoragePoolCreateXMLEnsureACL(conn, newDef) < 0) @@ -818,7 +818,7 @@ storagePoolDefineXML(virConnectPtr conn, virCheckFlags(0, NULL); - if (!(newDef = virStoragePoolDefParseString(xml))) + if (!(newDef = virStoragePoolDefParseString(xml, 0))) goto cleanup; if (virXMLCheckIllegalChars("name", newDef->name, "\n") < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f60ea870db..33bab0f9d3 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6645,7 +6645,7 @@ testStoragePoolCreateXML(virConnectPtr conn, virCheckFlags(0, NULL); virObjectLock(privconn); - if (!(newDef = virStoragePoolDefParseString(xml))) + if (!(newDef = virStoragePoolDefParseString(xml, 0))) goto cleanup; if (!(obj = virStoragePoolObjListAdd(privconn->pools, newDef, @@ -6708,7 +6708,7 @@ testStoragePoolDefineXML(virConnectPtr conn, virCheckFlags(0, NULL); virObjectLock(privconn); - if (!(newDef = virStoragePoolDefParseString(xml))) + if (!(newDef = virStoragePoolDefParseString(xml, 0))) goto cleanup; newDef->capacity = defaultPoolCap; -- 2.31.1

Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/storage/storage_driver.c | 4 ++-- src/test/test_driver.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 37b84d038a..51daf6a05d 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -816,9 +816,9 @@ storagePoolDefineXML(virConnectPtr conn, virObjectEvent *event = NULL; g_autoptr(virStoragePoolDef) newDef = NULL; - virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_POOL_DEFINE_VALIDATE, NULL); - if (!(newDef = virStoragePoolDefParseString(xml, 0))) + if (!(newDef = virStoragePoolDefParseString(xml, flags))) goto cleanup; if (virXMLCheckIllegalChars("name", newDef->name, "\n") < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 33bab0f9d3..2f19b7c520 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6705,10 +6705,10 @@ testStoragePoolDefineXML(virConnectPtr conn, virObjectEvent *event = NULL; g_autoptr(virStoragePoolDef) newDef = NULL; - virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_POOL_DEFINE_VALIDATE, NULL); virObjectLock(privconn); - if (!(newDef = virStoragePoolDefParseString(xml, 0))) + if (!(newDef = virStoragePoolDefParseString(xml, flags))) goto cleanup; newDef->capacity = defaultPoolCap; -- 2.31.1

Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- docs/manpages/virsh.rst | 4 +++- tools/virsh-pool.c | 11 +++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 850a862fd9..2204bed3bb 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -5958,10 +5958,12 @@ pool-define :: - pool-define file + pool-define file [--validate] Define an inactive persistent storage pool or modify an existing persistent one from the XML *file*. +Optionally, the format of the input XML file can be validated against an +internal RNG schema with *--validate*. pool-define-as diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index f0ee95bdf8..1a2ab8cc53 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -520,7 +520,10 @@ static const vshCmdInfo info_pool_define[] = { static const vshCmdOptDef opts_pool_define[] = { VIRSH_COMMON_OPT_FILE(N_("file containing an XML pool description")), - + {.name = "validate", + .type = VSH_OT_BOOL, + .help = N_("validate the XML against the schema") + }, {.name = NULL} }; @@ -531,15 +534,19 @@ cmdPoolDefine(vshControl *ctl, const vshCmd *cmd) const char *from = NULL; bool ret = true; g_autofree char *buffer = NULL; + unsigned int flags = 0; virshControl *priv = ctl->privData; if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; + if (vshCommandOptBool(cmd, "validate")) + flags |= VIR_STORAGE_POOL_DEFINE_VALIDATE; + if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) return false; - pool = virStoragePoolDefineXML(priv->conn, buffer, 0); + pool = virStoragePoolDefineXML(priv->conn, buffer, flags); if (pool != NULL) { vshPrintExtra(ctl, _("Pool %s defined from %s\n"), -- 2.31.1

On 8/24/21 4:51 PM, Kristina Hanicova wrote:
Kristina Hanicova (4): api: add virStoragePoolDefineFlags storage_conf: add validation against schema in pool define storage_driver & test_driver: allow VIR_STORAGE_POOL_DEFINE_VALIDATE flag virsh: add support for '--validate' option in define storage pool
docs/manpages/virsh.rst | 4 +++- include/libvirt/libvirt-storage.h | 4 ++++ src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 13 ++++++++----- src/conf/storage_conf.h | 3 ++- src/libvirt-storage.c | 2 +- src/storage/storage_driver.c | 6 +++--- src/test/test_driver.c | 6 +++--- tools/virsh-pool.c | 11 +++++++++-- 9 files changed, 34 insertions(+), 17 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Kristina Hanicova
-
Michal Prívozník