server: fix finding pools for volume migration (#4693)

While finding pools for volume migration list following compatible storages:
- all zone-wide storages of the same hypervisor.
- when the volume is attached to a VM, then all storages from the same cluster as that of VM.
- for detached volume, all storages that belong to clusters of the same hypervisor. 

Fixes #4692 
Fixes #4400

engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/StoragePoolAllocator.java

@@ -28,6 +28,12 @@ import com.cloud.vm.VirtualMachineProfile;
 /**
  */
 public interface StoragePoolAllocator extends Adapter {
+    /**
+     * Overloaded method calls allocateToPool with bypassStorageTypeCheck = false
+     * and returns a list of pools suitable.
+     **/
+    List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo);
+
     /**
      * Determines which storage pools are suitable for the guest virtual machine
      * and returns a list of pools suitable.
@@ -45,10 +51,12 @@ public interface StoragePoolAllocator extends Adapter {
      * @param ExcludeList
      *            avoid
      * @param int returnUpTo (use -1 to return all possible pools)
+     * @param boolean bypassStorageTypeCheck allows bypassing useLocalStorage check for provided DiskProfile when true
      * @return List<StoragePool> List of storage pools that are suitable for the
      *         VM
      **/
-    List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo);
+    List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck);
+
 
     static int RETURN_UPTO_ALL = -1;
 

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

@@ -122,6 +122,8 @@ public interface PrimaryDataStoreDao extends GenericDao<StoragePoolVO, Long> {
 
     List<StoragePoolVO> listLocalStoragePoolByPath(long datacenterId, String path);
 
+    List<StoragePoolVO> findPoolsInClusters(List<Long> clusterIds);
+
     void deletePoolTags(long poolId);
 
     List<StoragePoolVO> listChildStoragePoolsInDatastoreCluster(long poolId);

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

@@ -56,6 +56,7 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
     private final SearchBuilder<StoragePoolVO> DeleteLvmSearch;
     private final SearchBuilder<StoragePoolVO> DcLocalStorageSearch;
     private final GenericSearchBuilder<StoragePoolVO, Long> StatusCountSearch;
+    private final SearchBuilder<StoragePoolVO> ClustersSearch;
 
     @Inject
     private StoragePoolDetailsDao _detailsDao;
@@ -133,6 +134,10 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
         DcLocalStorageSearch.and("scope", DcLocalStorageSearch.entity().getScope(), SearchCriteria.Op.EQ);
         DcLocalStorageSearch.done();
 
+        ClustersSearch = createSearchBuilder();
+        ClustersSearch.and("clusterIds", ClustersSearch.entity().getClusterId(), Op.IN);
+        ClustersSearch.and("status", ClustersSearch.entity().getStatus(), Op.EQ);
+
     }
 
     @Override
@@ -568,4 +573,12 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
         sc.addAnd("removed", SearchCriteria.Op.NULL);
         return getCount(sc);
     }
+
+    @Override
+    public List<StoragePoolVO> findPoolsInClusters(List<Long> clusterIds) {
+        SearchCriteria<StoragePoolVO> sc = ClustersSearch.create();
+        sc.setParameters("clusterIds", clusterIds.toArray());
+        sc.setParameters("status", StoragePoolStatus.Up);
+        return listBy(sc);
+    }
 }

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java

@@ -26,15 +26,12 @@ import java.util.Map;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import com.cloud.exception.StorageUnavailableException;
-import com.cloud.storage.StoragePoolStatus;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
 import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.log4j.Logger;
 
 import com.cloud.capacity.Capacity;
 import com.cloud.capacity.dao.CapacityDao;
@@ -42,10 +39,12 @@ import com.cloud.dc.ClusterVO;
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.deploy.DeploymentPlan;
 import com.cloud.deploy.DeploymentPlanner.ExcludeList;
+import com.cloud.exception.StorageUnavailableException;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.storage.Storage;
 import com.cloud.storage.StorageManager;
 import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolStatus;
 import com.cloud.storage.StorageUtil;
 import com.cloud.storage.Volume;
 import com.cloud.storage.dao.VolumeDao;
