On 25.09.2013 21:15, Cole Robinson wrote:
> ---
> src/conf/snapshot_conf.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++
> src/conf/snapshot_conf.h | 7 +++
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_driver.c | 131 +----------------------------------------
> 4 files changed, 160 insertions(+), 129 deletions(-)
Almost.
>
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index 207a8fe..1fcc4cb 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -1099,3 +1099,153 @@ virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap)
> {
> return virDomainSnapshotDefIsExternal(snap->def);
> }
> +
> +int
> +virDomainSnapshotRedefinePrep(virDomainPtr domain,
> + virDomainObjPtr vm,
> + virDomainSnapshotDefPtr *defptr,
> + virDomainSnapshotObjPtr *snap,
> + bool *update_current,
> + unsigned int flags)
> +{
> + virDomainSnapshotDefPtr def = *defptr;
> + int ret = -1;
> + int align_location;
> + int align_match;
These two ^^^ had a default set ...
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virDomainSnapshotObjPtr other;
> +
> + virUUIDFormat(domain->uuid, uuidstr);
> +
> + /* Prevent circular chains */
> + if (def->parent) {
> + if (STREQ(def->name, def->parent)) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("cannot set snapshot %s as its own parent"),
> + def->name);
> + goto cleanup;
> + }
> + other = virDomainSnapshotFindByName(vm->snapshots, def->parent);
> + if (!other) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("parent %s for snapshot %s not found"),
> + def->parent, def->name);
> + goto cleanup;
> + }
> + while (other->def->parent) {
> + if (STREQ(other->def->parent, def->name)) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("parent %s would create cycle to %s"),
> + other->def->name, def->name);
> + goto cleanup;
> + }
> + other = virDomainSnapshotFindByName(vm->snapshots,
> + other->def->parent);
> + if (!other) {
> + VIR_WARN("snapshots are inconsistent for %s",
> + vm->def->name);
> + break;
> + }
> + }
> + }
> +
> + /* Check that any replacement is compatible */
> + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) &&
> + def->state != VIR_DOMAIN_DISK_SNAPSHOT) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("disk-only flag for snapshot %s requires "
> + "disk-snapshot state"),
> + def->name);
> + goto cleanup;
> +
> + }
> +
> + if (def->dom &&
> + memcmp(def->dom->uuid, domain->uuid, VIR_UUID_BUFLEN)) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("definition for snapshot %s must use uuid %s"),
> + def->name, uuidstr);
> + goto cleanup;
> + }
> +
> + other = virDomainSnapshotFindByName(vm->snapshots, def->name);
> + if (other) {
> + if ((other->def->state == VIR_DOMAIN_RUNNING ||
> + other->def->state == VIR_DOMAIN_PAUSED) !=
> + (def->state == VIR_DOMAIN_RUNNING ||
> + def->state == VIR_DOMAIN_PAUSED)) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("cannot change between online and offline "
> + "snapshot state in snapshot %s"),
> + def->name);
> + goto cleanup;
> + }
> +
> + if ((other->def->state == VIR_DOMAIN_DISK_SNAPSHOT) !=
> + (def->state == VIR_DOMAIN_DISK_SNAPSHOT)) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("cannot change between disk snapshot and "
> + "system checkpoint in snapshot %s"),
> + def->name);
> + goto cleanup;
> + }
> +
> + if (other->def->dom) {
> + if (def->dom) {
> + if (!virDomainDefCheckABIStability(other->def->dom,
> + def->dom))
> + goto cleanup;
> + } else {
> + /* Transfer the domain def */
> + def->dom = other->def->dom;
> + other->def->dom = NULL;
> + }
> + }
> +
> + if (def->dom) {
> + if (def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
> + virDomainSnapshotDefIsExternal(def)) {
> + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
> + align_match = false;
> + }
> +
> + if (virDomainSnapshotAlignDisks(def, align_location,
> + align_match) < 0) {
... and here you're calling them with a random value (unless the
previous if evaluates true).
> + /* revert stealing of the snapshot domain definition */
> + if (def->dom && !other->def->dom) {
> + other->def->dom = def->dom;
> + def->dom = NULL;
> + }
> + goto cleanup;
> + }
> + }
ACK once you fix it.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list