9:37 a.m.
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
9:37 a.m.
New subject: [PATCH 1/6] conf: node_device: Add 'validate' argument to virNodeDeviceDefParse
Allow callers to request XML validation against the schema. All callers
for now pass 'false'.
Signed-off-by: Peter Krempa <pkrempa(a)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
9:37 a.m.
New subject: [PATCH 2/6] nodedev: Add VIR_NODE_DEVICE_(CREATE|DEFINE)_XML_VALIDATE flags
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(a)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
9:43 a.m.
New subject: [PATCH 2/6] nodedev: Add VIR_NODE_DEVICE_(CREATE|DEFINE)_XML_VALIDATE flags
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(a)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
10:08 a.m.
New subject: [PATCH 2/6] nodedev: Add VIR_NODE_DEVICE_(CREATE|DEFINE)_XML_VALIDATE flags
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(a)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.
9:37 a.m.
New subject: [PATCH 3/6] nodedev|test: Implement support for validating node device XMLs
Signed-off-by: Peter Krempa <pkrempa(a)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
9:37 a.m.
New subject: [PATCH 4/6] conf: storage: Add support for validating storage vol XML to virStorageVolDefParse
Introduce the VIR_VOL_XML_PARSE_VALIDATE parser flag and wire it up into
the validator.
Signed-off-by: Peter Krempa <pkrempa(a)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
9:37 a.m.
New subject: [PATCH 5/6] storage: Add VIR_STORAGE_VOL_CREATE_VALIDATE flag
Allow users to request validation of the storage volume XML. Add new
flag and virsh support.
Signed-off-by: Peter Krempa <pkrempa(a)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
9:37 a.m.
New subject: [PATCH 6/6] storage|test|vbox: Implement support for validating storage volume XMLs
Signed-off-by: Peter Krempa <pkrempa(a)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
10:03 a.m.
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(a)redhat.com>
747
days inactive
759
days old
9 comments
3 participants
participants (3)
-
Jonathon Jongsma -
Michal Prívozník -
Peter Krempa