On 3/4/19 10:34 PM, Eric Blake wrote:
> Implement the new flags for bulk snapshot dump and redefine. This
> borrows from ideas in the test driver, but is further complicated
> by the fact that qemu must write snapshot XML to disk, and thus must
> do additional validation after the initial parse to ensure the user
> didn't attempt to rename a snapshot with "../" or similar.
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> src/qemu/qemu_domain.c | 26 +++++++++++++----
> src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 83 insertions(+), 9 deletions(-)
>
[...]
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 39cc45537d..6a8f8e2bbe 100644
[...]
> static virDomainSnapshotPtr
> qemuDomainSnapshotCreateXML(virDomainPtr domain,
> const char *xmlDesc,
> @@ -15765,7 +15792,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_REDEFINE_LIST, NULL);
>
> VIR_REQUIRE_FLAG_RET(VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE,
> VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY,
> @@ -15797,6 +15825,38 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
> goto cleanup;
> }
>
> + if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE_LIST) {
> + struct qemuDomainSnapshotBulk bulk = {
> + .vm = vm,
> + .driver = driver,
> + .snapshotDir = cfg->snapshotDir,
> + .flags = flags,
> + };
> +
> + if (virDomainSnapshotObjListParse(xmlDesc, vm->def->uuid,
> + vm->snapshots, &vm->current_snapshot,
> + caps, driver->xmlopt,
> + parse_flags) < 0)
> + goto cleanup;
> + /* Validate and save the snapshots to disk. Since we don't get
> + * here unless there were no snapshots beforehand, just delete
> + * everything if anything failed, ignoring further errors. */
> + if (virDomainSnapshotForEach(vm->snapshots,
> + qemuDomainSnapshotBulkRedefine,
> + &bulk) < 0) {
> + virErrorPtr orig_err = virSaveLastError();
I've seen newer code using virErrorPreserveLast
> +
> + qemuDomainSnapshotDiscardAllMetadata(driver, vm);
> + virSetError(orig_err);
> + virFreeError(orig_err);
Similar newer code using virErrorRestore instead of both
> + goto cleanup;
> + }
> + /* Return is arbitrary, so use the first root */
> + snap = virDomainSnapshotFindByName(vm->snapshots, NULL);
> + snapshot = virGetDomainSnapshot(domain, snap->first_child->def->name);
> + goto cleanup;
> + }
> +
> if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("cannot halt after transient domain snapshot"));
>
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
John
FYI: I ran the series through Coverity too with no new errors
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
