From 5db5dd9bacc587746a8db1ddbc5e60e2dd6f735f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Thu, 5 Feb 2026 09:26:43 -0300 Subject: [PATCH 1/2] fix snapshot chain and merge regression --- .../datastore/db/SnapshotDataStoreDao.java | 2 ++ .../db/SnapshotDataStoreDaoImpl.java | 24 ++++++++++++++++++- .../snapshot/DefaultSnapshotStrategy.java | 2 +- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java index 6aeee1ad1ccd..bd52c60be8e2 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java @@ -51,6 +51,8 @@ public interface SnapshotDataStoreDao extends GenericDao findBySnapshotIdAndDataStoreRoleAndStateIn(long snapshotId, DataStoreRole role, ObjectInDataStoreStateMachine.State... state); + List listReadyByVolumeIdAndCheckpointPathNotNull(long volumeId); SnapshotDataStoreVO findOneBySnapshotId(long snapshotId, long zoneId); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java index cdf903407c17..bfee0832ef8a 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java @@ -68,6 +68,9 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase searchFilteringStoreIdEqStateEqStoreRoleEqIdEqUpdateCountEqSnapshotIdEqVolumeIdEq; private SearchBuilder stateSearch; private SearchBuilder idStateNeqSearch; + + private SearchBuilder idStateNinSearch; + private SearchBuilder idEqRoleEqStateInSearch; protected SearchBuilder snapshotVOSearch; private SearchBuilder snapshotCreatedSearch; private SearchBuilder dataStoreAndInstallPathSearch; @@ -151,6 +154,16 @@ public boolean configure(String name, Map params) throws Configu idStateNeqSearch.and(STATE, idStateNeqSearch.entity().getState(), SearchCriteria.Op.NEQ); idStateNeqSearch.done(); + idStateNinSearch = createSearchBuilder(); + idStateNinSearch.and(SNAPSHOT_ID, idStateNinSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ); + idStateNinSearch.and(STATE, idStateNinSearch.entity().getState(), SearchCriteria.Op.NOTIN); + idStateNinSearch.done(); + + idEqRoleEqStateInSearch = createSearchBuilder(); + idEqRoleEqStateInSearch.and(SNAPSHOT_ID, idEqRoleEqStateInSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ); + idEqRoleEqStateInSearch.and(STORE_ROLE, idEqRoleEqStateInSearch.entity().getRole(), SearchCriteria.Op.EQ); + idEqRoleEqStateInSearch.and(STATE, idEqRoleEqStateInSearch.entity().getState(), SearchCriteria.Op.IN); + snapshotVOSearch = snapshotDao.createSearchBuilder(); snapshotVOSearch.and(VOLUME_ID, snapshotVOSearch.entity().getVolumeId(), SearchCriteria.Op.EQ); snapshotVOSearch.done(); @@ -387,6 +400,15 @@ public SnapshotDataStoreVO findBySnapshotIdAndDataStoreRoleAndState(long snapsho return findOneBy(sc); } + @Override + public List findBySnapshotIdAndDataStoreRoleAndStateIn(long snapshotId, DataStoreRole role, State... state) { + SearchCriteria sc = idEqRoleEqStateInSearch.create(); + sc.setParameters(SNAPSHOT_ID, snapshotId); + sc.setParameters(STORE_ROLE, role); + sc.setParameters(STATE, (Object[])state); + return listBy(sc); + } + @Override public SnapshotDataStoreVO findOneBySnapshotId(long snapshotId, long zoneId) { try (TransactionLegacy transactionLegacy = TransactionLegacy.currentTxn()) { @@ -488,7 +510,7 @@ public List findBySnapshotIdWithNonDestroyedState(long snap @Override public List findBySnapshotIdAndNotInDestroyedHiddenState(long snapshotId) { - SearchCriteria sc = idStateNeqSearch.create(); + SearchCriteria sc = idStateNinSearch.create(); sc.setParameters(SNAPSHOT_ID, snapshotId); sc.setParameters(STATE, State.Destroyed.name(), State.Hidden.name()); return listBy(sc); diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java index 0e38df5a2b15..6dedf9883265 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java @@ -120,7 +120,7 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase { private final List snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error, Snapshot.State.Hidden); public SnapshotDataStoreVO getSnapshotImageStoreRef(long snapshotId, long zoneId) { - List snaps = snapshotStoreDao.listReadyBySnapshot(snapshotId, DataStoreRole.Image); + List snaps = snapshotStoreDao.findBySnapshotIdAndDataStoreRoleAndStateIn(snapshotId, DataStoreRole.Image, State.Ready, State.Hidden); for (SnapshotDataStoreVO ref : snaps) { if (zoneId == dataStoreMgr.getStoreZoneId(ref.getDataStoreId(), ref.getRole())) { return ref; From 1e1e7a999c56a2c888b99cb6d068e9143c062325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Thu, 5 Feb 2026 10:16:23 -0300 Subject: [PATCH 2/2] fix tests --- .../storage/snapshot/DefaultSnapshotStrategyTest.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategyTest.java index 53f98c18f1be..b9c6ae99da1a 100644 --- a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategyTest.java +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategyTest.java @@ -257,11 +257,6 @@ public void verifyIfTheSnapshotIsBeingUsedByAnyVolumeTestDetailsIsNotEmptyThrowC @Test public void testGetSnapshotImageStoreRefNull() { - SnapshotDataStoreVO ref1 = Mockito.mock(SnapshotDataStoreVO.class); - Mockito.when(ref1.getDataStoreId()).thenReturn(1L); - Mockito.when(ref1.getRole()).thenReturn(DataStoreRole.Image); - Mockito.when(snapshotDataStoreDao.listReadyBySnapshot(Mockito.anyLong(), Mockito.any(DataStoreRole.class))).thenReturn(List.of(ref1)); - Mockito.when(dataStoreManager.getStoreZoneId(1L, DataStoreRole.Image)).thenReturn(2L); Assert.assertNull(defaultSnapshotStrategySpy.getSnapshotImageStoreRef(1L, 1L)); } @@ -270,7 +265,7 @@ public void testGetSnapshotImageStoreRefNotNull() { SnapshotDataStoreVO ref1 = Mockito.mock(SnapshotDataStoreVO.class); Mockito.when(ref1.getDataStoreId()).thenReturn(1L); Mockito.when(ref1.getRole()).thenReturn(DataStoreRole.Image); - Mockito.when(snapshotDataStoreDao.listReadyBySnapshot(Mockito.anyLong(), Mockito.any(DataStoreRole.class))).thenReturn(List.of(ref1)); + Mockito.when(snapshotDataStoreDao.findBySnapshotIdAndDataStoreRoleAndStateIn(Mockito.anyLong(), Mockito.any(DataStoreRole.class), Mockito.any(), Mockito.any())).thenReturn(List.of(ref1)); Mockito.when(dataStoreManager.getStoreZoneId(1L, DataStoreRole.Image)).thenReturn(1L); Assert.assertNotNull(defaultSnapshotStrategySpy.getSnapshotImageStoreRef(1L, 1L)); }