Before external snapshot revert every delete operation did block commit in order to delete a snapshot. But now when user reverts to non-leaf snapshot deleting leaf snapshot will not have any overlay files so we can just simply delete the snapshot images. Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx> --- src/qemu/qemu_snapshot.c | 93 ++++++++++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 08cff2a9a2..9c4d26bad5 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2569,9 +2569,26 @@ qemuSnapshotFindParentSnapForDisk(virDomainMomentObj *snap, } +/** + * qemuSnapshotDeleteExternalPrepareData: + * @vm: domain object + * @snap: snapshot object + * @merge: whether we are just deleting image or not + * @externalData: prepared data to delete external snapshot + * + * Validate if we can delete selected snapshot @snap and prepare all necessary + * data that will be used when deleting snapshot as @externalData. + * + * If @merge is set to true we will merge the deleted snapshot into parent one + * instead of just deleting it. This is necessary when operating on snapshot + * that has existing overlay files. + * + * Returns -1 on error, 0 on success. + */ static int qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm, virDomainMomentObj *snap, + bool merge, GSList **externalData) { ssize_t i; @@ -2579,7 +2596,6 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm, g_autoslist(qemuSnapshotDeleteExternalData) ret = NULL; for (i = 0; i < snapdef->ndisks; i++) { - g_autofree qemuSnapshotDeleteExternalData *data = NULL; virDomainSnapshotDiskDef *snapDisk = &(snapdef->disks[i]); if (snapDisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) @@ -2591,14 +2607,18 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm, snapDisk->name); return -1; } + } + + for (i = 0; i < snapdef->ndisks; i++) { + virDomainSnapshotDiskDef *snapDisk = &(snapdef->disks[i]); + g_autofree qemuSnapshotDeleteExternalData *data = NULL; + + if (snapDisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) + continue; data = g_new0(qemuSnapshotDeleteExternalData, 1); data->snapDisk = snapDisk; - data->domDisk = qemuDomainDiskByName(vm->def, snapDisk->name); - if (!data->domDisk) - return -1; - data->parentDomDisk = virDomainDiskByTarget(snapdef->parent.dom, data->snapDisk->name); if (!data->parentDomDisk) { @@ -2608,39 +2628,46 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm, return -1; } - if (virDomainObjIsActive(vm)) { - data->diskSrc = virStorageSourceChainLookupBySource(data->domDisk->src, - data->snapDisk->src, - &data->prevDiskSrc); - if (!data->diskSrc) + if (merge) { + data->domDisk = qemuDomainDiskByName(vm->def, snapDisk->name); + if (!data->domDisk) return -1; - if (!virStorageSourceIsSameLocation(data->diskSrc, data->snapDisk->src)) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("VM disk source and snapshot disk source are not the same")); - return -1; - } + if (virDomainObjIsActive(vm)) { + data->diskSrc = virStorageSourceChainLookupBySource(data->domDisk->src, + data->snapDisk->src, + &data->prevDiskSrc); + if (!data->diskSrc) + return -1; - data->parentDiskSrc = data->diskSrc->backingStore; - if (!virStorageSourceIsBacking(data->parentDiskSrc)) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("failed to find parent disk source in backing chain")); - return -1; - } + if (!virStorageSourceIsSameLocation(data->diskSrc, data->snapDisk->src)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("VM disk source and snapshot disk source are not the same")); + return -1; + } - if (!virStorageSourceIsSameLocation(data->parentDiskSrc, data->parentDomDisk->src)) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("snapshot VM disk source and parent disk source are not the same")); - return -1; + data->parentDiskSrc = data->diskSrc->backingStore; + if (!virStorageSourceIsBacking(data->parentDiskSrc)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("failed to find parent disk source in backing chain")); + return -1; + } + + if (!virStorageSourceIsSameLocation(data->parentDiskSrc, + data->parentDomDisk->src)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("snapshot VM disk source and parent disk source are not the same")); + return -1; + } } - } - data->parentSnap = qemuSnapshotFindParentSnapForDisk(snap, data->snapDisk); + data->parentSnap = qemuSnapshotFindParentSnapForDisk(snap, data->snapDisk); - if (data->parentSnap && !virDomainSnapshotIsExternal(data->parentSnap)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("deleting external snapshot that has internal snapshot as parent not supported")); - return -1; + if (data->parentSnap && !virDomainSnapshotIsExternal(data->parentSnap)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("deleting external snapshot that has internal snapshot as parent not supported")); + return -1; + } } ret = g_slist_prepend(ret, g_steal_pointer(&data)); @@ -2672,7 +2699,7 @@ qemuSnapshotDeleteExternalPrepare(virDomainObj *vm, } /* this also serves as validation whether the snapshot can be deleted */ - if (qemuSnapshotDeleteExternalPrepareData(vm, snap, &tmpData) < 0) + if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true, &tmpData) < 0) return -1; if (!virDomainObjIsActive(vm)) { @@ -2687,7 +2714,7 @@ qemuSnapshotDeleteExternalPrepare(virDomainObj *vm, /* Call the prepare again as some data require that the VM is * running to get everything we need. */ - if (qemuSnapshotDeleteExternalPrepareData(vm, snap, externalData) < 0) + if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true, externalData) < 0) return -1; } else { qemuDomainJobPrivate *jobPriv = vm->job->privateData; -- 2.41.0