[libvirt PATCH] qemu: snapshot: delete disk image only if parent snapshot is external
When we are deleting external snapshot that is not active we only need to delete overlay disk image of the parent snapshot. This works correctly even if parent snapshot is external and active as it will have another overlay created when user reverted to that snapshot. In case the parent snapshot is internal there are no overlay disk images created as everything is stored internally within the disk image. In this case we would delete the actual disk image storing internal snapshots and most likely the original disk image as well resulting in data loss once the VM is shutoff. Fixes: https://gitlab.com/libvirt/libvirt/-/issues/734 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_snapshot.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 18b2e478f6..80cd54bf33 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -3144,6 +3144,8 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm, return -1; } + data->parentSnap = qemuSnapshotFindParentSnapForDisk(snap, data->snapDisk); + if (data->merge) { virStorageSource *snapDiskSrc = NULL; @@ -3185,8 +3187,6 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm, qemuSnapshotGetDisksWithBackingStore(vm, snap, data); } - 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")); @@ -3642,10 +3642,12 @@ qemuSnapshotDiscardExternal(virDomainObj *vm, if (!data->job) goto error; } else { - if (virStorageSourceInit(data->parentDomDisk->src) < 0 || - virStorageSourceUnlink(data->parentDomDisk->src) < 0) { - VIR_WARN("Failed to remove snapshot image '%s'", - data->snapDisk->name); + if (data->parentSnap && virDomainSnapshotIsExternal(data->parentSnap)) { + if (virStorageSourceInit(data->parentDomDisk->src) < 0 || + virStorageSourceUnlink(data->parentDomDisk->src) < 0) { + VIR_WARN("Failed to remove snapshot image '%s'", + data->snapDisk->name); + } } } } -- 2.47.1
On Fri, Jan 10, 2025 at 15:21:21 +0100, Pavel Hrdina wrote:
When we are deleting external snapshot that is not active we only need to delete overlay disk image of the parent snapshot. This works correctly even if parent snapshot is external and active as it will have another overlay created when user reverted to that snapshot.
In case the parent snapshot is internal there are no overlay disk images created as everything is stored internally within the disk image. In this case we would delete the actual disk image storing internal snapshots and most likely the original disk image as well resulting in data loss once the VM is shutoff.
This definitely requires a NEWS entry as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_snapshot.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Fri, Jan 10, 2025 at 04:10:53PM +0100, Peter Krempa wrote:
On Fri, Jan 10, 2025 at 15:21:21 +0100, Pavel Hrdina wrote:
When we are deleting external snapshot that is not active we only need to delete overlay disk image of the parent snapshot. This works correctly even if parent snapshot is external and active as it will have another overlay created when user reverted to that snapshot.
In case the parent snapshot is internal there are no overlay disk images created as everything is stored internally within the disk image. In this case we would delete the actual disk image storing internal snapshots and most likely the original disk image as well resulting in data loss once the VM is shutoff.
This definitely requires a NEWS entry as well.
Good point, I'll send it as follow up.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_snapshot.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Pavel Hrdina -
Peter Krempa