Fix snapshot chaining on Xen (#12597)

Author: JoaoJandre

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java

@@ -51,6 +51,8 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
 
     SnapshotDataStoreVO findBySnapshotIdAndDataStoreRoleAndState(long snapshotId, DataStoreRole role, ObjectInDataStoreStateMachine.State state);
 
+    List<SnapshotDataStoreVO> listBySnapshotIdAndDataStoreRoleAndStateIn(long snapshotId, DataStoreRole role, ObjectInDataStoreStateMachine.State... state);
+
     List<SnapshotDataStoreVO> listReadyByVolumeIdAndCheckpointPathNotNull(long volumeId);
 
     SnapshotDataStoreVO findOneBySnapshotId(long snapshotId, long zoneId);

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java

@@ -68,6 +68,7 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
     protected SearchBuilder<SnapshotDataStoreVO> searchFilteringStoreIdEqStateEqStoreRoleEqIdEqUpdateCountEqSnapshotIdEqVolumeIdEq;
     private SearchBuilder<SnapshotDataStoreVO> stateSearch;
     private SearchBuilder<SnapshotDataStoreVO> idStateNinSearch;
+    private SearchBuilder<SnapshotDataStoreVO> idEqRoleEqStateInSearch;
     protected SearchBuilder<SnapshotVO> snapshotVOSearch;
     private SearchBuilder<SnapshotDataStoreVO> snapshotCreatedSearch;
     private SearchBuilder<SnapshotDataStoreVO> dataStoreAndInstallPathSearch;
@@ -151,6 +152,11 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
         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 +393,15 @@ public SnapshotDataStoreVO findBySnapshotIdAndDataStoreRoleAndState(long snapsho
         return findOneBy(sc);
     }
 
+    @Override
+    public List<SnapshotDataStoreVO> listBySnapshotIdAndDataStoreRoleAndStateIn(long snapshotId, DataStoreRole role, State... state) {
+        SearchCriteria<SnapshotDataStoreVO> 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()) {

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<Snapshot.State> snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error, Snapshot.State.Hidden);
 
     public SnapshotDataStoreVO getSnapshotImageStoreRef(long snapshotId, long zoneId) {
-        List<SnapshotDataStoreVO> snaps = snapshotStoreDao.listReadyBySnapshot(snapshotId, DataStoreRole.Image);
+        List<SnapshotDataStoreVO> snaps = snapshotStoreDao.listBySnapshotIdAndDataStoreRoleAndStateIn(snapshotId, DataStoreRole.Image, State.Ready, State.Hidden);
         for (SnapshotDataStoreVO ref : snaps) {
             if (zoneId == dataStoreMgr.getStoreZoneId(ref.getDataStoreId(), ref.getRole())) {
                 return ref;

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.listBySnapshotIdAndDataStoreRoleAndStateIn(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));
     }