server: protect against stray snapshot-details without snapshot (#4924)

This PR makes sure no orphaned snapshot details are considered in the cleanup at startup job.
a real solution would be to implement some kind of cascading delete, but as the parent record is "only" marked as removed this would be a bit com

Co-authored-by: Daan Hoogland <dahn@onecht.net>

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java

@@ -200,7 +200,9 @@ public class DataMigrationUtility {
                     snapshotVO != null && snapshotVO.getHypervisorType() != Hypervisor.HypervisorType.Simulator
                     && snapshot.getParentSnapshotId() == 0 ) {
                 SnapshotInfo snap = snapshotFactory.getSnapshot(snapshotVO.getSnapshotId(), DataStoreRole.Image);
-                files.add(snap);
+                if (snap != null) {
+                    files.add(snap);
+                }
             }
         }
 

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java

@@ -87,6 +87,9 @@ public class SnapshotDataFactoryImpl implements SnapshotDataFactory {
     @Override
     public SnapshotInfo getSnapshot(long snapshotId, DataStoreRole role) {
         SnapshotVO snapshot = snapshotDao.findById(snapshotId);
+        if (snapshot == null) {
+            return null;
+        }
         SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findBySnapshot(snapshotId, role);
         if (snapshotStore == null) {
             snapshotStore = snapshotStoreDao.findByVolume(snapshot.getVolumeId(), role);

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java

@@ -142,7 +142,10 @@ public class SnapshotObject implements SnapshotInfo {
         List<SnapshotInfo> children = new ArrayList<>();
         if (vos != null) {
             for (SnapshotDataStoreVO vo : vos) {
-                children.add(snapshotFactory.getSnapshot(vo.getSnapshotId(), DataStoreRole.Image));
+                SnapshotInfo info = snapshotFactory.getSnapshot(vo.getSnapshotId(), DataStoreRole.Image);
+                if (info != null) {
+                    children.add(info);
+                }
             }
         }
         return children;

framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java

@@ -1097,6 +1097,10 @@ public class AsyncJobManagerImpl extends ManagerBase implements AsyncJobManager,
                     final List<SnapshotDetailsVO> snapshotList = _snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid), false);
                     for (final SnapshotDetailsVO snapshotDetailsVO : snapshotList) {
                         SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotDetailsVO.getResourceId(), DataStoreRole.Primary);
+                        if (snapshot == null) {
+                            _snapshotDetailsDao.remove(snapshotDetailsVO.getId());
+                            continue;
+                        }
                         snapshotSrv.processEventOnSnapshotObject(snapshot, Snapshot.Event.OperationFailed);
                         _snapshotDetailsDao.removeDetail(snapshotDetailsVO.getResourceId(), AsyncJob.Constants.MS_ID);
                     }

server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java

@@ -24,7 +24,6 @@ import org.apache.cloudstack.framework.config.Configurable;
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.Command;
 import com.cloud.exception.ResourceAllocationException;
-import com.cloud.storage.Snapshot;
 import com.cloud.storage.SnapshotVO;
 import com.cloud.storage.Volume;
 
@@ -83,7 +82,5 @@ public interface SnapshotManager extends Configurable {
 
     SnapshotVO getParentSnapshot(VolumeInfo volume);
 
-    Snapshot backupSnapshot(Long snapshotId);
-
     SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationException;
 }

server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java

@@ -420,16 +420,6 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
         return snapshotOnSecondary;
     }
 
-    @Override
-    public Snapshot backupSnapshot(Long snapshotId) {
-        SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId, DataStoreRole.Image);
-        if (snapshot != null) {
-            throw new CloudRuntimeException("Already in the backup snapshot:" + snapshotId);
-        }
-
-        return snapshotSrv.backupSnapshot(snapshot);
-    }
-
     @Override
     public Snapshot backupSnapshotFromVmSnapshot(Long snapshotId, Long vmId, Long volumeId, Long vmSnapshotId) {
         VMInstanceVO vm = _vmDao.findById(vmId);