Re: [libvirt] [PATCH 4/8] 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, 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(a)redhat.com&gt; --- 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/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/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)

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

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