snapshot: Add VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE flag

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, didn't document the use of the flag, and didn't
cover other XML). New APIs (like checkpoints) should do the validation
unconditionally, but it doesn't hurt to continue retrofitting existing
APIs to at least allow the option.

While there are many APIs that could be improved, this patch focuses
on wiring 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 generally always comply (note that validation
might cause failures where it used to succeed, such as if we tighten
the RNG to reject a name of '../\n'); but blindly passing the flag
means we also have to add in fallback code to disable validation if
the server is too old to understand the flag.

Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
This commit is contained in:
Eric Blake 2019-07-05 22:05:37 -05:00
parent 88ae8b8b0c
commit 95f8e3237e
10 changed files with 56 additions and 14 deletions

View File

@ -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 */

View File

@ -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)
parse_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;

View File

@ -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
* is validated 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

View File

@ -15525,7 +15525,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,
@ -15566,6 +15567,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;

View File

@ -7183,7 +7183,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;
@ -7199,6 +7200,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,

View File

@ -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;

View File

@ -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)))

View File

@ -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

View File

@ -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;
@ -383,7 +396,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"))

View File

@ -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 the 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