@@ -87,11 +86,16 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement
         return false;
     }
 
-    protected abstract List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo);
+    protected abstract List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck);
 
     @Override
     public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
-        List<StoragePool> pools = select(dskCh, vmProfile, plan, avoid, returnUpTo);
+        return allocateToPool(dskCh, vmProfile, plan, avoid, returnUpTo, false);
+    }
+
+    @Override
+    public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck) {
+        List<StoragePool> pools = select(dskCh, vmProfile, plan, avoid, returnUpTo, bypassStorageTypeCheck);
         return reorderPools(pools, vmProfile, plan);
     }
 

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java

@@ -45,9 +45,13 @@ public class ClusterScopeStoragePoolAllocator extends AbstractStoragePoolAllocat
     DiskOfferingDao _diskOfferingDao;
 
     @Override
-    protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
+    protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck) {
         s_logger.debug("ClusterScopeStoragePoolAllocator looking for storage pool");
 
+        if (!bypassStorageTypeCheck && dskCh.useLocalStorage()) {
+            return null;
+        }
+
         List<StoragePool> suitablePools = new ArrayList<StoragePool>();
 
         long dcId = plan.getDataCenterId();

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/GarbageCollectingStoragePoolAllocator.java

@@ -47,7 +47,7 @@ public class GarbageCollectingStoragePoolAllocator extends AbstractStoragePoolAl
     boolean _storagePoolCleanupEnabled;
 
     @Override
-    public List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
+    public List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck) {
         s_logger.debug("GarbageCollectingStoragePoolAllocator looking for storage pool");
         if (!_storagePoolCleanupEnabled) {
             s_logger.debug("Storage pool cleanup is not enabled, so GarbageCollectingStoragePoolAllocator is being skipped.");
@@ -68,7 +68,7 @@ public class GarbageCollectingStoragePoolAllocator extends AbstractStoragePoolAl
         ExcludeList myAvoids =
             new ExcludeList(avoid.getDataCentersToAvoid(), avoid.getPodsToAvoid(), avoid.getClustersToAvoid(), avoid.getHostsToAvoid(), avoid.getPoolsToAvoid());
 
-        return allocator.allocateToPool(dskCh, vmProfile, plan, myAvoids, returnUpTo);
+        return allocator.allocateToPool(dskCh, vmProfile, plan, myAvoids, returnUpTo, bypassStorageTypeCheck);
     }
 
     @Override

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java

@@ -60,10 +60,10 @@ public class LocalStoragePoolAllocator extends AbstractStoragePoolAllocator {
     ConfigurationDao _configDao;
 
     @Override
-    protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
+    protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck) {
         s_logger.debug("LocalStoragePoolAllocator trying to find storage pool to fit the vm");
 
-        if (!dskCh.useLocalStorage()) {
+        if (!bypassStorageTypeCheck && !dskCh.useLocalStorage()) {
             return null;
         }
 

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/UseLocalForRootAllocator.java

@@ -39,12 +39,17 @@ public class UseLocalForRootAllocator extends LocalStoragePoolAllocator implemen
 
     @Override
     public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
+        return allocateToPool(dskCh, vmProfile, plan, avoid, returnUpTo, false);
+    }
+
+    @Override
+    public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck) {
         DataCenterVO dc = _dcDao.findById(plan.getDataCenterId());
         if (!dc.isLocalStorageEnabled()) {
             return null;
         }
 
-        return super.allocateToPool(dskCh, vmProfile, plan, avoid, returnUpTo);
+        return super.allocateToPool(dskCh, vmProfile, plan, avoid, returnUpTo, bypassStorageTypeCheck);
     }
 
     @Override

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java

@@ -50,10 +50,10 @@ public class ZoneWideStoragePoolAllocator extends AbstractStoragePoolAllocator {
 
 
     @Override
-    protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
+    protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck) {
         LOGGER.debug("ZoneWideStoragePoolAllocator to find storage pool");
 
-        if (dskCh.useLocalStorage()) {
+        if (!bypassStorageTypeCheck && dskCh.useLocalStorage()) {
             return null;
         }
 

plugins/storage-allocators/random/src/main/java/org/apache/cloudstack/storage/allocator/RandomStoragePoolAllocator.java

@@ -35,7 +35,7 @@ public class RandomStoragePoolAllocator extends AbstractStoragePoolAllocator {
     private static final Logger s_logger = Logger.getLogger(RandomStoragePoolAllocator.class);
 
     @Override
-    public List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
+    public List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck) {
 
         List<StoragePool> suitablePools = new ArrayList<StoragePool>();
 

server/src/main/java/com/cloud/server/ManagementServerImpl.java

@@ -27,6 +27,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TimeZone;
 import java.util.concurrent.Executors;
@@ -40,7 +41,6 @@ import javax.crypto.spec.SecretKeySpec;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import com.cloud.storage.Storage;
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.affinity.AffinityGroupProcessor;
 import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
@@ -573,6 +573,8 @@ import com.cloud.alert.AlertManager;
 import com.cloud.alert.AlertVO;
 import com.cloud.alert.dao.AlertDao;
 import com.cloud.api.ApiDBUtils;
+import com.cloud.api.query.dao.StoragePoolJoinDao;
+import com.cloud.api.query.vo.StoragePoolJoinVO;
 import com.cloud.capacity.Capacity;
 import com.cloud.capacity.CapacityVO;
 import com.cloud.capacity.dao.CapacityDao;
@@ -657,8 +659,10 @@ import com.cloud.storage.GuestOSHypervisorVO;
 import com.cloud.storage.GuestOSVO;
 import com.cloud.storage.GuestOsCategory;
 import com.cloud.storage.ScopeType;
+import com.cloud.storage.Storage;
 import com.cloud.storage.StorageManager;
 import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolStatus;
 import com.cloud.storage.Volume;
 import com.cloud.storage.VolumeApiServiceImpl;
 import com.cloud.storage.VolumeVO;
@@ -789,6 +793,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     @Inject
     private PrimaryDataStoreDao _poolDao;
     @Inject
+    private StoragePoolJoinDao _poolJoinDao;
+    @Inject
     private NetworkDao _networkDao;
     @Inject
     private StorageManager _storageMgr;
@@ -1145,7 +1151,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         return new Pair<List<? extends Cluster>, Integer>(result.first(), result.second());
     }
 
-    private HypervisorType getHypervisorType(VMInstanceVO vm, StoragePool srcVolumePool, VirtualMachineProfile profile) {
+    private HypervisorType getHypervisorType(VMInstanceVO vm, StoragePool srcVolumePool) {
         HypervisorType type = null;
         if (vm == null) {
             StoragePoolVO poolVo = _poolDao.findById(srcVolumePool.getId());
@@ -1155,18 +1161,13 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
                     ClusterVO cluster = _clusterDao.findById(clusterId);
                     type = cluster.getHypervisorType();
                 }
-            } else if (ScopeType.ZONE.equals(poolVo.getScope())) {
-                Long zoneId = poolVo.getDataCenterId();
-                if (zoneId != null) {
-                    DataCenterVO dc = _dcDao.findById(zoneId);
-                }
             }
 
             if (null == type) {
                 type = srcVolumePool.getHypervisor();
             }
         } else {
-            type = profile.getHypervisorType();
+            type = vm.getHypervisorType();
         }
         return type;
     }
@@ -1488,12 +1489,17 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         }
 
         StoragePool srcVolumePool = _poolDao.findById(volume.getPoolId());
-        allPools = getAllStoragePoolCompatileWithVolumeSourceStoragePool(srcVolumePool);
+        HypervisorType hypervisorType = getHypervisorType(vm, srcVolumePool);
+        Pair<Host, List<Cluster>> hostClusterPair = getVolumeVmHostClusters(srcVolumePool, vm, hypervisorType);
+        Host vmHost = hostClusterPair.first();
+        List<Cluster> clusters = hostClusterPair.second();
+        allPools = getAllStoragePoolCompatibleWithVolumeSourceStoragePool(srcVolumePool, hypervisorType, clusters);
         allPools.remove(srcVolumePool);
         if (vm != null) {
-            suitablePools = findAllSuitableStoragePoolsForVm(volume, vm, srcVolumePool);
+            suitablePools = findAllSuitableStoragePoolsForVm(volume, vm, vmHost, srcVolumePool,
+                    CollectionUtils.isNotEmpty(clusters) ? clusters.get(0) : null, hypervisorType);
         } else {
-            suitablePools = allPools;
+            suitablePools = findAllSuitableStoragePoolsForDetachedVolume(volume, allPools);
         }
         List<StoragePool> avoidPools = new ArrayList<>();
         if (srcVolumePool.getParent() != 0L) {
@@ -1520,6 +1526,30 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         }
     }
 
