Skip to content

Commit

Permalink
core: fix snapshot remove when image already merged
Browse files Browse the repository at this point in the history
When trying to remove a snapshot which was already removed before
and image merged, but not removed from the DB for some reason -
the RemoveSnapshot command fails and the snapshot and images stay in
the DB and cannot be removed.

The reason for this is that during RemoveSnapshotSingleDiskLiveCommand
the DestroyImage command fails (since the image does not not exist),
thus the whole command fails, and the snapshot/image are not removed
from the DB.

In the ColdMerge case, the ColdMergeSnapshotSingleDiskCommand fails
already at the PrepareMerge stage.

In this change, for both Live and Cold merge - we first check that the
Destination image (or the Top Image) exists on storage with
GetVolumeInfo. If it does not, we conclude that it was already merged
and removed, and proceed to finishing the command successfully and
removing the snapshot and the image from the DB, skipping all the
steps.

Bug-Url: https://bugzilla.redhat.com/1948599
  • Loading branch information
mkemel authored and ahadas committed Aug 8, 2022
1 parent f7561f9 commit 9c7e00c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected void executeCommand() {
// Let doPolling() drive the execution; we don't have any guarantee that
// executeCommand() will finish before doPolling() is called, and we don't
// want to possibly run the first command twice.
getParameters().setCommandStep(RemoveSnapshotSingleDiskStep.PREPARE_MERGE);
getParameters().setCommandStep(getInitialMergeStepForImage());
getParameters().setChildCommands(new HashMap<>());
setSucceeded(true);
}
Expand Down Expand Up @@ -113,6 +113,14 @@ public boolean performNextOperation(int completedChildCount) {
}
}

private RemoveSnapshotSingleDiskStep getInitialMergeStepForImage() {
if (getImageInfoFromVdsm(getDestinationDiskImage()) == null) {
log.info("Image does not exist, attempting to synchronize the database");
return RemoveSnapshotSingleDiskStep.COMPLETE;
}
return RemoveSnapshotSingleDiskStep.PREPARE_MERGE;
}

@Override
public void endSuccessfully() {
syncDbRecords(getImageInfoFromVdsm(getDiskImage()), Collections.singleton(getDestinationImageId()), true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,13 @@ private RemoveSnapshotSingleDiskStep getInitialMergeStepForImage(Guid imageId) {
return RemoveSnapshotSingleDiskStep.DESTROY_IMAGE;
}
}
if (getImageInfoFromVdsm(getDestinationDiskImage()) == null) {
log.info("Image does not exist, attempting to synchronize the database");
Set<Guid> imagesToRemove = new HashSet<>();
imagesToRemove.add(getDestinationDiskImage().getImageId());
getParameters().setMergeStatusReturnValue(new MergeStatusReturnValue(imagesToRemove));
return RemoveSnapshotSingleDiskStep.COMPLETE;
}
return RemoveSnapshotSingleDiskStep.EXTEND;
}

Expand Down

0 comments on commit 9c7e00c

Please sign in to comment.