Skip to content

Fix for KVM: createVolume / createTemplate from incremental snapshot fails randomly, and managesnapshot.sh skips mkdir #14568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
sureshanaparti wants to merge 2 commits into
base: 4.22
Choose a base branch
from
+38 −13
Draft

Fix for KVM: createVolume / createTemplate from incremental snapshot fails randomly, and managesnapshot.sh skips mkdir #14568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
60b96ae
Fix for KVM: createVolume / createTemplate from incremental snapshot …
sureshanaparti Nov 3, 2026
0e55bf8
review
sureshanaparti Nov 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...ge/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public DeleteSnapshotContext(AsyncCompletionCallback<T> callback, SnapshotInfo s

}

static private class CopySnapshotContext<T> extends AsyncRpcContext<T> {
static class CopySnapshotContext<T> extends AsyncRpcContext<T> {
final SnapshotInfo srcSnapshot;
final SnapshotInfo destSnapshot;
final AsyncCallFuture<SnapshotResult> future;
Expand Down Expand Up @@ -494,7 +494,7 @@ protected Void copySnapshotAsyncCallback(AsyncCallbackDispatcher<SnapshotService
SnapshotResult snapResult = new SnapshotResult(destSnapshot, result.getAnswer());
if (result.isFailed()) {
try {
if (BooleanUtils.isTrue(createSnapshotPayload.getAsyncBackup())) {
if (createSnapshotPayload != null && BooleanUtils.isTrue(createSnapshotPayload.getAsyncBackup())) {

@Damans227 Damans227 Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sureshanaparti if this copy fails during createVolume or createTemplate, does the users snapshot get deleted now? looks like the cleanup below removes the snapshot itself, not just the temp copy

_snapshotDao.remove(srcSnapshot.getId());
destSnapshot.processEvent(Event.OperationFailed);
throw new SnapshotBackupException(String.format("Failed in creating backup of snapshot %s", srcSnapshot));
Expand Down
28 changes: 28 additions & 0 deletions ...napshot/src/test/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
import com.cloud.agent.api.Answer;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.ImageStore;
import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
Expand All @@ -31,7 +33,9 @@
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
import org.apache.cloudstack.framework.async.AsyncCallFuture;
import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher;
import org.apache.cloudstack.secstorage.heuristics.HeuristicType;
import org.apache.cloudstack.storage.command.CommandResult;
import org.apache.cloudstack.storage.command.CreateObjectAnswer;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
Expand Down Expand Up @@ -257,4 +261,28 @@ public void updateSnapSizeAndCheckpointPathIfPossibleTestResultIsTrueAnswerIsCre

Mockito.verify(snapshotDataStoreDaoMock).update(Mockito.anyLong(), Mockito.any());
}

@Test
@SuppressWarnings("unchecked")
public void copySnapshotAsyncCallbackTestFailedResultWithoutPayloadCompletesFutureWithAgentError() throws Exception {
SnapshotObject srcSnapshot = Mockito.mock(SnapshotObject.class);
Mockito.when(srcSnapshot.getPayload()).thenReturn(null);
SnapshotInfo destSnapshot = Mockito.mock(SnapshotInfo.class);
AsyncCallFuture<SnapshotResult> future = new AsyncCallFuture<>();
SnapshotServiceImpl.CopySnapshotContext<CommandResult> context = new SnapshotServiceImpl.CopySnapshotContext<>(null, srcSnapshot, destSnapshot, future);

CopyCommandResult result = new CopyCommandResult(null, null);
result.setResult("Failed to backup snapshot: qemu-img: Could not create destination: No such file or directory");
AsyncCallbackDispatcher<SnapshotServiceImpl, CopyCommandResult> callback = Mockito.mock(AsyncCallbackDispatcher.class);
Mockito.when(callback.getResult()).thenReturn(result);
Mockito.doNothing().when(snapshotService).cleanupOnSnapshotBackupFailure(srcSnapshot);

snapshotService.copySnapshotAsyncCallback(callback, context);

SnapshotResult snapshotResult = future.get();
Assert.assertTrue(snapshotResult.isFailed());
Assert.assertEquals(result.getResult(), snapshotResult.getResult());
Mockito.verify(destSnapshot).processEvent(ObjectInDataStoreStateMachine.Event.OperationFailed);
Mockito.verify(snapshotService).cleanupOnSnapshotBackupFailure(srcSnapshot);
}
}
19 changes: 8 additions & 11 deletions scripts/storage/qcow2/managesnapshot.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,11 @@ backup_snapshot() {
local destName=$4
local forceShareFlag="-U"

if [ ! -d $destPath ]
mkdir_ret=$(mkdir -p "$destPath" 2>&1)
if [ $? -gt 0 ]
then
mkdir -p $destPath >& /dev/null
if [ $? -gt 0 ]
then
printf "Failed to create $destPath\n" >&2
return 3
fi
printf "Failed to create %s: %s\n" "$destPath" "$mkdir_ret" >&2
return 3
fi

is_lv ${disk}
Expand Down Expand Up @@ -228,12 +225,12 @@ backup_snapshot() {
elif [ -f ${disk} ]; then
if [[ $disk == *"/snapshots/"* ]]; then
#Backup volume snapshot
$qemu_img convert $forceShareFlag -f qcow2 -O qcow2 $disk $destPath/$destName >& /dev/null
qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 $disk $destPath/$destName 2>&1 > /dev/null)
ret_code=$?

if [ $ret_code -gt 0 ]
then
printf "Failed to backup $snapshotname for disk $disk to $destPath\n" >&2
printf "Failed to backup %s for disk %s to %s: %s\n" "$snapshotname" "$disk" "$destPath" "$qemuimg_ret" >&2
return 2
fi
else
Expand All @@ -260,12 +257,12 @@ backup_snapshot() {
fi

if [ $ret_code -gt 0 ] && [[ $qemuimg_ret == *"convert: invalid option -- 'l'"* ]]; then
$qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -s $snapshotname $disk $destPath/$destName >& /dev/null
qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -s $snapshotname $disk $destPath/$destName 2>&1 > /dev/null)
ret_code=$?
fi

if [ $ret_code -gt 0 ]; then
printf "Failed to backup $snapshotname for disk $disk to $destPath\n" >&2
printf "Failed to backup %s for disk %s to %s: %s\n" "$snapshotname" "$disk" "$destPath" "$qemuimg_ret" >&2
return 2
fi
fi
Expand Down
Loading