added option in the UI for the storage pools

Added drop down to choose the primary storage pools to copy a snapshot
Small fixes

plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/collector/StorPoolAbandonObjectsCollector.java

@@ -41,7 +41,6 @@
 import org.apache.cloudstack.storage.datastore.util.StorPoolUtil;
 import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpApiResponse;
 import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpConnectionDesc;
-import org.apache.cloudstack.storage.snapshot.StorPoolSnapshotStrategy;
 import org.apache.commons.collections.CollectionUtils;
 
 import javax.inject.Inject;
@@ -379,12 +378,13 @@ protected void runInContext() {
                         }
                         if (snapshots.contains(name)) {
                             Long clusterId = StorPoolHelper.findClusterIdByGlobalId(StorPoolUtil.getSnapshotClusterId(name, conn), clusterDao);
-                            conn = StorPoolSnapshotStrategy.getSpConnectionDesc(conn, clusterId);
+                            conn = StorPoolHelper.getSpConnectionDesc(conn, clusterId);
                             SpApiResponse resp = StorPoolUtil.snapshotUnexport(name, location, conn);
                             if (resp.getError() == null) {
+                                StorPoolUtil.spLog("Unexport of snapshot %s was successful", name);
                                 recoveredSnapshots.add(snapshot.getId());
                             } else {
-                                logger.debug(String.format("Could not recover StorPool snapshot %s", resp.getError()));
+                                StorPoolUtil.spLog("Could not recover StorPool snapshot %s", resp.getError());
                             }
                         }
                     }

plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolHelper.java

@@ -305,4 +305,15 @@ public static boolean isPoolSupportsAllFunctionalityFromPreviousVersion(StorageP
         }
         return true;
     }
+
+    public static StorPoolUtil.SpConnectionDesc getSpConnectionDesc(StorPoolUtil.SpConnectionDesc connectionLocal, Long clusterId) {
+
+        String subClusterEndPoint = StorPoolConfigurationManager.StorPoolSubclusterEndpoint.valueIn(clusterId);
+        if (StringUtils.isNotEmpty(subClusterEndPoint)) {
+            String host = subClusterEndPoint.split(";")[0].split("=")[1];
+            String token = subClusterEndPoint.split(";")[1].split("=")[1];
+            connectionLocal = new StorPoolUtil.SpConnectionDesc(host, token, connectionLocal.getTemplateName());
+        }
+        return connectionLocal;
+    }
 }

plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java

