From d775163586204a8de054aa62a3fe839ace2f8145 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 2 Apr 2021 09:26:41 +0200 Subject: [PATCH 1/3] protect against stray snapshot-details without snapshot --- .../cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java | 3 +++ .../cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java | 4 ++++ 2 files changed, 7 insertions(+) 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 f8be1adee565..42578850d543 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 @@ -87,6 +87,9 @@ public List getSnapshots(long volumeId, DataStoreRole role) { @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); 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 54ce065f9cd8..34f58933cd41 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 @@ -1097,6 +1097,10 @@ public void doInTransactionWithoutResult(TransactionStatus status) { final List 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); } From 120dd41c1d677adf314001c8fe1df8b610576a81 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Sun, 18 Apr 2021 14:59:38 +0200 Subject: [PATCH 2/3] extra null checks --- .../engine/orchestration/DataMigrationUtility.java | 4 +++- .../apache/cloudstack/storage/snapshot/SnapshotObject.java | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java index dd451f594654..7fad871462da 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java @@ -200,7 +200,9 @@ protected List getAllReadySnapshotsAndChains(DataStore srcDataStore, 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); + } } } diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java index f107343f0def..1d343ab22d2f 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java @@ -142,7 +142,10 @@ public List getChildren() { List 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; From bd3f6391c506fb54df89e3e169e578ceadc0cd94 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 19 Apr 2021 15:07:22 +0200 Subject: [PATCH 3/3] remove unused --- .../com/cloud/storage/snapshot/SnapshotManager.java | 3 --- .../cloud/storage/snapshot/SnapshotManagerImpl.java | 10 ---------- 2 files changed, 13 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java index c900b2d14ba1..012c2ab86414 100644 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java @@ -24,7 +24,6 @@ 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; } diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index f46367835ee0..43ceb8197854 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -412,16 +412,6 @@ public Snapshot archiveSnapshot(Long snapshotId) { 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);