code improvements

Author: sureshanaparti

server/src/main/java/com/cloud/template/TemplateManagerImpl.java

@@ -1692,17 +1692,21 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
             if (store == null) {
                 throw new CloudRuntimeException("cannot find an image store for zone " + zoneId);
             }
-            AsyncCallFuture<TemplateApiResult> future = null;
+            AsyncCallFuture<TemplateApiResult> future;
 
             if (snapshotId != null) {
-                DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId);
-                kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId);
-
+                DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot);
+                kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole);
                 snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId);
-                boolean kvmIncrementalSnapshot = SnapshotManager.kvmIncrementalSnapshot.valueIn(_hostDao.findClusterIdByVolumeInfo(snapInfo.getBaseVolume()));
+                logger.debug("Creating template for snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage);
 
                 boolean storageSupportsSnapshotToTemplate = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo);
                 if (storageSupportsSnapshotToTemplate) {
+                    dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId);
+                    kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId);
+                    snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId);
+                    logger.debug("Creating template for snapshot for storage supporting snapshot to template, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage);
+
                     ImageStoreVO imageStore = _imgStoreDao.findOneByZoneAndProtocol(zoneId, "nfs");
                     if (imageStore == null) {
                         throw new CloudRuntimeException(String.format("Could not find an NFS secondary storage pool on zone %s to use as a temporary location " +
@@ -1721,6 +1725,8 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
                         store = snapStore; // pick snapshot image store to create template
                     }
                 }
+
+                boolean kvmIncrementalSnapshot = SnapshotManager.kvmIncrementalSnapshot.valueIn(_hostDao.findClusterIdByVolumeInfo(snapInfo.getBaseVolume()));
                 if (kvmIncrementalSnapshot && DataStoreRole.Image.equals(dataStoreRole)) {
                     snapInfo = snapshotHelper.convertSnapshotIfNeeded(snapInfo);
                 }

server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java

@@ -156,7 +156,8 @@ public void expungeTemporarySnapshot(boolean kvmSnapshotOnlyInPrimaryStorage, Sn
     public boolean isStorageSupportSnapshotToTemplate(SnapshotInfo snapInfo) {
         if (DataStoreRole.Primary.equals(snapInfo.getDataStore().getRole())) {
             Map<String, String> capabilities = snapInfo.getDataStore().getDriver().getCapabilities();
-            return org.apache.commons.collections4.MapUtils.isNotEmpty(capabilities) && capabilities.containsKey(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString());
+            return org.apache.commons.collections4.MapUtils.isNotEmpty(capabilities)
+                    && Boolean.parseBoolean(capabilities.get(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString()));
         }
         return false;
     }
@@ -214,36 +215,36 @@ protected boolean isSnapshotBackupable(SnapshotInfo snapInfo, DataStoreRole data
      * @return true if hypervisor is {@link  HypervisorType#KVM} and data store role is {@link  DataStoreRole#Primary} and global setting "snapshot.backup.to.secondary" is false,
      * else false.
      */
+    public boolean isKvmSnapshotOnlyInPrimaryStorage(Snapshot snapshot, DataStoreRole dataStoreRole) {
+        return snapshot.getHypervisorType() == Hypervisor.HypervisorType.KVM && dataStoreRole == DataStoreRole.Primary && !backupSnapshotAfterTakingSnapshot;
+    }
+
     public boolean isKvmSnapshotOnlyInPrimaryStorage(Snapshot snapshot, DataStoreRole dataStoreRole, Long zoneId){
         List<SnapshotJoinVO> snapshots = snapshotJoinDao.listBySnapshotIdAndZoneId(zoneId, snapshot.getSnapshotId());
-        boolean isKvmSnapshotOnlyInPrimaryStorage = snapshots.stream().filter(s -> s.getStoreRole().equals(DataStoreRole.Image)).count() == 0;
+        boolean isKvmSnapshotOnlyInPrimaryStorage = snapshots.stream().noneMatch(s -> s.getStoreRole().equals(DataStoreRole.Image));
 
         return snapshot.getHypervisorType() == Hypervisor.HypervisorType.KVM && dataStoreRole == DataStoreRole.Primary && isKvmSnapshotOnlyInPrimaryStorage;
     }
 
     public DataStoreRole getDataStoreRole(Snapshot snapshot) {
         SnapshotDataStoreVO snapshotStore = snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(snapshot.getId(), DataStoreRole.Primary);
-
         if (snapshotStore == null) {
             return DataStoreRole.Image;
         }
 
         long storagePoolId = snapshotStore.getDataStoreId();
-
         StoragePoolVO storagePoolVO = primaryDataStoreDao.findById(storagePoolId);
         if ((storagePoolTypesToValidateWithBackupSnapshotAfterTakingSnapshot.contains(storagePoolVO.getPoolType()) || snapshot.getHypervisorType() == HypervisorType.KVM)
                 && !backupSnapshotAfterTakingSnapshot) {
             return DataStoreRole.Primary;
         }
 
         DataStore dataStore = dataStorageManager.getDataStore(storagePoolId, DataStoreRole.Primary);
-
         if (dataStore == null) {
             return DataStoreRole.Image;
         }
 
         Map<String, String> mapCapabilities = dataStore.getDriver().getCapabilities();
-
         if (MapUtils.isNotEmpty(mapCapabilities) && BooleanUtils.toBoolean(mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()))) {
             return DataStoreRole.Primary;
         }
@@ -255,11 +256,13 @@ public DataStoreRole getDataStoreRole(Snapshot snapshot, Long zoneId) {
         if (zoneId == null) {
             getDataStoreRole(snapshot);
         }
+
         List<SnapshotJoinVO> snapshots = snapshotJoinDao.listBySnapshotIdAndZoneId(zoneId, snapshot.getId());
         boolean snapshotOnPrimary = snapshots.stream().anyMatch(s -> s.getStoreRole().equals(DataStoreRole.Primary));
         if (snapshotOnPrimary) {
             return DataStoreRole.Primary;
         }
+
         return DataStoreRole.Image;
     }
         /**