@@ -665,7 +665,8 @@ public static SpApiResponse snapshotExport(String name, String location, SpConne
     public static SpApiResponse snapshotUnexport(String name, String location, SpConnectionDesc conn) {
         Map<String, Object> json = new HashMap<>();
         json.put("snapshot", name);
-        json.put("location", location);
+        json.put("force", true);
+        json.put("all", true);
         return POST("SnapshotUnexport", json, conn);
     }
 

plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java

@@ -57,7 +57,6 @@
 import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpConnectionDesc;
 import org.apache.cloudstack.storage.to.SnapshotObjectTO;
 import org.apache.commons.collections.CollectionUtils;
-import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -196,6 +195,12 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat
         if (CollectionUtils.isEmpty(pools)) {
             return StrategyPriority.CANT_HANDLE;
         }
+        List<SnapshotJoinVO> snapshots = snapshotJoinDao.listBySnapshotIdAndZoneId(zoneId, snapshot.getId());
+        boolean snapshotNotOnStorPool = snapshots.stream().filter(s -> s.getStoreRole().equals(DataStoreRole.Primary)).count() == 0;
+
+        if (snapshotNotOnStorPool) {
+            return StrategyPriority.CANT_HANDLE;
+        }
         for (StoragePoolVO pool : pools) {
             SnapshotDataStoreVO snapshotOnPrimary = _snapshotStoreDao.findByStoreSnapshot(DataStoreRole.Primary, pool.getId(), snapshot.getId());
             if (snapshotOnPrimary != null && (snapshotOnPrimary.getState().equals(State.Ready) || snapshotOnPrimary.getState().equals(State.Created))) {
@@ -391,7 +396,7 @@ public void copySnapshot(DataObject snapshot, DataObject snapshotDest, AsyncComp
                     snapshot.getDataStore().getId(), storagePoolDetailsDao, _primaryDataStoreDao);
             String snapshotName = StorPoolStorageAdaptor.getVolumeNameFromPath(srcSnapshot.getPath(), false);
             Long clusterId = StorPoolHelper.findClusterIdByGlobalId(StorPoolUtil.getSnapshotClusterId("~" + snapshotName, connectionLocal), clusterDao);
-            connectionLocal = getSpConnectionDesc(connectionLocal, clusterId);
+            connectionLocal = StorPoolHelper.getSpConnectionDesc(connectionLocal, clusterId);
             SpApiResponse resp = StorPoolUtil.snapshotExport("~" + snapshotName, location, connectionLocal);
             if (resp.getError() != null) {
                 StorPoolUtil.spLog("Failed to export snapshot %s from %s due to %s", snapshotName, location, resp.getError());
@@ -401,6 +406,9 @@ public void copySnapshot(DataObject snapshot, DataObject snapshotDest, AsyncComp
                 callback.complete(res);
                 return;
             }
+            String detail = "~" + snapshotName + ";" + location;
+            SnapshotDetailsVO snapshotForRecovery = new SnapshotDetailsVO(snapshot.getId(), StorPoolUtil.SP_RECOVERED_SNAPSHOT, detail, true);
+            _snapshotDetailsDao.persist(snapshotForRecovery);
             SpConnectionDesc connectionRemote = StorPoolUtil.getSpConnection(storagePoolVO.getUuid(),
                     storagePoolVO.getId(), storagePoolDetailsDao, _primaryDataStoreDao);
             String localLocation = StorPoolConfigurationManager.StorPoolClusterLocation
@@ -418,9 +426,7 @@ public void copySnapshot(DataObject snapshot, DataObject snapshotDest, AsyncComp
                 return;
             }
             StorPoolUtil.spLog("The snapshot [%s] was copied from remote", snapshotName);
-            String detail = "~" + snapshotName + ";" + location;
-            SnapshotDetailsVO snapshotForRecovery = new SnapshotDetailsVO(snapshot.getId(), StorPoolUtil.SP_RECOVERED_SNAPSHOT, detail, true);
-            _snapshotDetailsDao.persist(snapshotForRecovery);
+
             respFromRemote = StorPoolUtil.snapshotReconcile("~" + snapshotName, connectionRemote);
             if (respFromRemote.getError() != null) {
                 StorPoolUtil.spLog("Failed to reconcile snapshot %s from %s due to %s", snapshotName, location, respFromRemote.getError());
@@ -447,15 +453,4 @@ public void copySnapshot(DataObject snapshot, DataObject snapshotDest, AsyncComp
         res.setResult(err);
         callback.complete(res);
     }
-
-    public static SpConnectionDesc getSpConnectionDesc(SpConnectionDesc connectionLocal, Long clusterId) {
-
-        String subClusterEndPoint = StorPoolConfigurationManager.StorPoolSubclusterEndpoint.valueIn(clusterId);
-        if (StringUtils.isNotEmpty(subClusterEndPoint)) {
-            String host = subClusterEndPoint.split(";")[0].split("=")[1];
-            String token = subClusterEndPoint.split(";")[1].split("=")[1];
-            connectionLocal = new SpConnectionDesc(host, token, connectionLocal.getTemplateName());
-        }
-        return connectionLocal;
-    }
 }

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

@@ -3862,6 +3862,7 @@ private boolean canCopyOnPrimary(List<Long> poolIds, VolumeInfo volume, boolean
         } else {
             return false;
         }
+        snapshotHelper.checkIfThereAreMoreThanOnePoolInTheZone(poolIds);
         return true;
     }
 

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

@@ -953,6 +953,7 @@ protected void validatePolicyZones(List<Long> zoneIds, List<Long> poolIds, Volum
             }
         }
         if (hasPools) {
+            snapshotHelper.checkIfThereAreMoreThanOnePoolInTheZone(poolIds);
             for (Long poolId : poolIds) {
                 getCheckedDestinationStorageForSnapshotCopy(poolId, isRootAdminCaller);
             }
@@ -2027,6 +2028,7 @@ public Snapshot copySnapshot(CopySnapshotCmd cmd) throws StorageUnavailableExcep
             throw new InvalidParameterValueException(String.format("There is no snapshot ID: %s ready on image store", snapshot.getUuid()));
         }
         if (canCopyBetweenStoragePools) {
+            snapshotHelper.checkIfThereAreMoreThanOnePoolInTheZone(storagePoolIds);
             copySnapshotToPrimaryDifferentZone(storagePoolIds, snapshot);
         }
         List<String> failedZones = copySnapshotToZones(snapshot, srcSecStore, new ArrayList<>(dataCenterVOs.values()));

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

@@ -1683,15 +1683,15 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
                 if (kvmSnapshotOnlyInPrimaryStorage && keepOnPrimary) {
                     skipCopyToSecondary = true;
                 }
-                if (dataStoreRole == DataStoreRole.Image || !skipCopyToSecondary) {
+                if (dataStoreRole == DataStoreRole.Image) {
                     snapInfo = snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo, dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage);
                     _accountMgr.checkAccess(caller, null, true, snapInfo);
                     DataStore snapStore = snapInfo.getDataStore();
 
                     if (snapStore != null) {
                         store = snapStore; // pick snapshot image store to create template
                     }
-                } else if (skipCopyToSecondary) {
+                } else if (keepOnPrimary) {
                     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 " +

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

@@ -51,7 +51,9 @@
 import org.apache.logging.log4j.Logger;
 
 import javax.inject.Inject;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -289,4 +291,18 @@ protected Set<Long> getSnapshotIdsOnlyInPrimaryStorage(long volumeId) {
 
         return snapshotIdsOnlyInPrimaryStorage;
     }
+
+    public void checkIfThereAreMoreThanOnePoolInTheZone(List<Long> poolIds) {
+        List<Long> poolsInOneZone = new ArrayList<>();
+        for (Long poolId : poolIds) {
+            StoragePoolVO pool = primaryDataStoreDao.findById(poolId);
+            if (pool != null) {
+                poolsInOneZone.add(pool.getDataCenterId());
+            }
+        }
+        boolean moreThanOnePoolForZone = poolsInOneZone.stream().filter(itr -> Collections.frequency(poolsInOneZone, itr) > 1).count() > 1;
+        if (moreThanOnePoolForZone) {
+            throw new CloudRuntimeException("Cannot copy the snapshot on multiple storage pools in one zone");
+        }
+    }
 }

ui/public/locales/en.json

@@ -1949,6 +1949,7 @@
 "label.select.source.vcenter.datacenter": "Select the source VMware vCenter Datacenter",
 "label.select.tier": "Select Network Tier",
 "label.select.zones": "Select zones",
+"label.select.storagepools": "Select storage pools",
 "label.select.2fa.provider": "Select the provider",
 "label.selected.storage": "Selected storage",
 "label.self": "Mine",
@@ -2107,6 +2108,7 @@
 "label.storagemotionenabled": "Storage motion enabled",
 "label.storagepolicy": "Storage policy",
 "label.storagepool": "Storage pool",
+"label.storagepools": "Storage pools",
 "label.storagepool.tooltip": "Destination Storage Pool. Volume should be located in this Storage Pool",
 "label.storagetags": "Storage tags",
 "label.storagetype": "Storage type",

ui/src/views/storage/FormSchedule.vue

@@ -169,6 +169,32 @@
                 </a-select>
               </a-form-item>
             </a-col>
+            <a-col :md="24" :lg="24" v-if="resourceType === 'Volume'">
+              <a-form-item ref="storageids" name="storageids">
+                <template #label>
+                  <tooltip-label :title="$t('label.storagepools')" :tooltip="''"/>
+                </template>
+                <a-select
+                  id="storagepool-selection"
+                  v-model:value="form.storageids"
+                  mode="multiple"
+                  showSearch
+                  optionFilterProp="label"
+                  :filterOption="(input, option) => {
+                    return  option.label.toLowerCase().indexOf(input.toLowerCase()) >= 0
+                  }"
+                  :loading="storagePoolLoading"
+                  :placeholder="''">
+                  <a-select-option v-for="opt in this.storagePools" :key="opt.id" :label="opt.name || opt.description">
+                    <span>
+                      <resource-icon v-if="opt.icon" :image="opt.icon.base64image" size="1x" style="margin-right: 5px"/>
+                      <global-outlined v-else style="margin-right: 5px"/>
+                      {{ opt.name || opt.description }}
+                    </span>
+                  </a-select-option>
+                </a-select>
+              </a-form-item>
+            </a-col>
           </a-row>
           <a-divider/>
           <div class="tagsTitle">{{ $t('label.tags') }}</div>
@@ -272,7 +298,8 @@ export default {
       timeZoneMap: [],
       fetching: false,
       listDayOfWeek: ['sunday', 'monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday'],
-      zones: []
+      zones: [],
+      storagePools: []
     }
   },
   created () {
@@ -307,6 +334,7 @@ export default {
       })
       if (this.resourceType === 'Volume') {
         this.fetchZoneData()
+        this.fetchStoragePoolData()
       }
     },
     fetchZoneData () {
@@ -323,6 +351,20 @@ export default {
         this.zoneLoading = false
       })
     },
