[PATCH 0/6] Implement XML validation feature for node devices and storage volumes
In the last round of adding support for built-in validation the node device APIs and storage volume creation were not covered. Note that due to the close freeze date I've already marked the APIs for v8.10. Peter Krempa (6): conf: node_device: Add 'validate' argument to virNodeDeviceDefParse nodedev: Add VIR_NODE_DEVICE_(CREATE|DEFINE)_XML_VALIDATE flags nodedev|test: Implement support for validating node device XMLs conf: storage: Add support for validating storage vol XML to virStorageVolDefParse storage: Add VIR_STORAGE_VOL_CREATE_VALIDATE flag storage|test|vbox: Implement support for validating storage volume XMLs docs/manpages/virsh.rst | 19 +++++++++++++++---- include/libvirt/libvirt-nodedev.h | 19 +++++++++++++++++++ include/libvirt/libvirt-storage.h | 1 + src/conf/node_device_conf.c | 5 +++-- src/conf/node_device_conf.h | 3 ++- src/conf/storage_conf.c | 3 ++- src/conf/storage_conf.h | 2 ++ src/hypervisor/domain_driver.c | 6 +++--- src/libvirt-nodedev.c | 4 ++-- src/node_device/node_device_driver.c | 10 ++++++---- src/storage/storage_driver.c | 18 ++++++++++++++---- src/test/test_driver.c | 24 +++++++++++++++++------- src/vbox/vbox_storage.c | 8 ++++++-- tests/nodedevmdevctltest.c | 4 ++-- tests/nodedevxml2xmltest.c | 3 ++- tools/virsh-nodedev.c | 20 ++++++++++++++++++-- tools/virsh-volume.c | 14 ++++++++++++++ 17 files changed, 128 insertions(+), 35 deletions(-) -- 2.37.3
Allow callers to request XML validation against the schema. All callers for now pass 'false'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 5 +++-- src/conf/node_device_conf.h | 3 ++- src/hypervisor/domain_driver.c | 6 +++--- src/node_device/node_device_driver.c | 4 ++-- src/test/test_driver.c | 4 ++-- tests/nodedevmdevctltest.c | 4 ++-- tests/nodedevxml2xmltest.c | 3 ++- 7 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index bdfbbab434..f5283a77b3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2479,14 +2479,15 @@ virNodeDeviceDefParse(const char *str, int create, const char *virt_type, virNodeDeviceDefParserCallbacks *parserCallbacks, - void *opaque) + void *opaque, + bool validate) { g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; g_autoptr(virNodeDeviceDef) def = NULL; if (!(xml = virXMLParse(filename, str, _("(node_device_definition)"), - "device", &ctxt, NULL, false))) + "device", &ctxt, "nodedev.rng", validate))) return NULL; if (!(def = virNodeDeviceDefParseXML(ctxt, create, virt_type))) diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index a556358632..2b2c8f797e 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -382,7 +382,8 @@ virNodeDeviceDefParse(const char *str, int create, const char *virt_type, virNodeDeviceDefParserCallbacks *parserCallbacks, - void *opaque); + void *opaque, + bool validate); virNodeDeviceDef * virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index c154f00eea..d020b94921 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -395,7 +395,7 @@ virDomainDriverNodeDeviceReset(virNodeDevicePtr dev, if (!xml) return -1; - def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL); + def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL, false); if (!def) return -1; @@ -440,7 +440,7 @@ virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev, if (!xml) return -1; - def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL); + def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL, false); if (!def) return -1; @@ -488,7 +488,7 @@ virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev, if (!xml) return -1; - def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL); + def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL, false); if (!def) return -1; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 8e93b0dd6f..d067234ab3 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -885,7 +885,7 @@ nodeDeviceCreateXML(virConnectPtr conn, virt_type = virConnectGetType(conn); if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, virt_type, - &driver->parserCallbacks, NULL))) + &driver->parserCallbacks, NULL, false))) return NULL; if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0) @@ -1406,7 +1406,7 @@ nodeDeviceDefineXML(virConnect *conn, virt_type = virConnectGetType(conn); if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, virt_type, - &driver->parserCallbacks, NULL))) + &driver->parserCallbacks, NULL, false))) return NULL; if (virNodeDeviceDefineXMLEnsureACL(conn, def) < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 67c70de11d..9b397e66b1 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -7629,7 +7629,7 @@ testNodeDeviceMockCreateVport(testDriver *driver, if (!xml) goto cleanup; - if (!(def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL))) + if (!(def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL, false))) goto cleanup; VIR_FREE(def->name); @@ -7691,7 +7691,7 @@ testNodeDeviceCreateXML(virConnectPtr conn, virCheckFlags(0, NULL); - if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, NULL, NULL, NULL))) + if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, NULL, NULL, NULL, false))) goto cleanup; /* We run this simply for validation - it essentially validates that diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 02e85d4779..4dc524b5a5 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -71,7 +71,7 @@ testMdevctlCmd(virMdevctlCommand cmd_type, } if (!(def = virNodeDeviceDefParse(NULL, mdevxml, create, VIRT_TYPE, - &parser_callbacks, NULL))) + &parser_callbacks, NULL, false))) return -1; /* this function will set a stdin buffer containing the json configuration @@ -143,7 +143,7 @@ testMdevctlAutostart(const void *data G_GNUC_UNUSED) g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); if (!(def = virNodeDeviceDefParse(NULL, mdevxml, CREATE_DEVICE, VIRT_TYPE, - &parser_callbacks, NULL))) + &parser_callbacks, NULL, false))) return -1; virCommandSetDryRun(dryRunToken, &buf, true, true, NULL, NULL); diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index d1c0652e7d..068ec68769 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -23,7 +23,8 @@ testCompareXMLToXMLFiles(const char *xml, const char *outfile) if (virTestLoadFile(xml, &xmlData) < 0) goto fail; - if (!(dev = virNodeDeviceDefParse(xmlData, NULL, EXISTING_DEVICE, NULL, NULL, NULL))) + if (!(dev = virNodeDeviceDefParse(xmlData, NULL, EXISTING_DEVICE, NULL, + NULL, NULL, false))) goto fail; /* Calculate some things that are not read in */ -- 2.37.3
The node device APIs which get XML from the user don't yet support XML validation flags. Introduce virNodeDeviceCreateXMLFlags and virNodeDeviceDefineXMLFlags with the appropriate flags and add virsh support for the new flags. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 10 ++++++++-- include/libvirt/libvirt-nodedev.h | 19 +++++++++++++++++++ src/libvirt-nodedev.c | 4 ++-- tools/virsh-nodedev.c | 20 ++++++++++++++++++-- 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 61fcb2e9ca..532d4e779c 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -5203,7 +5203,7 @@ nodedev-create :: - nodedev-create FILE + nodedev-create FILE [--validate] Create a device on the host node that can then be assigned to virtual machines. Normally, libvirt is able to automatically determine which @@ -5211,6 +5211,9 @@ host nodes are available for use, but this allows registration of host hardware that libvirt did not automatically detect. *file* contains xml for a top-level <device> description of a node device. +If *--validate* is specified, validates the format of the XML document against +an internal RNG schema. + nodedev-destroy --------------- @@ -5234,11 +5237,14 @@ nodedev-define :: - nodedev-define FILE + nodedev-define FILE [--validate] Define an inactive persistent device or modify an existing persistent one from the XML *FILE*. +If *--validate* is specified, validates the format of the XML document against +an internal RNG schema. + nodedev-undefine ---------------- diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index 4fccd3f614..428b0d722f 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -130,12 +130,31 @@ int virNodeDeviceDetachFlags(virNodeDevicePtr dev, int virNodeDeviceReAttach (virNodeDevicePtr dev); int virNodeDeviceReset (virNodeDevicePtr dev); +/** + * virNodeDeviceCreateXMLFlags: + * + * Since: 8.10.0 + */ +typedef enum { + VIR_NODE_DEVICE_CREATE_XML_VALIDATE = 1 << 0, /* Validate the XML document against schema (Since: 8.10.0) */ +} virNodeDeviceCreateXMLFlags; + virNodeDevicePtr virNodeDeviceCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); int virNodeDeviceDestroy (virNodeDevicePtr dev); + +/** + * virNodeDeviceDefineXMLFlags: + * + * Since: 8.10.0 + */ +typedef enum { + VIR_NODE_DEVICE_DEFINE_XML_VALIDATE = 1 << 0, /* Validate the XML document against schema (Since: 8.10.0) */ +} virNodeDeviceDefineXMLFlags; + virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags); diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index d5a24ea2ef..1b7dee113e 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -694,7 +694,7 @@ virNodeDeviceReset(virNodeDevicePtr dev) * virNodeDeviceCreateXML: * @conn: pointer to the hypervisor connection * @xmlDesc: string containing an XML description of the device to be created - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of supported virNodeDeviceCreateXMLFlags * * Create a new device on the VM host machine, for example, virtual * HBAs created using vport_create. @@ -778,7 +778,7 @@ virNodeDeviceDestroy(virNodeDevicePtr dev) * virNodeDeviceDefineXML: * @conn: pointer to the hypervisor connection * @xmlDesc: string containing an XML description of the device to be defined - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of supported virNodeDeviceDefineXMLFlags * * Define a new device on the VM host machine, for example, a mediated device * diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 2adcad9c10..5dbec65367 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -50,6 +50,10 @@ static const vshCmdInfo info_node_device_create[] = { static const vshCmdOptDef opts_node_device_create[] = { VIRSH_COMMON_OPT_FILE(N_("file containing an XML description " "of the device")), + {.name = "validate", + .type = VSH_OT_BOOL, + .help = N_("validate the XML against the schema") + }, {.name = NULL} }; @@ -60,6 +64,7 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd) const char *from = NULL; g_autofree char *buffer = NULL; virshControl *priv = ctl->privData; + unsigned int flags = 0; if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; @@ -67,7 +72,10 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd) if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) return false; - if (!(dev = virNodeDeviceCreateXML(priv->conn, buffer, 0))) { + if (vshCommandOptBool(cmd, "validate")) + flags |= VIR_NODE_DEVICE_CREATE_XML_VALIDATE; + + if (!(dev = virNodeDeviceCreateXML(priv->conn, buffer, flags))) { vshError(ctl, _("Failed to create node device from %s"), from); return false; } @@ -1058,6 +1066,10 @@ static const vshCmdInfo info_node_device_define[] = { static const vshCmdOptDef opts_node_device_define[] = { VIRSH_COMMON_OPT_FILE(N_("file containing an XML description " "of the device")), + {.name = "validate", + .type = VSH_OT_BOOL, + .help = N_("validate the XML against the schema") + }, {.name = NULL} }; @@ -1068,6 +1080,7 @@ cmdNodeDeviceDefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) const char *from = NULL; g_autofree char *buffer = NULL; virshControl *priv = ctl->privData; + unsigned int flags = 0; if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; @@ -1075,7 +1088,10 @@ cmdNodeDeviceDefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) return false; - if (!(dev = virNodeDeviceDefineXML(priv->conn, buffer, 0))) { + if (vshCommandOptBool(cmd, "validate")) + flags |= VIR_NODE_DEVICE_DEFINE_XML_VALIDATE; + + if (!(dev = virNodeDeviceDefineXML(priv->conn, buffer, flags))) { vshError(ctl, _("Failed to define node device from '%s'"), from); return false; } -- 2.37.3
On 10/20/22 16:37, Peter Krempa wrote:
The node device APIs which get XML from the user don't yet support XML validation flags. Introduce virNodeDeviceCreateXMLFlags and virNodeDeviceDefineXMLFlags with the appropriate flags and add virsh support for the new flags.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 10 ++++++++-- include/libvirt/libvirt-nodedev.h | 19 +++++++++++++++++++ src/libvirt-nodedev.c | 4 ++-- tools/virsh-nodedev.c | 20 ++++++++++++++++++-- 4 files changed, 47 insertions(+), 6 deletions(-)
+ +/** + * virNodeDeviceDefineXMLFlags: + * + * Since: 8.10.0 + */ +typedef enum { + VIR_NODE_DEVICE_DEFINE_XML_VALIDATE = 1 << 0, /* Validate the XML document against schema (Since: 8.10.0) */ +} virNodeDeviceDefineXMLFlags; +
I know you already pushed these, but I am just wondering whether we ought to drop the _XML_ infix as it diverges from the rest of _VALIDATE flags. libvirt.git $ git grep VALIDATE -- include/ | grep _XML_ include/libvirt/libvirt-nodedev.h: VIR_NODE_DEVICE_CREATE_XML_VALIDATE = 1 << 0, /* Validate the XML document against schema (Since: 8.10.0) */ include/libvirt/libvirt-nodedev.h: VIR_NODE_DEVICE_DEFINE_XML_VALIDATE = 1 << 0, /* Validate the XML document against schema (Since: 8.10.0) */ Michal
On Tue, Nov 01, 2022 at 15:43:40 +0100, Michal Prívozník wrote:
On 10/20/22 16:37, Peter Krempa wrote:
The node device APIs which get XML from the user don't yet support XML validation flags. Introduce virNodeDeviceCreateXMLFlags and virNodeDeviceDefineXMLFlags with the appropriate flags and add virsh support for the new flags.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 10 ++++++++-- include/libvirt/libvirt-nodedev.h | 19 +++++++++++++++++++ src/libvirt-nodedev.c | 4 ++-- tools/virsh-nodedev.c | 20 ++++++++++++++++++-- 4 files changed, 47 insertions(+), 6 deletions(-)
+ +/** + * virNodeDeviceDefineXMLFlags: + * + * Since: 8.10.0 + */ +typedef enum { + VIR_NODE_DEVICE_DEFINE_XML_VALIDATE = 1 << 0, /* Validate the XML document against schema (Since: 8.10.0) */ +} virNodeDeviceDefineXMLFlags; +
I know you already pushed these, but I am just wondering whether we ought to drop the _XML_ infix as it diverges from the rest of _VALIDATE flags.
libvirt.git $ git grep VALIDATE -- include/ | grep _XML_ include/libvirt/libvirt-nodedev.h: VIR_NODE_DEVICE_CREATE_XML_VALIDATE = 1 << 0, /* Validate the XML document against schema (Since: 8.10.0) */ include/libvirt/libvirt-nodedev.h: VIR_NODE_DEVICE_DEFINE_XML_VALIDATE = 1 << 0, /* Validate the XML document against schema (Since: 8.10.0) */
The infix was based on the full API name the flags bind to: * 'virNodeDeviceCreateXMLFlags' are for 'virNodeDeviceCreateXML' There is 'virNodeDeviceCreate' which does not take an XML thus in such case I think having the proper full name of the API in the name of the flag should be done to differentiate diverging flags. * 'virNodeDeviceDefineXMLFlags' are for 'virNodeDeviceDefineXML' Now here is no other 'define' API so the XML suffix doesn't differentiate it, but for symmetry IMO the full function name should be the prefix for the flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/node_device/node_device_driver.c | 10 ++++++---- src/test/test_driver.c | 6 ++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index d067234ab3..0fdfe1db96 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -876,8 +876,9 @@ nodeDeviceCreateXML(virConnectPtr conn, g_autofree char *wwpn = NULL; virNodeDevicePtr device = NULL; const char *virt_type = NULL; + bool validate = flags & VIR_NODE_DEVICE_CREATE_XML_VALIDATE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_NODE_DEVICE_CREATE_XML_VALIDATE, NULL); if (nodeDeviceInitWait() < 0) return NULL; @@ -885,7 +886,7 @@ nodeDeviceCreateXML(virConnectPtr conn, virt_type = virConnectGetType(conn); if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, virt_type, - &driver->parserCallbacks, NULL, false))) + &driver->parserCallbacks, NULL, validate))) return NULL; if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0) @@ -1397,8 +1398,9 @@ nodeDeviceDefineXML(virConnect *conn, const char *virt_type = NULL; g_autofree char *uuid = NULL; g_autofree char *name = NULL; + bool validate = flags & VIR_NODE_DEVICE_DEFINE_XML_VALIDATE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_NODE_DEVICE_DEFINE_XML_VALIDATE, NULL); if (nodeDeviceInitWait() < 0) return NULL; @@ -1406,7 +1408,7 @@ nodeDeviceDefineXML(virConnect *conn, virt_type = virConnectGetType(conn); if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, virt_type, - &driver->parserCallbacks, NULL, false))) + &driver->parserCallbacks, NULL, validate))) return NULL; if (virNodeDeviceDefineXMLEnsureACL(conn, def) < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 9b397e66b1..58c2a02561 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -7688,10 +7688,12 @@ testNodeDeviceCreateXML(virConnectPtr conn, virNodeDeviceDef *objdef; g_autofree char *wwnn = NULL; g_autofree char *wwpn = NULL; + bool validate = flags & VIR_NODE_DEVICE_CREATE_XML_VALIDATE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_NODE_DEVICE_CREATE_XML_VALIDATE, NULL); - if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, NULL, NULL, NULL, false))) + if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, NULL, NULL, + NULL, validate))) goto cleanup; /* We run this simply for validation - it essentially validates that -- 2.37.3
Introduce the VIR_VOL_XML_PARSE_VALIDATE parser flag and wire it up into the validator. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/storage_conf.c | 3 ++- src/conf/storage_conf.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 0f4fe1451e..72c53872cb 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1407,9 +1407,10 @@ virStorageVolDefParse(virStoragePoolDef *pool, { g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; + bool validate = flags & VIR_VOL_XML_PARSE_VALIDATE; if (!(xml = virXMLParse(filename, xmlStr, _("(storage_volume_definition)"), - "volume", &ctxt, NULL, false))) + "volume", &ctxt, "storagevol.rng", validate))) return NULL; return virStorageVolDefParseXML(pool, ctxt, flags); diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index bbfdbc2f2f..fc67957cfe 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -284,6 +284,8 @@ typedef enum { VIR_VOL_XML_PARSE_NO_CAPACITY = 1 << 0, /* do not require volume capacity if the volume has a backing store */ VIR_VOL_XML_PARSE_OPT_CAPACITY = 1 << 1, + /* validate the XML against the RNG schema */ + VIR_VOL_XML_PARSE_VALIDATE = 1 << 2, } virStorageVolDefParseFlags; virStorageVolDef * -- 2.37.3
Allow users to request validation of the storage volume XML. Add new flag and virsh support. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 9 +++++++-- include/libvirt/libvirt-storage.h | 1 + tools/virsh-volume.c | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 532d4e779c..1e85b6ae78 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -6542,7 +6542,7 @@ vol-create :: - vol-create pool-or-uuid FILE [--prealloc-metadata] + vol-create pool-or-uuid FILE [--prealloc-metadata] [--validate] Create a volume from an XML <file>. @@ -6557,6 +6557,9 @@ support full allocation). This option creates a sparse image file with metadata, resulting in higher performance compared to images with no preallocation and only slightly higher initial disk space usage. +If *--validate* is specified, validates the format of the XML document against +an internal RNG schema. + **Example:** :: @@ -6574,7 +6577,7 @@ vol-create-from :: vol-create-from pool-or-uuid FILE vol-name-or-key-or-path - [--inputpool pool-or-uuid] [--prealloc-metadata] [--reflink] + [--inputpool pool-or-uuid] [--prealloc-metadata] [--reflink] [--validate] Create a volume, using another volume as input. @@ -6596,6 +6599,8 @@ When *--reflink* is specified, perform a COW lightweight copy, where the data blocks are copied only when modified. If this is not possible, the copy fails. +If *--validate* is specified, validates the format of the XML document against +an internal RNG schema. vol-create-as ------------- diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index af1c50b9f1..aaad4a3da1 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -437,6 +437,7 @@ const char* virStorageVolGetKey (virStorageVolPtr vol); typedef enum { VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA = 1 << 0, /* (Since: 1.0.1) */ VIR_STORAGE_VOL_CREATE_REFLINK = 1 << 1, /* perform a btrfs lightweight copy (Since: 1.2.13) */ + VIR_STORAGE_VOL_CREATE_VALIDATE = 1 << 2, /* Validate the XML document against schema (Since: 8.10.0) */ } virStorageVolCreateFlags; virStorageVolPtr virStorageVolCreateXML (virStoragePoolPtr pool, diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 300a0aa8e5..4f23481180 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -380,6 +380,10 @@ static const vshCmdOptDef opts_vol_create[] = { .type = VSH_OT_BOOL, .help = N_("preallocate metadata (for qcow2 instead of full allocation)") }, + {.name = "validate", + .type = VSH_OT_BOOL, + .help = N_("validate the XML against the schema") + }, {.name = NULL} }; @@ -395,6 +399,9 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "prealloc-metadata")) flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + if (vshCommandOptBool(cmd, "validate")) + flags |= VIR_STORAGE_VOL_CREATE_VALIDATE; + if (!(pool = virshCommandOptPool(ctl, cmd, "pool", NULL))) return false; @@ -446,6 +453,10 @@ static const vshCmdOptDef opts_vol_create_from[] = { .type = VSH_OT_BOOL, .help = N_("use btrfs COW lightweight copy") }, + {.name = "validate", + .type = VSH_OT_BOOL, + .help = N_("validate the XML against the schema") + }, {.name = NULL} }; @@ -468,6 +479,9 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "reflink")) flags |= VIR_STORAGE_VOL_CREATE_REFLINK; + if (vshCommandOptBool(cmd, "validate")) + flags |= VIR_STORAGE_VOL_CREATE_VALIDATE; + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; -- 2.37.3
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage/storage_driver.c | 18 ++++++++++++++---- src/test/test_driver.c | 16 ++++++++++++---- src/vbox/vbox_storage.c | 8 ++++++-- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index c25d9ca1ad..d90c1c9ee8 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1877,8 +1877,13 @@ storageVolCreateXML(virStoragePoolPtr pool, virStorageBackend *backend; virStorageVolPtr vol = NULL, newvol = NULL; g_autoptr(virStorageVolDef) voldef = NULL; + unsigned int parseFlags = VIR_VOL_XML_PARSE_OPT_CAPACITY; - virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA | + VIR_STORAGE_VOL_CREATE_VALIDATE, NULL); + + if (flags & VIR_STORAGE_VOL_CREATE_VALIDATE) + parseFlags |= VIR_VOL_XML_PARSE_VALIDATE; if (!(obj = virStoragePoolObjFromStoragePool(pool))) return NULL; @@ -1893,7 +1898,7 @@ storageVolCreateXML(virStoragePoolPtr pool, if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; - voldef = virStorageVolDefParse(def, xmldesc, NULL, VIR_VOL_XML_PARSE_OPT_CAPACITY); + voldef = virStorageVolDefParse(def, xmldesc, NULL, parseFlags); if (voldef == NULL) goto cleanup; @@ -2012,11 +2017,16 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, virStorageVolPtr vol = NULL; int buildret; g_autoptr(virStorageVolDef) voldef = NULL; + unsigned int parseFlags = VIR_VOL_XML_PARSE_NO_CAPACITY; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA | - VIR_STORAGE_VOL_CREATE_REFLINK, + VIR_STORAGE_VOL_CREATE_REFLINK | + VIR_STORAGE_VOL_CREATE_VALIDATE, NULL); + if (flags & VIR_STORAGE_VOL_CREATE_VALIDATE) + parseFlags |= VIR_VOL_XML_PARSE_VALIDATE; + obj = virStoragePoolObjFindByUUID(driver->pools, pool->uuid); if (obj && STRNEQ(pool->name, volsrc->pool)) { virObjectUnlock(obj); @@ -2066,7 +2076,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, goto cleanup; } - voldef = virStorageVolDefParse(def, xmldesc, NULL, VIR_VOL_XML_PARSE_NO_CAPACITY); + voldef = virStorageVolDefParse(def, xmldesc, NULL, parseFlags); if (voldef == NULL) goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 58c2a02561..87c7d8cf65 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -7182,14 +7182,18 @@ testStorageVolCreateXML(virStoragePoolPtr pool, virStoragePoolDef *def; virStorageVolPtr ret = NULL; g_autoptr(virStorageVolDef) privvol = NULL; + unsigned int parseFlags = 0; - virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_VOL_CREATE_VALIDATE, NULL); + + if (flags & VIR_STORAGE_VOL_CREATE_VALIDATE) + parseFlags |= VIR_VOL_XML_PARSE_VALIDATE; if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name))) return NULL; def = virStoragePoolObjGetDef(obj); - privvol = virStorageVolDefParse(def, xmldesc, NULL, 0); + privvol = virStorageVolDefParse(def, xmldesc, NULL, parseFlags); if (privvol == NULL) goto cleanup; @@ -7241,14 +7245,18 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, virStorageVolDef *origvol = NULL; virStorageVolPtr ret = NULL; g_autoptr(virStorageVolDef) privvol = NULL; + unsigned int parseFlags = 0; - virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_VOL_CREATE_VALIDATE, NULL); + + if (flags & VIR_STORAGE_VOL_CREATE_VALIDATE) + parseFlags |= VIR_VOL_XML_PARSE_VALIDATE; if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name))) return NULL; def = virStoragePoolObjGetDef(obj); - privvol = virStorageVolDefParse(def, xmldesc, NULL, 0); + privvol = virStorageVolDefParse(def, xmldesc, NULL, parseFlags); if (privvol == NULL) goto cleanup; diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c index 7d1cee562f..f6ede700f9 100644 --- a/src/vbox/vbox_storage.c +++ b/src/vbox/vbox_storage.c @@ -409,11 +409,15 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool, virStorageVolPtr ret = NULL; g_autoptr(virStorageVolDef) def = NULL; g_autofree char *homedir = NULL; + unsigned int parseFlags = 0; if (!data->vboxObj) return ret; - virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_VOL_CREATE_VALIDATE, NULL); + + if (flags & VIR_STORAGE_VOL_CREATE_VALIDATE) + parseFlags |= VIR_VOL_XML_PARSE_VALIDATE; /* since there is currently one default pool now * and virStorageVolDefFormat() just checks it type @@ -423,7 +427,7 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool, memset(&poolDef, 0, sizeof(poolDef)); poolDef.type = VIR_STORAGE_POOL_DIR; - if ((def = virStorageVolDefParse(&poolDef, xml, NULL, 0)) == NULL) + if ((def = virStorageVolDefParse(&poolDef, xml, NULL, parseFlags)) == NULL) goto cleanup; if (!def->name || -- 2.37.3
On 10/20/22 9:37 AM, Peter Krempa wrote:
In the last round of adding support for built-in validation the node device APIs and storage volume creation were not covered.
Note that due to the close freeze date I've already marked the APIs for v8.10.
Peter Krempa (6): conf: node_device: Add 'validate' argument to virNodeDeviceDefParse nodedev: Add VIR_NODE_DEVICE_(CREATE|DEFINE)_XML_VALIDATE flags nodedev|test: Implement support for validating node device XMLs conf: storage: Add support for validating storage vol XML to virStorageVolDefParse storage: Add VIR_STORAGE_VOL_CREATE_VALIDATE flag storage|test|vbox: Implement support for validating storage volume XMLs
docs/manpages/virsh.rst | 19 +++++++++++++++---- include/libvirt/libvirt-nodedev.h | 19 +++++++++++++++++++ include/libvirt/libvirt-storage.h | 1 + src/conf/node_device_conf.c | 5 +++-- src/conf/node_device_conf.h | 3 ++- src/conf/storage_conf.c | 3 ++- src/conf/storage_conf.h | 2 ++ src/hypervisor/domain_driver.c | 6 +++--- src/libvirt-nodedev.c | 4 ++-- src/node_device/node_device_driver.c | 10 ++++++---- src/storage/storage_driver.c | 18 ++++++++++++++---- src/test/test_driver.c | 24 +++++++++++++++++------- src/vbox/vbox_storage.c | 8 ++++++-- tests/nodedevmdevctltest.c | 4 ++-- tests/nodedevxml2xmltest.c | 3 ++- tools/virsh-nodedev.c | 20 ++++++++++++++++++-- tools/virsh-volume.c | 14 ++++++++++++++ 17 files changed, 128 insertions(+), 35 deletions(-)
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
participants (3)
-
Jonathon Jongsma -
Michal Prívozník -
Peter Krempa