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 committed Aug 4, 2022
1 parent 1359c3d commit 561123f
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ 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().setChildCommands(new HashMap<>());
setSucceeded(true);
}

Expand All @@ -65,6 +63,10 @@ public void handleFailure() {

@Override
public boolean performNextOperation(int completedChildCount) {
if (getParameters().getCommandStep() == null) {
getParameters().setCommandStep(getInitialMergeStepForImage());
getParameters().setChildCommands(new HashMap<>());
}
// Upon recovery or after invoking a new child command, our map may be missing an entry
syncChildCommandList(getParameters());
Guid currentChildId = getCurrentChildId(getParameters());
Expand Down Expand Up @@ -113,6 +115,14 @@ public boolean performNextOperation(int completedChildCount) {
}
}

private RemoveSnapshotSingleDiskStep getInitialMergeStepForImage() {
if (getImageInfoFromVdsm(getDestinationDiskImage()) == null) {
log.info("Image has been previously merged and deleted, proceeding to DB sync");
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 has been previously merged and deleted, proceeding to DB sync");
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 561123f

Please sign in to comment.