+    fetchStoragePoolData () {
+      const params = {}
+      params.showicon = true
+      this.storagePoolsLoading = true
+      api('listStoragePools', params).then(json => {
+        const listStoragePools = json.liststoragepoolsresponse.storagepool
+        if (listStoragePools) {
+          this.storagePools = listStoragePools
+          this.storagePools = this.storagePools.filter(pool => pool.storagecapabilities.CAN_COPY_SNAPSHOT_BETWEEN_ZONES && pool.zoneid !== this.resource.zoneid)
+        }
+      }).finally(() => {
+        this.storagePoolsLoading = false
+      })
+    },
     fetchTimeZone (value) {
       this.timeZoneMap = []
       this.fetching = true
@@ -422,6 +464,9 @@ export default {
         if (values.zoneids && values.zoneids.length > 0) {
           params.zoneids = values.zoneids.join()
         }
+        if (values.storageids && values.storageids.length > 0) {
+          params.storageids = values.storageids.join()
+        }
         switch (values.intervaltype) {
           case 'hourly':
             params.schedule = values.time

ui/src/views/storage/SnapshotZones.vue

@@ -137,10 +137,39 @@
               </a-select-option>
             </a-select>
           </a-form-item>
-
+          <a-form-item ref="storageid" name="storageid" :label="$t('label.storagepools')">
+            <a-select
+              id="storage-selection"
+              mode="multiple"
+              :placeholder="$t('label.select.storagepools')"
+              v-model:value="form.storageid"
+              showSearch
+              optionFilterProp="label"
+              :filterOption="(input, option) => {
+                return option.label.toLowerCase().indexOf(input.toLowerCase()) >= 0
+              }"
+              :loading="storagePoolLoading"
+              v-focus="true">
+              <a-select-option v-for="opt in storagePools" :key="opt.id" :label="opt.name || opt.description">
+                <div>
+                  <span v-if="opt.icon && opt.icon.base64image">
+                    <resource-icon :image="opt.icon.base64image" size="1x" style="margin-right: 5px"/>
+                  </span>
+                  <global-outlined v-else style="margin-right: 5px" />
+                  {{ opt.name || opt.description }}
+                </div>
+              </a-select-option>
+            </a-select>
+          </a-form-item>
           <div :span="24" class="action-button">
             <a-button @click="onCloseModal">{{ $t('label.cancel') }}</a-button>
-            <a-button type="primary" ref="submit" @click="handleCopySnapshotSubmit">{{ $t('label.ok') }}</a-button>
+            <a-button
+              type="primary"
+              ref="submit"
+              :disabled="isCopySnapshotSubmitDisabled"
+              @click="handleCopySnapshotSubmit">
+                {{ $t('label.ok') }}
+              </a-button>
           </div>
         </a-form>
       </a-spin>
@@ -238,6 +267,8 @@ export default {
       currentRecord: {},
       zones: [],
       zoneLoading: false,
+      storagePools: [],
+      storagePoolLoading: false,
       copyLoading: false,
       deleteLoading: false,
       showDeleteSnapshot: false,
@@ -297,12 +328,17 @@ export default {
       }
     }
   },
+  computed: {
+    isCopySnapshotSubmitDisabled () {
+      return this.form.storageid.length === 0 && this.form.zoneid.length === 0
+    }
+  },
   methods: {
     initForm () {
       this.formRef = ref()
       this.form = reactive({})
       this.rules = reactive({
-        zoneid: [{ type: 'array', required: true, message: this.$t('message.error.select') }]
+        zoneid: [{ type: 'array', required: false }]
       })
     },
     fetchData () {
@@ -488,10 +524,26 @@ export default {
         this.zoneLoading = false
       })
     },
+    fetchStoragePoolData () {
+      const params = {}
+      params.showicon = true
+      this.storagePoolsLoading = true
+      api('listStoragePools', params).then(json => {
+        const listStoragePools = json.liststoragepoolsresponse.storagepool
+        if (listStoragePools) {
+          this.storagePools = listStoragePools
+          this.storagePools = this.storagePools.filter(pool => pool.storagecapabilities.CAN_COPY_SNAPSHOT_BETWEEN_ZONES && pool.zoneid !== this.resource.zoneid)
+        }
+      }).finally(() => {
+        this.storagePoolsLoading = false
+      })
+    },
     showCopySnapshot (record) {
       this.currentRecord = record
       this.form.zoneid = []
+      this.form.storageid = []
       this.fetchZoneData()
+      this.fetchStoragePoolData()
       this.showCopyActionForm = true
     },
     onShowDeleteModal (record) {
@@ -520,7 +572,8 @@ export default {
         const params = {
           id: this.currentRecord.id,
           sourcezoneid: this.currentRecord.zoneid,
-          destzoneids: values.zoneid.join()
+          destzoneids: values.zoneid.join(),
+          storageids: values.storageid.join()
         }
         this.copyLoading = true
         api(this.copyApi, params).then(json => {