From 40dec996591a99d40b51313fa23c8d937bed780f Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 23 Sep 2025 12:07:10 +0530 Subject: [PATCH] server: Cleanup allocated snapshots / vm snapshots, and update pending ones to Error on MS start (#8452) * Remove allocated snapshots / vm snapshots on start * Check and Cleanup snapshots / vm snapshots on MS start * rebase fixes * Update volume state (from Snapshotting) on MS start when its snapshot job not finished and snapshot in Creating state --- .../cloud/vm/snapshot/VMSnapshotService.java | 3 + .../api/storage/SnapshotDataFactory.java | 3 + .../api/storage/VMSnapshotStrategy.java | 3 + .../cloud/vm/snapshot/VMSnapshotManager.java | 1 - .../cloud/vm/snapshot/dao/VMSnapshotDao.java | 2 + .../snapshot/SnapshotDataFactoryImpl.java | 22 +++++++- .../vmsnapshot/DefaultVMSnapshotStrategy.java | 10 ++++ .../vmsnapshot/ScaleIOVMSnapshotStrategy.java | 10 ++++ .../jobs/impl/AsyncJobManagerImpl.java | 55 ++++++++++++++++++- .../vm/snapshot/VMSnapshotManagerImpl.java | 7 +++ 10 files changed, 111 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/com/cloud/vm/snapshot/VMSnapshotService.java b/api/src/main/java/com/cloud/vm/snapshot/VMSnapshotService.java index 84a56aaedd3..754e463e710 100644 --- a/api/src/main/java/com/cloud/vm/snapshot/VMSnapshotService.java +++ b/api/src/main/java/com/cloud/vm/snapshot/VMSnapshotService.java @@ -19,6 +19,7 @@ package com.cloud.vm.snapshot; import java.util.List; +import com.cloud.utils.fsm.NoTransitionException; import org.apache.cloudstack.api.command.user.vmsnapshot.ListVMSnapshotCmd; import com.cloud.exception.ConcurrentOperationException; @@ -53,4 +54,6 @@ public interface VMSnapshotService { * @param id vm id */ boolean deleteVMSnapshotsFromDB(Long vmId, boolean unmanage); + + void updateOperationFailed(VMSnapshot vmSnapshot) throws NoTransitionException; } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java index 2a2db4af3e5..d318707a29a 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java @@ -21,6 +21,7 @@ package org.apache.cloudstack.engine.subsystem.api.storage; import java.util.List; import com.cloud.storage.DataStoreRole; +import com.cloud.utils.fsm.NoTransitionException; public interface SnapshotDataFactory { SnapshotInfo getSnapshot(long snapshotId, DataStore store); @@ -42,4 +43,6 @@ public interface SnapshotDataFactory { List listSnapshotOnCache(long snapshotId); SnapshotInfo getReadySnapshotOnCache(long snapshotId); + + void updateOperationFailed(long snapshotId) throws NoTransitionException; } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java index 6372aa3bd94..2939342ec27 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java @@ -18,6 +18,7 @@ */ package org.apache.cloudstack.engine.subsystem.api.storage; +import com.cloud.utils.fsm.NoTransitionException; import com.cloud.vm.snapshot.VMSnapshot; public interface VMSnapshotStrategy { @@ -44,4 +45,6 @@ public interface VMSnapshotStrategy { * @return true if vm snapshot removed from DB, false if not. */ boolean deleteVMSnapshotFromDB(VMSnapshot vmSnapshot, boolean unmanage); + + void updateOperationFailed(VMSnapshot vmSnapshot) throws NoTransitionException; } diff --git a/engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSnapshotManager.java b/engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSnapshotManager.java index a01d4ee5cae..997b413c099 100644 --- a/engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSnapshotManager.java +++ b/engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSnapshotManager.java @@ -54,5 +54,4 @@ public interface VMSnapshotManager extends VMSnapshotService, Manager { boolean hasActiveVMSnapshotTasks(Long vmId); RestoreVMSnapshotCommand createRestoreCommand(UserVmVO userVm, List vmSnapshotVOs); - } diff --git a/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java b/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java index 0143aaa1e73..bbdbfb5b520 100644 --- a/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java @@ -38,6 +38,8 @@ public interface VMSnapshotDao extends GenericDao, StateDao< VMSnapshotVO findByName(Long vmId, String name); List listByAccountId(Long accountId); + List searchByVms(List vmIds); + List searchRemovedByVms(List vmIds, Long batchSize); } diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java index 4d8919ccc48..cd314b0be63 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java @@ -23,11 +23,16 @@ import java.util.List; import javax.inject.Inject; +import com.cloud.storage.Snapshot; +import com.cloud.storage.Volume; +import com.cloud.utils.fsm.NoTransitionException; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; 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.SnapshotDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; import org.apache.commons.collections.CollectionUtils; @@ -73,7 +78,7 @@ public class SnapshotDataFactoryImpl implements SnapshotDataFactory { for (SnapshotDataStoreVO snapshotDataStoreVO : allSnapshotsFromVolumeAndDataStore) { DataStore store = storeMgr.getDataStore(snapshotDataStoreVO.getDataStoreId(), role); SnapshotVO snapshot = snapshotDao.findById(snapshotDataStoreVO.getSnapshotId()); - if (snapshot == null){ //snapshot may have been removed; + if (snapshot == null) { //snapshot may have been removed; continue; } SnapshotObject info = SnapshotObject.getSnapshotObject(snapshot, store); @@ -107,8 +112,6 @@ public class SnapshotDataFactoryImpl implements SnapshotDataFactory { return infos; } - - @Override public SnapshotInfo getSnapshot(long snapshotId, long storeId, DataStoreRole role) { SnapshotVO snapshot = snapshotDao.findById(snapshotId); @@ -202,4 +205,17 @@ public class SnapshotDataFactoryImpl implements SnapshotDataFactory { return snapObjs; } + @Override + public void updateOperationFailed(long snapshotId) throws NoTransitionException { + List snapshotStoreRefs = snapshotStoreDao.findBySnapshotId(snapshotId); + for (SnapshotDataStoreVO snapshotStoreRef : snapshotStoreRefs) { + SnapshotInfo snapshotInfo = getSnapshot(snapshotStoreRef.getSnapshotId(), snapshotStoreRef.getDataStoreId(), snapshotStoreRef.getRole()); + if (snapshotInfo != null) { + VolumeInfo volumeInfo = snapshotInfo.getBaseVolume(); + volumeInfo.stateTransit(Volume.Event.OperationFailed); + ((SnapshotObject)snapshotInfo).processEvent(Snapshot.Event.OperationFailed); + snapshotInfo.processEvent(ObjectInDataStoreStateMachine.Event.OperationFailed); + } + } + } } diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java index 09f569e6f19..1801c877893 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java @@ -481,4 +481,14 @@ public class DefaultVMSnapshotStrategy extends ManagerBase implements VMSnapshot } return StrategyPriority.DEFAULT; } + + @Override + public void updateOperationFailed(VMSnapshot vmSnapshot) throws NoTransitionException { + try { + vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed); + } catch (NoTransitionException e) { + logger.debug("Failed to change vm snapshot state with event OperationFailed"); + throw e; + } + } } diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java index 1ec6e20fc9e..8afbcab9b4b 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java @@ -479,6 +479,16 @@ public class ScaleIOVMSnapshotStrategy extends ManagerBase implements VMSnapshot return vmSnapshotDao.remove(vmSnapshot.getId()); } + @Override + public void updateOperationFailed(VMSnapshot vmSnapshot) throws NoTransitionException { + try { + vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed); + } catch (NoTransitionException e) { + logger.debug("Failed to change vm snapshot state with event OperationFailed"); + throw e; + } + } + private void publishUsageEvent(String type, VMSnapshot vmSnapshot, UserVm userVm, VolumeObjectTO volumeTo) { VolumeVO volume = volumeDao.findById(volumeTo.getId()); Long diskOfferingId = volume.getDiskOfferingId(); diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java index 47bf27bd6c4..80140b0d950 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java @@ -35,6 +35,11 @@ import java.util.concurrent.TimeUnit; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.storage.SnapshotVO; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotService; +import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.vm.snapshot.dao.VMSnapshotDao; import org.apache.cloudstack.api.ApiCommandResourceType; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.context.CallContext; @@ -153,11 +158,15 @@ public class AsyncJobManagerImpl extends ManagerBase implements AsyncJobManager, @Inject private SnapshotDao _snapshotDao; @Inject + private VMSnapshotDao _vmSnapshotDao; + @Inject private SnapshotService snapshotSrv; @Inject private SnapshotDataFactory snapshotFactory; @Inject private SnapshotDetailsDao _snapshotDetailsDao; + @Inject + private VMSnapshotService _vmSnapshotService; @Inject private VolumeDataFactory volFactory; @@ -1149,6 +1158,10 @@ public class AsyncJobManagerImpl extends ManagerBase implements AsyncJobManager, return cleanupVirtualMachine(job.getInstanceId()); case Network: return cleanupNetwork(job.getInstanceId()); + case Snapshot: + return cleanupSnapshot(job.getInstanceId()); + case VmSnapshot: + return cleanupVmSnapshot(job.getInstanceId()); } } catch (Exception e) { logger.warn("Error while cleaning up resource: [" + job.getInstanceType().toString() + "] with Id: " + job.getInstanceId(), e); @@ -1187,7 +1200,7 @@ public class AsyncJobManagerImpl extends ManagerBase implements AsyncJobManager, return true; } - private boolean cleanupNetwork(final long networkId) throws Exception { + private boolean cleanupNetwork(final long networkId) { NetworkVO networkVO = networkDao.findById(networkId); if (networkVO == null) { logger.warn("Network not found. Skip Cleanup. NetworkId: " + networkId); @@ -1206,6 +1219,46 @@ public class AsyncJobManagerImpl extends ManagerBase implements AsyncJobManager, return true; } + private boolean cleanupSnapshot(final long snapshotId) { + SnapshotVO snapshotVO = _snapshotDao.findById(snapshotId); + if (snapshotVO == null) { + logger.warn("Snapshot not found. Skip Cleanup. SnapshotId: " + snapshotId); + return true; + } + if (Snapshot.State.Allocated.equals(snapshotVO.getState())) { + _snapshotDao.remove(snapshotId); + } + if (Snapshot.State.Creating.equals(snapshotVO.getState())) { + try { + snapshotFactory.updateOperationFailed(snapshotId); + } catch (NoTransitionException e) { + snapshotVO.setState(Snapshot.State.Error); + _snapshotDao.update(snapshotVO.getId(), snapshotVO); + } + } + return true; + } + + private boolean cleanupVmSnapshot(final long vmSnapshotId) { + VMSnapshotVO vmSnapshotVO = _vmSnapshotDao.findById(vmSnapshotId); + if (vmSnapshotVO == null) { + logger.warn("VM Snapshot not found. Skip Cleanup. VMSnapshotId: " + vmSnapshotId); + return true; + } + if (VMSnapshot.State.Allocated.equals(vmSnapshotVO.getState())) { + _vmSnapshotDao.remove(vmSnapshotId); + } + if (VMSnapshot.State.Creating.equals(vmSnapshotVO.getState())) { + try { + _vmSnapshotService.updateOperationFailed(vmSnapshotVO); + } catch (NoTransitionException e) { + vmSnapshotVO.setState(VMSnapshot.State.Error); + _vmSnapshotDao.update(vmSnapshotVO.getId(), vmSnapshotVO); + } + } + return true; + } + private void cleanupFailedVolumesCreatedFromSnapshots(final long volumeId) { try { VolumeDetailVO volumeDetail = _volumeDetailsDao.findDetail(volumeId, VolumeService.SNAPSHOT_ID); diff --git a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java index 907182edc2a..26f7603240f 100644 --- a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java @@ -28,6 +28,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; import com.cloud.storage.snapshot.SnapshotManager; +import com.cloud.utils.fsm.NoTransitionException; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.ApiConstants; @@ -1387,6 +1388,12 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme return true; } + @Override + public void updateOperationFailed(VMSnapshot vmSnapshot) throws NoTransitionException { + VMSnapshotStrategy strategy = findVMSnapshotStrategy(vmSnapshot); + strategy.updateOperationFailed(vmSnapshot); + } + @Override public String getConfigComponentName() { return VMSnapshotManager.class.getSimpleName();