fix async backup for clvm type pools

Author: Pearl Dsilva

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

@@ -165,6 +165,7 @@
 import com.cloud.utils.DateUtil.IntervalType;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.Pair;
+import com.cloud.storage.clvm.ClvmPoolManager;
 import com.cloud.utils.Ternary;
 import com.cloud.utils.concurrency.NamedThreadFactory;
 import com.cloud.utils.db.DB;
@@ -1661,7 +1662,7 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
             if (backupSnapToSecondary) {
                 if (!isKvmAndFileBasedStorage) {
                     backupSnapshotToSecondary(payload.getAsyncBackup(), snapshotStrategy, snapshotOnPrimary, payload.getZoneIds(), payload.getStoragePoolIds());
-                    if (storagePool.getPoolType() == StoragePoolType.CLVM || storagePool.getPoolType() == StoragePoolType.CLVM_NG) {
+                    if (!payload.getAsyncBackup() && (storagePool.getPoolType() == StoragePoolType.CLVM || storagePool.getPoolType() == StoragePoolType.CLVM_NG)) {
                         _snapshotStoreDao.removeBySnapshotStore(snapshotId, snapshotOnPrimary.getDataStore().getId(), snapshotOnPrimary.getDataStore().getRole());
                     }
                 } else {
@@ -1683,7 +1684,12 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
                 postCreateSnapshot(volume.getId(), snapshotId, payload.getSnapshotPolicyId(), clusterId);
                 snapshotZoneDao.addSnapshotToZone(snapshotId, snapshot.getDataCenterId());
 
-                DataStoreRole dataStoreRole = backupSnapToSecondary ? snapshotHelper.getDataStoreRole(snapshot) : DataStoreRole.Primary;
+                DataStoreRole dataStoreRole;
+                if (payload.getAsyncBackup() && backupSnapToSecondary && !isKvmAndFileBasedStorage) {
+                    dataStoreRole = DataStoreRole.Primary;
+                } else {
+                    dataStoreRole = backupSnapToSecondary ? snapshotHelper.getDataStoreRole(snapshot) : DataStoreRole.Primary;
+                }
 
                 List<SnapshotDataStoreVO> snapshotStoreRefs = _snapshotStoreDao.listReadyBySnapshot(snapshotId, dataStoreRole);
                 if (CollectionUtils.isEmpty(snapshotStoreRefs)) {
@@ -1821,6 +1827,7 @@ protected void runInContext() {
                     SnapshotInfo backupedSnapshot = snapshotStrategy.backupSnapshot(snapshotOnPrimary);
                     if (backupedSnapshot != null) {
                         snapshotStrategy.postSnapshotCreation(snapshotOnPrimary);
+                        removeClvmPrimarySnapshotStoreRefIfNeeded(snapshotOnPrimary);
                         copyNewSnapshotToZones(snapshotOnPrimary.getId(), snapshotOnPrimary.getDataCenterId(), zoneIds);
                     }
                 }
@@ -1846,6 +1853,14 @@ private void decriseBackupSnapshotAttempts() {
         }
     }
 
+    void removeClvmPrimarySnapshotStoreRefIfNeeded(SnapshotInfo snapshotOnPrimary) {
+        DataStore dataStore = snapshotOnPrimary.getDataStore();
+        StoragePoolVO pool = _storagePoolDao.findById(dataStore.getId());
+        if (pool != null && ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
+            _snapshotStoreDao.removeBySnapshotStore(snapshotOnPrimary.getId(), dataStore.getId(), dataStore.getRole());
+        }
+    }
+
     private void updateSnapshotPayload(long storagePoolId, CreateSnapshotPayload payload, boolean isKvmAndFileBasedStorage, StoragePoolType poolType, Long clusterId) {
         StoragePoolVO storagePoolVO = _storagePoolDao.findById(storagePoolId);
 

server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java

@@ -26,6 +26,7 @@
 import com.cloud.org.Grouping;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.Snapshot;
+import com.cloud.storage.Storage.StoragePoolType;
 import com.cloud.storage.SnapshotPolicyVO;
 import com.cloud.storage.SnapshotVO;
 import com.cloud.storage.VolumeVO;
@@ -55,8 +56,10 @@
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
 import org.apache.cloudstack.framework.async.AsyncCallFuture;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 
 import org.junit.Assert;
 import org.junit.After;
@@ -105,6 +108,8 @@ public class SnapshotManagerImplTest {
     SnapshotScheduler snapshotScheduler;
     @Mock
     TaggedResourceService taggedResourceService;
+    @Mock
+    PrimaryDataStoreDao primaryDataStoreDao;
     @InjectMocks
     SnapshotManagerImpl snapshotManager = new SnapshotManagerImpl();
 
@@ -115,6 +120,7 @@ public void setUp() {
         snapshotManager._accountMgr = accountManager;
         snapshotManager._snapSchedMgr = snapshotScheduler;
         snapshotManager.taggedResourceService = taggedResourceService;
+        snapshotManager._storagePoolDao = primaryDataStoreDao;
     }
 
     @After
@@ -610,4 +616,57 @@ public void testDeleteSnapshotPoliciesManualPolicyId() {
 
         snapshotManager.deleteSnapshotPolicies(cmd);
     }
+
+    @Test
+    public void testRemoveClvmPrimarySnapshotStoreRefIfNeeded_ClvmPool() {
+        SnapshotInfo snapshot = Mockito.mock(SnapshotInfo.class);
+        DataStore dataStore = Mockito.mock(DataStore.class);
+        StoragePoolVO pool = Mockito.mock(StoragePoolVO.class);
+
+        Mockito.when(snapshot.getDataStore()).thenReturn(dataStore);
+        Mockito.when(snapshot.getId()).thenReturn(1L);
+        Mockito.when(dataStore.getId()).thenReturn(2L);
+        Mockito.when(dataStore.getRole()).thenReturn(DataStoreRole.Primary);
+        Mockito.when(primaryDataStoreDao.findById(2L)).thenReturn(pool);
+        Mockito.when(pool.getPoolType()).thenReturn(StoragePoolType.CLVM);
+
+        snapshotManager.removeClvmPrimarySnapshotStoreRefIfNeeded(snapshot);
+
+        Mockito.verify(snapshotStoreDao).removeBySnapshotStore(1L, 2L, DataStoreRole.Primary);
+    }
+
+    @Test
+    public void testRemoveClvmPrimarySnapshotStoreRefIfNeeded_ClvmNgPool() {
+        SnapshotInfo snapshot = Mockito.mock(SnapshotInfo.class);
+        DataStore dataStore = Mockito.mock(DataStore.class);
+        StoragePoolVO pool = Mockito.mock(StoragePoolVO.class);
+
+        Mockito.when(snapshot.getDataStore()).thenReturn(dataStore);
+        Mockito.when(snapshot.getId()).thenReturn(3L);
+        Mockito.when(dataStore.getId()).thenReturn(4L);
+        Mockito.when(dataStore.getRole()).thenReturn(DataStoreRole.Primary);
+        Mockito.when(primaryDataStoreDao.findById(4L)).thenReturn(pool);
+        Mockito.when(pool.getPoolType()).thenReturn(StoragePoolType.CLVM_NG);
+
+        snapshotManager.removeClvmPrimarySnapshotStoreRefIfNeeded(snapshot);
+
+        Mockito.verify(snapshotStoreDao).removeBySnapshotStore(3L, 4L, DataStoreRole.Primary);
+    }
+
+    @Test
+    public void testRemoveClvmPrimarySnapshotStoreRefIfNeeded_NonClvmPool() {
+        SnapshotInfo snapshot = Mockito.mock(SnapshotInfo.class);
+        DataStore dataStore = Mockito.mock(DataStore.class);
+        StoragePoolVO pool = Mockito.mock(StoragePoolVO.class);
+
+        Mockito.when(snapshot.getDataStore()).thenReturn(dataStore);
+        Mockito.when(dataStore.getId()).thenReturn(5L);
+        Mockito.when(primaryDataStoreDao.findById(5L)).thenReturn(pool);
+        Mockito.when(pool.getPoolType()).thenReturn(StoragePoolType.NetworkFilesystem);
+
+        snapshotManager.removeClvmPrimarySnapshotStoreRefIfNeeded(snapshot);
+
+        Mockito.verify(snapshotStoreDao, Mockito.never()).removeBySnapshotStore(
+                Mockito.anyLong(), Mockito.anyLong(), Mockito.any());
+    }
 }