mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-12-22 13:45:38 +00:00
qemu_snapshot: correctly update metadata when deleting external snapshot with multiple branches
XML metadata for snapshot contains only single list of disk overlays from the moment when the snapshot was taken. When user creates multiple branches of snapshots the parent snapshot will still list only the original disk overlays. This may cause an issue in a specific scenario: s1 | +- s2 +- s3 (active) For this snapshot topology when we delete s2 metadata for s1 are not updated. Now when we delete s1 the code operated with incorrect overlays from s1 metadata in order to update s3 metadata resulting in no changes to s3 metadata. Now when user tries to delete s3 it fails with following error: error: Failed to delete snapshot s3 error: operation failed: snapshot VM disk source and parent disk source are not the same For the actual deletion there is a code to figure out the correct disk source but it was not used to update metadata as well. Due to reasons how block commit in libvirt works we need to create a copy of that disk source in order to have it available when updating metadata as the original source will be freed at that point. Resolves: https://issues.redhat.com/browse/RHEL-26276 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This commit is contained in:
parent
79654f425c
commit
0a164b74eb
@ -2639,6 +2639,8 @@ typedef struct _qemuSnapshotDeleteExternalData {
|
|||||||
virDomainSnapshotDiskDef *snapDisk; /* snapshot disk definition */
|
virDomainSnapshotDiskDef *snapDisk; /* snapshot disk definition */
|
||||||
virDomainDiskDef *domDisk; /* VM disk definition */
|
virDomainDiskDef *domDisk; /* VM disk definition */
|
||||||
virStorageSource *diskSrc; /* source of disk we are deleting */
|
virStorageSource *diskSrc; /* source of disk we are deleting */
|
||||||
|
virStorageSource *diskSrcMetadata; /* copy of diskSrc to be used when updating
|
||||||
|
metadata because diskSrc is freed */
|
||||||
virDomainMomentObj *parentSnap;
|
virDomainMomentObj *parentSnap;
|
||||||
virDomainDiskDef *parentDomDisk; /* disk definition from snapshot metadata */
|
virDomainDiskDef *parentDomDisk; /* disk definition from snapshot metadata */
|
||||||
virStorageSource *parentDiskSrc; /* backing disk source of the @diskSrc */
|
virStorageSource *parentDiskSrc; /* backing disk source of the @diskSrc */
|
||||||
@ -2657,6 +2659,7 @@ qemuSnapshotDeleteExternalDataFree(qemuSnapshotDeleteExternalData *data)
|
|||||||
if (!data)
|
if (!data)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
|
virObjectUnref(data->diskSrcMetadata);
|
||||||
virObjectUnref(data->job);
|
virObjectUnref(data->job);
|
||||||
g_slist_free_full(data->disksWithBacking, g_free);
|
g_slist_free_full(data->disksWithBacking, g_free);
|
||||||
|
|
||||||
@ -2893,6 +2896,8 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
|
|||||||
if (!data->diskSrc)
|
if (!data->diskSrc)
|
||||||
return -1;
|
return -1;
|
||||||
|
|
||||||
|
data->diskSrcMetadata = virStorageSourceCopy(data->diskSrc, false);
|
||||||
|
|
||||||
if (!virStorageSourceIsSameLocation(data->diskSrc, snapDiskSrc)) {
|
if (!virStorageSourceIsSameLocation(data->diskSrc, snapDiskSrc)) {
|
||||||
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
|
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
|
||||||
_("VM disk source and snapshot disk source are not the same"));
|
_("VM disk source and snapshot disk source are not the same"));
|
||||||
@ -3049,6 +3054,7 @@ typedef struct _qemuSnapshotUpdateDisksData qemuSnapshotUpdateDisksData;
|
|||||||
struct _qemuSnapshotUpdateDisksData {
|
struct _qemuSnapshotUpdateDisksData {
|
||||||
virDomainMomentObj *snap;
|
virDomainMomentObj *snap;
|
||||||
virDomainObj *vm;
|
virDomainObj *vm;
|
||||||
|
GSList *externalData;
|
||||||
int error;
|
int error;
|
||||||
};
|
};
|
||||||
|
|
||||||
@ -3078,7 +3084,8 @@ static int
|
|||||||
qemuSnapshotUpdateDisksSingle(virDomainMomentObj *snap,
|
qemuSnapshotUpdateDisksSingle(virDomainMomentObj *snap,
|
||||||
virDomainDef *def,
|
virDomainDef *def,
|
||||||
virDomainDef *parentDef,
|
virDomainDef *parentDef,
|
||||||
virDomainSnapshotDiskDef *snapDisk)
|
virDomainSnapshotDiskDef *snapDisk,
|
||||||
|
virStorageSource *diskSrc)
|
||||||
{
|
{
|
||||||
virDomainDiskDef *disk = NULL;
|
virDomainDiskDef *disk = NULL;
|
||||||
|
|
||||||
@ -3091,7 +3098,7 @@ qemuSnapshotUpdateDisksSingle(virDomainMomentObj *snap,
|
|||||||
if (!(parentDisk = qemuDomainDiskByName(parentDef, snapDisk->name)))
|
if (!(parentDisk = qemuDomainDiskByName(parentDef, snapDisk->name)))
|
||||||
return -1;
|
return -1;
|
||||||
|
|
||||||
if (virStorageSourceIsSameLocation(snapDisk->src, disk->src)) {
|
if (virStorageSourceIsSameLocation(diskSrc, disk->src)) {
|
||||||
virObjectUnref(disk->src);
|
virObjectUnref(disk->src);
|
||||||
disk->src = virStorageSourceCopy(parentDisk->src, false);
|
disk->src = virStorageSourceCopy(parentDisk->src, false);
|
||||||
}
|
}
|
||||||
@ -3102,7 +3109,7 @@ qemuSnapshotUpdateDisksSingle(virDomainMomentObj *snap,
|
|||||||
virStorageSource *next = disk->src->backingStore;
|
virStorageSource *next = disk->src->backingStore;
|
||||||
|
|
||||||
while (next) {
|
while (next) {
|
||||||
if (virStorageSourceIsSameLocation(snapDisk->src, next)) {
|
if (virStorageSourceIsSameLocation(diskSrc, next)) {
|
||||||
cur->backingStore = next->backingStore;
|
cur->backingStore = next->backingStore;
|
||||||
next->backingStore = NULL;
|
next->backingStore = NULL;
|
||||||
virObjectUnref(next);
|
virObjectUnref(next);
|
||||||
@ -3128,17 +3135,15 @@ qemuSnapshotDeleteUpdateDisks(void *payload,
|
|||||||
qemuDomainObjPrivate *priv = data->vm->privateData;
|
qemuDomainObjPrivate *priv = data->vm->privateData;
|
||||||
virQEMUDriver *driver = priv->driver;
|
virQEMUDriver *driver = priv->driver;
|
||||||
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
|
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
|
||||||
virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(data->snap);
|
GSList *cur = NULL;
|
||||||
ssize_t i;
|
|
||||||
|
|
||||||
for (i = 0; i < snapdef->ndisks; i++) {
|
for (cur = data->externalData; cur; cur = g_slist_next(cur)) {
|
||||||
virDomainSnapshotDiskDef *snapDisk = &(snapdef->disks[i]);
|
qemuSnapshotDeleteExternalData *curdata = cur->data;
|
||||||
|
|
||||||
if (snapDisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NO)
|
|
||||||
continue;
|
|
||||||
|
|
||||||
if (qemuSnapshotUpdateDisksSingle(snap, snap->def->dom,
|
if (qemuSnapshotUpdateDisksSingle(snap, snap->def->dom,
|
||||||
data->snap->def->dom, snapDisk) < 0) {
|
data->snap->def->dom,
|
||||||
|
curdata->snapDisk,
|
||||||
|
curdata->diskSrcMetadata) < 0) {
|
||||||
data->error = -1;
|
data->error = -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -3149,7 +3154,8 @@ qemuSnapshotDeleteUpdateDisks(void *payload,
|
|||||||
dom = data->snap->def->dom;
|
dom = data->snap->def->dom;
|
||||||
|
|
||||||
if (qemuSnapshotUpdateDisksSingle(snap, snap->def->inactiveDom,
|
if (qemuSnapshotUpdateDisksSingle(snap, snap->def->inactiveDom,
|
||||||
dom, snapDisk) < 0) {
|
dom, curdata->snapDisk,
|
||||||
|
curdata->diskSrcMetadata) < 0) {
|
||||||
data->error = -1;
|
data->error = -1;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -3513,6 +3519,7 @@ qemuSnapshotDeleteUpdateParent(virDomainObj *vm,
|
|||||||
static int
|
static int
|
||||||
qemuSnapshotDiscardMetadata(virDomainObj *vm,
|
qemuSnapshotDiscardMetadata(virDomainObj *vm,
|
||||||
virDomainMomentObj *snap,
|
virDomainMomentObj *snap,
|
||||||
|
GSList *externalData,
|
||||||
bool update_parent)
|
bool update_parent)
|
||||||
{
|
{
|
||||||
qemuDomainObjPrivate *priv = vm->privateData;
|
qemuDomainObjPrivate *priv = vm->privateData;
|
||||||
@ -3540,6 +3547,7 @@ qemuSnapshotDiscardMetadata(virDomainObj *vm,
|
|||||||
if (virDomainSnapshotIsExternal(snap)) {
|
if (virDomainSnapshotIsExternal(snap)) {
|
||||||
data.snap = snap;
|
data.snap = snap;
|
||||||
data.vm = vm;
|
data.vm = vm;
|
||||||
|
data.externalData = externalData;
|
||||||
data.error = 0;
|
data.error = 0;
|
||||||
virDomainMomentForEachDescendant(snap,
|
virDomainMomentForEachDescendant(snap,
|
||||||
qemuSnapshotDeleteUpdateDisks,
|
qemuSnapshotDeleteUpdateDisks,
|
||||||
@ -3643,7 +3651,7 @@ qemuSnapshotDiscardImpl(virQEMUDriver *driver,
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (qemuSnapshotDiscardMetadata(vm, snap, update_parent) < 0)
|
if (qemuSnapshotDiscardMetadata(vm, snap, externalData, update_parent) < 0)
|
||||||
return -1;
|
return -1;
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
|
Loading…
Reference in New Issue
Block a user