+    private Pair<Host, List<Cluster>> getVolumeVmHostClusters(StoragePool srcVolumePool, VirtualMachine vm, HypervisorType hypervisorType) {
+        Host host = null;
+        List<Cluster> clusters = new ArrayList<>();
+        Long clusterId = srcVolumePool.getClusterId();
+        if (vm != null) {
+            Long hostId = vm.getHostId();
+            if (hostId == null) {
+                hostId = vm.getLastHostId();
+            }
+            if (hostId != null) {
+                host = _hostDao.findById(hostId);
+            }
+        }
+        if (clusterId == null && host != null) {
+            clusterId = host.getClusterId();
+        }
+        if (clusterId != null && vm != null) {
+            clusters.add(_clusterDao.findById(clusterId));
+        } else {
+            clusters.addAll(_clusterDao.listByDcHyType(srcVolumePool.getDataCenterId(), hypervisorType.toString()));
+        }
+        return new Pair<>(host, clusters);
+    }
+
     /**
      * This method looks for all storage pools that are compatible with the given volume.
      * <ul>
@@ -1527,15 +1557,18 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
      *  <li>We also all storage available filtering by data center, pod and cluster as the current storage pool used by the given volume.</li>
      * </ul>
      */
-    private List<? extends StoragePool> getAllStoragePoolCompatileWithVolumeSourceStoragePool(StoragePool srcVolumePool) {
+    private List<? extends StoragePool> getAllStoragePoolCompatibleWithVolumeSourceStoragePool(StoragePool srcVolumePool, HypervisorType hypervisorType, List<Cluster> clusters) {
         List<StoragePoolVO> storagePools = new ArrayList<>();
-        List<StoragePoolVO> zoneWideStoragePools = _poolDao.findZoneWideStoragePoolsByTags(srcVolumePool.getDataCenterId(), null);
+        List<StoragePoolVO> zoneWideStoragePools = _poolDao.findZoneWideStoragePoolsByHypervisor(srcVolumePool.getDataCenterId(), hypervisorType);
         if (CollectionUtils.isNotEmpty(zoneWideStoragePools)) {
             storagePools.addAll(zoneWideStoragePools);
         }
-        List<StoragePoolVO> clusterAndLocalStoragePools = _poolDao.listBy(srcVolumePool.getDataCenterId(), srcVolumePool.getPodId(), srcVolumePool.getClusterId(), null);
+        if (CollectionUtils.isNotEmpty(clusters)) {
-        if (CollectionUtils.isNotEmpty(clusterAndLocalStoragePools)) {
+            List<Long> clusterIds = clusters.stream().map(Cluster::getId).collect(Collectors.toList());
-            storagePools.addAll(clusterAndLocalStoragePools);
+            List<StoragePoolVO> clusterAndLocalStoragePools = _poolDao.findPoolsInClusters(clusterIds);
+            if (CollectionUtils.isNotEmpty(clusterAndLocalStoragePools)) {
+                storagePools.addAll(clusterAndLocalStoragePools);
+            }
         }
         return storagePools;
     }
@@ -1547,35 +1580,33 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
      *
      *  Side note: the idea behind this method is to provide power for administrators of manually overriding deployments defined by CloudStack.
      */
-    private List<StoragePool> findAllSuitableStoragePoolsForVm(final VolumeVO volume, VMInstanceVO vm, StoragePool srcVolumePool) {
+    private List<StoragePool> findAllSuitableStoragePoolsForVm(final VolumeVO volume, VMInstanceVO vm, Host vmHost, StoragePool srcVolumePool, Cluster srcCluster, HypervisorType hypervisorType) {
         List<StoragePool> suitablePools = new ArrayList<>();
-
-        HostVO host = _hostDao.findById(vm.getHostId());
-        if (host == null) {
-            host = _hostDao.findById(vm.getLastHostId());
-        }
-
         ExcludeList avoid = new ExcludeList();
         avoid.addPool(srcVolumePool.getId());
-
+        Long clusterId = null;
-        DataCenterDeployment plan = new DataCenterDeployment(volume.getDataCenterId(), srcVolumePool.getPodId(), srcVolumePool.getClusterId(), null, null, null);
+        Long podId = null;
+        if (srcCluster != null) {
+            clusterId = srcCluster.getId();
+            podId = srcCluster.getPodId();
+        }
+        DataCenterDeployment plan = new DataCenterDeployment(volume.getDataCenterId(), podId, clusterId,
+                null, null, null, null);
         VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
         // OfflineVmwareMigration: vm might be null here; deal!
-        HypervisorType type = getHypervisorType(vm, srcVolumePool, profile);
 
         DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
-        //This is an override mechanism so we can list the possible local storage pools that a volume in a shared pool might be able to be migrated to
+        DiskProfile diskProfile = new DiskProfile(volume, diskOffering, hypervisorType);
-        DiskProfile diskProfile = new DiskProfile(volume, diskOffering, type);
-        diskProfile.setUseLocalStorage(true);
 
         for (StoragePoolAllocator allocator : _storagePoolAllocators) {
-            List<StoragePool> pools = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL);
+            List<StoragePool> pools = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL, true);
             if (CollectionUtils.isEmpty(pools)) {
                 continue;
             }
             for (StoragePool pool : pools) {
-                boolean isLocalPoolSameHostAsSourcePool = pool.isLocal() && StringUtils.equals(host.getPrivateIpAddress(), pool.getHostAddress());
+                boolean isLocalPoolSameHostAsVmHost = pool.isLocal() &&
-                if (isLocalPoolSameHostAsSourcePool || pool.isShared()) {
+                        (vmHost == null || StringUtils.equals(vmHost.getPrivateIpAddress(), pool.getHostAddress()));
+                if (isLocalPoolSameHostAsVmHost || pool.isShared()) {
                     suitablePools.add(pool);
                 }
 
@@ -1584,6 +1615,29 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         return suitablePools;
     }
 
+    private List<StoragePool> findAllSuitableStoragePoolsForDetachedVolume(Volume volume, List<? extends StoragePool> allPools) {
+        List<StoragePool> suitablePools = new ArrayList<>();
+        if (CollectionUtils.isEmpty(allPools)) {
+            return  suitablePools;
+        }
+        DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
+        List<String> tags = new ArrayList<>();
+        String[] tagsArray = diskOffering.getTagsArray();
+        if (tagsArray != null && tagsArray.length > 0) {
+            tags = Arrays.asList(tagsArray);
+        }
+        Long[] poolIds = allPools.stream().map(StoragePool::getId).toArray(Long[]::new);
+        List<StoragePoolJoinVO> pools = _poolJoinDao.searchByIds(poolIds);
+        for (StoragePoolJoinVO storagePool : pools) {
+            if (StoragePoolStatus.Up.equals(storagePool.getStatus()) &&
+                    (CollectionUtils.isEmpty(tags) || tags.contains(storagePool.getTag()))) {
+                Optional<? extends StoragePool> match = allPools.stream().filter(x -> x.getId() == storagePool.getId()).findFirst();
+                match.ifPresent(suitablePools::add);
+            }
+        }
+        return suitablePools;
+    }
+
     private Pair<List<HostVO>, Integer> searchForServers(final Long startIndex, final Long pageSize, final Object name, final Object type,
         final Object state, final Object zone, final Object pod, final Object cluster, final Object id, final Object keyword,
         final Object resourceState, final Object haHosts, final Object hypervisorType, final Object hypervisorVersion, final Object... excludes) {