We've been doing a terrible job of performing XML validation in our
various API that parse XML with a corresponding schema (we started
with domains back in commit dd69a14f, v1.2.12, but didn't catch all
domain-related APIs, and didn't cover other XMLM). New APIs (like
checkpoints) should do the validation unconditionally, but it doesn't
hurt to retrofit existing APIs to at least allow the option. Wire up
a new snapshot XML creation flag through all the hypervisors that
support snapshots, as well as exposing it in 'virsh snapshot-create'.
For 'virsh snapshot-create-as', we blindly set the flag without a
command-line option, since the XML we create from the command line
should always comply, but we have to add in code to disable validation
if the server is too old to understand the flag.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/libvirt/libvirt-domain-snapshot.h | 2 ++
src/libvirt-domain-snapshot.c | 3 +++
src/qemu/qemu_driver.c | 6 +++++-
src/test/test_driver.c | 6 +++++-
src/vbox/vbox_common.c | 11 ++++++++---
src/vz/vz_driver.c | 5 ++++-
tests/virsh-snapshot | 6 +++---
tools/virsh-snapshot.c | 15 ++++++++++++++-
tools/virsh.pod | 7 +++++--
9 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/include/libvirt/libvirt-domain-snapshot.h b/include/libvirt/libvirt-domain-snapshot.h
index 602e5def59..90673ed0fb 100644
--- a/include/libvirt/libvirt-domain-snapshot.h
+++ b/include/libvirt/libvirt-domain-snapshot.h
@@ -71,6 +71,8 @@ typedef enum {
VIR_DOMAIN_SNAPSHOT_CREATE_LIVE = (1 << 8), /* create the snapshot
while the guest is
running */
+ VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE = (1 << 9), /* validate the XML
+ against the schema */
} virDomainSnapshotCreateFlags;
/* Take a snapshot of the current VM state */
diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index 0c8023d9f6..2687a34b96 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -115,6 +115,9 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot)
* becomes current (see virDomainSnapshotCurrent()), and is a child
* of any previous current snapshot.
*
+ * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE, then @xmlDesc
+ * must validate against the <domainsnapshot> XML schema.
+ *
* If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE, then this
* is a request to reinstate snapshot metadata that was previously
* captured from virDomainSnapshotGetXMLDesc() before removing that
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9c05ab4ad1..97f3d7f786 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15508,7 +15508,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT |
VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE |
VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC |
- VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, NULL);
+ VIR_DOMAIN_SNAPSHOT_CREATE_LIVE |
+ VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE, NULL);
VIR_REQUIRE_FLAG_RET(VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE,
VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY,
@@ -15549,6 +15550,9 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
!virDomainObjIsActive(vm))
parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE;
+ if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE)
+ parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE;
+
if (!(def = virDomainSnapshotDefParseString(xmlDesc, caps, driver->xmlopt,
NULL, parse_flags)))
goto cleanup;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 7dd448bb20..e7ad4dbbd7 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -7163,7 +7163,8 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
VIR_DOMAIN_SNAPSHOT_CREATE_HALT |
VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE |
VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC |
- VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, NULL);
+ VIR_DOMAIN_SNAPSHOT_CREATE_LIVE |
+ VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE, NULL);
if ((redefine && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)))
update_current = false;
@@ -7179,6 +7180,9 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
goto cleanup;
}
+ if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE)
+ parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE;
+
if (!(def = virDomainSnapshotDefParseString(xmlDesc,
privconn->caps,
privconn->xmlopt,
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 54e31bec9d..8a912da50c 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -5487,6 +5487,8 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
nsresult rc;
resultCodeUnion result;
virDomainSnapshotPtr ret = NULL;
+ unsigned int parse_flags = (VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
+ VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL;
if (!data->vboxObj)
@@ -5496,12 +5498,15 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
/* VBox has no snapshot metadata, so this flag is trivial. */
virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA |
VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
- VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT, NULL);
+ VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT |
+ VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE, NULL);
+
+ if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE)
+ parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE;
if (!(def = virDomainSnapshotDefParseString(xmlDesc, data->caps,
data->xmlopt, NULL,
- VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
- VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE)))
+ parse_flags)))
goto cleanup;
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 2286f9a04f..50c883feca 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -2586,7 +2586,7 @@ vzDomainSnapshotCreateXML(virDomainPtr domain,
bool job = false;
VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL;
- virCheckFlags(0, NULL);
+ virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE, NULL);
if (!(dom = vzDomObjFromDomain(domain)))
return NULL;
@@ -2594,6 +2594,9 @@ vzDomainSnapshotCreateXML(virDomainPtr domain,
if (virDomainSnapshotCreateXMLEnsureACL(domain->conn, dom->def, flags) < 0)
goto cleanup;
+ if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE)
+ parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE;
+
if (!(def = virDomainSnapshotDefParseString(xmlDesc, driver->caps,
driver->xmlopt, NULL,
parse_flags)))
diff --git a/tests/virsh-snapshot b/tests/virsh-snapshot
index cb498cf54e..8eab67c9e0 100755
--- a/tests/virsh-snapshot
+++ b/tests/virsh-snapshot
@@ -180,11 +180,11 @@ compare exp err || fail=1
# Restore state with redefine
$abs_top_builddir/tools/virsh -c test:///default >out 2>err <<EOF || fail=1
# Redefine must be in topological order; this will fail
- snapshot-create test --redefine s2.xml
+ snapshot-create test --redefine s2.xml --validate
echo --err marker
# This is the right order
- snapshot-create test --redefine s3.xml
- snapshot-create test --redefine s2.xml --current
+ snapshot-create test --redefine s3.xml --validate
+ snapshot-create test --redefine s2.xml --current --validate
snapshot-info test --current
EOF
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index e9f0ee0810..f7772ce549 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -50,6 +50,13 @@ virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
snapshot = virDomainSnapshotCreateXML(dom, buffer, flags);
+ /* If no source file but validate was not recognized, try again without
+ * that flag. */
+ if (!snapshot && last_error->code == VIR_ERR_NO_SUPPORT && !from) {
+ flags &= ~VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE;
+ snapshot = virDomainSnapshotCreateXML(dom, buffer, flags);
+ }
+
/* Emulate --halt on older servers. */
if (!snapshot && last_error->code == VIR_ERR_INVALID_ARG &&
(flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
@@ -147,6 +154,10 @@ static const vshCmdOptDef opts_snapshot_create[] = {
.help = N_("require atomic operation")
},
VIRSH_COMMON_OPT_LIVE(N_("take a live snapshot")),
+ {.name = "validate",
+ .type = VSH_OT_BOOL,
+ .help = N_("validate the XML against the schema"),
+ },
{.name = NULL}
};
@@ -177,6 +188,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
if (vshCommandOptBool(cmd, "live"))
flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE;
+ if (vshCommandOptBool(cmd, "validate"))
+ flags |= VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
@@ -366,7 +379,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
const char *desc = NULL;
const char *memspec = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
- unsigned int flags = 0;
+ unsigned int flags = VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE;
const vshCmdOpt *opt = NULL;
if (vshCommandOptBool(cmd, "no-metadata"))
diff --git a/tools/virsh.pod b/tools/virsh.pod
index dc39004a66..865fb2b0da 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -4588,10 +4588,13 @@ used to represent properties of snapshots.
=item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]]
| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external>]
-[I<--quiesce>] [I<--atomic>] [I<--live>]}
+[I<--quiesce>] [I<--atomic>] [I<--live>]} [I<--validate>]
Create a snapshot for domain I<domain> with the properties specified in
-I<xmlfile>. Normally, the only properties settable for a domain snapshot
+I<xmlfile>. Optionally, the I<--validate> option can be passed to
+validate the format of the input XML file against an internal RNG
+schema (identical to using L<virt-xml-validate(1)> tool). Normally,
+the only properties settable for a domain snapshot
are the <name> and <description> elements, as well as <disks> if
I<--disk-only> is given; the rest of the fields are
ignored, and automatically filled in by libvirt. If I<xmlfile> is
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jul 05, 2019 at 23:37:31 -0500, Eric Blake wrote:
> We've been doing a terrible job of performing XML validation in our
> various API that parse XML with a corresponding schema (we started
> with domains back in commit dd69a14f, v1.2.12, but didn't catch all
> domain-related APIs, and didn't cover other XMLM). New APIs (like
s/XMLM/XMLs/ ?
> checkpoints) should do the validation unconditionally, but it doesn't
> hurt to retrofit existing APIs to at least allow the option. Wire up
> a new snapshot XML creation flag through all the hypervisors that
> support snapshots, as well as exposing it in 'virsh snapshot-create'.
> For 'virsh snapshot-create-as', we blindly set the flag without a
> command-line option, since the XML we create from the command line
> should always comply, but we have to add in code to disable validation
> if the server is too old to understand the flag.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> include/libvirt/libvirt-domain-snapshot.h | 2 ++
> src/libvirt-domain-snapshot.c | 3 +++
> src/qemu/qemu_driver.c | 6 +++++-
> src/test/test_driver.c | 6 +++++-
> src/vbox/vbox_common.c | 11 ++++++++---
> src/vz/vz_driver.c | 5 ++++-
The 'esx' driver also implements 'domainSnapshotCreateXML' as
esxDomainSnapshotCreateXML
> tests/virsh-snapshot | 6 +++---
> tools/virsh-snapshot.c | 15 ++++++++++++++-
> tools/virsh.pod | 7 +++++--
> 9 files changed, 49 insertions(+), 12 deletions(-)
[...]
> diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
> index 0c8023d9f6..2687a34b96 100644
> --- a/src/libvirt-domain-snapshot.c
> +++ b/src/libvirt-domain-snapshot.c
> @@ -115,6 +115,9 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot)
> * becomes current (see virDomainSnapshotCurrent()), and is a child
> * of any previous current snapshot.
> *
> + * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE, then @xmlDesc
> + * must validate against the <domainsnapshot> XML schema.
s/must validate/is validated/ ?
> * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE, then this
> * is a request to reinstate snapshot metadata that was previously
> * captured from virDomainSnapshotGetXMLDesc() before removing that
[...]
> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
> index 54e31bec9d..8a912da50c 100644
> --- a/src/vbox/vbox_common.c
> +++ b/src/vbox/vbox_common.c
> @@ -5487,6 +5487,8 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
> nsresult rc;
> resultCodeUnion result;
> virDomainSnapshotPtr ret = NULL;
> + unsigned int parse_flags = (VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
> + VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
Parentheses unnecessary.
> VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL;
>
> if (!data->vboxObj)
[...]
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index e9f0ee0810..f7772ce549 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -50,6 +50,13 @@ virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
>
> snapshot = virDomainSnapshotCreateXML(dom, buffer, flags);
>
> + /* If no source file but validate was not recognized, try again without
> + * that flag. */
> + if (!snapshot && last_error->code == VIR_ERR_NO_SUPPORT && !from) {
> + flags &= ~VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE;
> + snapshot = virDomainSnapshotCreateXML(dom, buffer, flags);
> + }
I think this compatibility glue is unnecessary ...
> +
> /* Emulate --halt on older servers. */
> if (!snapshot && last_error->code == VIR_ERR_INVALID_ARG &&
> (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
[...]
> @@ -177,6 +188,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
> flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
> if (vshCommandOptBool(cmd, "live"))
> flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE;
> + if (vshCommandOptBool(cmd, "validate"))
> + flags |= VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE;
>
> if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> goto cleanup;
> @@ -366,7 +379,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
> const char *desc = NULL;
> const char *memspec = NULL;
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> - unsigned int flags = 0;
> + unsigned int flags = VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE;
... just to validate something we always generated ourselves.
ACK if you remove the use of the flag in cmdSnapshotCreateAs. Other are
at your discretion.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 7/8/19 2:56 AM, Peter Krempa wrote:
> On Fri, Jul 05, 2019 at 23:37:31 -0500, Eric Blake wrote:
>> We've been doing a terrible job of performing XML validation in our
>> various API that parse XML with a corresponding schema (we started
>> with domains back in commit dd69a14f, v1.2.12, but didn't catch all
>> domain-related APIs, and didn't cover other XMLM). New APIs (like
>
> s/XMLM/XMLs/ ?
Yes. Not sure how I let that one through, but I also spotted it locally
before your mail.
>
> The 'esx' driver also implements 'domainSnapshotCreateXML' as
> esxDomainSnapshotCreateXML
Fixed on my tree:
diff --git i/src/esx/esx_driver.c w/src/esx/esx_driver.c
index 47d95abd6d..9173896fe1 100644
--- i/src/esx/esx_driver.c
+++ w/src/esx/esx_driver.c
@@ -4101,18 +4101,23 @@ esxDomainSnapshotCreateXML(virDomainPtr domain,
const char *xmlDesc,
bool diskOnly = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) != 0;
bool quiesce = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) != 0;
VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL;
+ unsigned int parse_flags = 0;
/* ESX supports disk-only and quiesced snapshots; libvirt tracks no
* snapshot metadata so supporting that flag is trivial. */
virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY |
VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE |
- VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL);
+ VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA |
+ VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE, NULL);
+
+ if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE)
+ format_flags = VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE;
if (esxVI_EnsureSession(priv->primary) < 0)
return NULL;
def = virDomainSnapshotDefParseString(xmlDesc, priv->caps,
- priv->xmlopt, NULL, 0);
+ priv->xmlopt, NULL, parse_flags);
if (!def)
return NULL;
>> +++ b/src/libvirt-domain-snapshot.c
>> @@ -115,6 +115,9 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot)
>> * becomes current (see virDomainSnapshotCurrent()), and is a child
>> * of any previous current snapshot.
>> *
>> + * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE, then @xmlDesc
>> + * must validate against the <domainsnapshot> XML schema.
>
> s/must validate/is validated/ ?
Sure. Oddly enough, we do NOT document the VIR_DOMAIN_DEFINE_VALIDATE
flag and friends; I guess I'll add another patch to my queue to rectify
that. (Uggh, this pile of yak shavings at my desk is growing taller...)
>> +++ b/src/vbox/vbox_common.c
>> @@ -5487,6 +5487,8 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
>> nsresult rc;
>> resultCodeUnion result;
>> virDomainSnapshotPtr ret = NULL;
>> + unsigned int parse_flags = (VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
>> + VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
>
> Parentheses unnecessary.
But compliant with the syntax-check, and allows for nicer indentation of
the second line. Qemu just recently had a patch related to that:
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01726.html
>> +++ b/tools/virsh-snapshot.c
>> @@ -50,6 +50,13 @@ virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
>>
>> snapshot = virDomainSnapshotCreateXML(dom, buffer, flags);
>>
>> + /* If no source file but validate was not recognized, try again without
>> + * that flag. */
>> + if (!snapshot && last_error->code == VIR_ERR_NO_SUPPORT && !from) {
>> + flags &= ~VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE;
>> + snapshot = virDomainSnapshotCreateXML(dom, buffer, flags);
>> + }
>
> I think this compatibility glue is unnecessary ...
It's necessary if snapshot-create-as uses the flag...
>> @@ -366,7 +379,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
>> const char *desc = NULL;
>> const char *memspec = NULL;
>> virBuffer buf = VIR_BUFFER_INITIALIZER;
>> - unsigned int flags = 0;
>> + unsigned int flags = VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE;
>
> ... just to validate something we always generated ourselves.
...but I can drop the use here, if you think we are safe.
>
> ACK if you remove the use of the flag in cmdSnapshotCreateAs. Other are
> at your discretion.
Okay, will drop the use in snapshot-create-as.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jul 08, 2019 at 22:12:38 -0500, Eric Blake wrote: > On 7/8/19 2:56 AM, Peter Krempa wrote: > > On Fri, Jul 05, 2019 at 23:37:31 -0500, Eric Blake wrote: > >> We've been doing a terrible job of performing XML validation in our > >> various API that parse XML with a corresponding schema (we started > >> with domains back in commit dd69a14f, v1.2.12, but didn't catch all > >> domain-related APIs, and didn't cover other XMLM). New APIs (like [...] > >> @@ -366,7 +379,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) > >> const char *desc = NULL; > >> const char *memspec = NULL; > >> virBuffer buf = VIR_BUFFER_INITIALIZER; > >> - unsigned int flags = 0; > >> + unsigned int flags = VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE; > > > > ... just to validate something we always generated ourselves. > > ...but I can drop the use here, if you think we are safe. > > > > > ACK if you remove the use of the flag in cmdSnapshotCreateAs. Other are > > at your discretion. Hmm, on a second thought, the XML is created from user-provided bits which may be validated insufficiently, so if you didn't follow through on this one you can use my ACK even with the compat glue and explicit validation. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.