[CLOUDSTACK-10240] ACS cannot migrate a local volume to shared storage (#2425)

* [CLOUDSTACK-10240] ACS cannot migrate a volume from local to shared storage.

CloudStack is logically restricting the migration of local storages to shared storage and vice versa. This restriction is a logical one and can be removed for XenServer deployments. Therefore, we will enable migration of volumes between local-shared storages in XenServers independently of their service offering. This will work as an override mechanism to the disk offering used by volumes. If administrators want to migrate local volumes to a shared storage, they should be able to do so (the hypervisor already allows that). The same the other way around.

* Cleanups implemented while working on [CLOUDSTACK-10240]

* Fix test case test_03_migrate_options_storage_tags

The changes applied were:
- When loading hypervisors capabilities we must use "default" instead of nulls
- "Enable" storage migration for simulator hypervisor
- Remove restriction on "ClusterScopeStoragePoolAllocator" to find shared pools

.travis.yml

@@ -71,8 +71,9 @@ env:
              smoke/test_portable_publicip
              smoke/test_portforwardingrules"
     
-    - TESTS="smoke/test_primary_storage
+    - TESTS="smoke/test_primary_storage"
-             smoke/test_privategw_acl
+    
+    - TESTS="smoke/test_privategw_acl
              smoke/test_projects
              smoke/test_public_ip_range
              smoke/test_pvlan

api/src/main/java/org/apache/cloudstack/api/command/admin/storage/FindStoragePoolsForMigrationCmd.java

@@ -17,10 +17,10 @@
 package org.apache.cloudstack.api.command.admin.storage;
 
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiCommandJobType;
 import org.apache.cloudstack.api.ApiConstants;
@@ -29,6 +29,8 @@ import org.apache.cloudstack.api.Parameter;
 import org.apache.cloudstack.api.response.ListResponse;
 import org.apache.cloudstack.api.response.StoragePoolResponse;
 import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.log4j.Logger;
 
 import com.cloud.storage.StoragePool;
 import com.cloud.utils.Pair;
@@ -81,7 +83,7 @@ public class FindStoragePoolsForMigrationCmd extends BaseListCmd {
             StoragePoolResponse poolResponse = _responseGenerator.createStoragePoolForMigrationResponse(pool);
             Boolean suitableForMigration = false;
             for (StoragePool suitablePool : suitablePoolList) {
-                if (suitablePool.getId() == pool.getId()) {
+                if (StringUtils.equals(suitablePool.getUuid(), pool.getUuid())) {
                     suitableForMigration = true;
                     break;
                 }
@@ -90,9 +92,27 @@ public class FindStoragePoolsForMigrationCmd extends BaseListCmd {
             poolResponse.setObjectName("storagepool");
             poolResponses.add(poolResponse);
         }
-
+        sortPoolsBySuitabilityAndName(poolResponses);
         response.setResponses(poolResponses);
         response.setResponseName(getCommandName());
         this.setResponseObject(response);
     }
+
+    protected void sortPoolsBySuitabilityAndName(List<StoragePoolResponse> poolResponses) {
+        Collections.sort(poolResponses, new Comparator<StoragePoolResponse>() {
+            @Override
+            public int compare(StoragePoolResponse o1, StoragePoolResponse o2) {
+                if (o1.getSuitableForMigration() && o2.getSuitableForMigration()) {
+                    return o1.getName().compareTo(o2.getName());
+                }
+                if (o1.getSuitableForMigration()) {
+                    return -1;
+                }
+                if (o2.getSuitableForMigration()) {
+                    return 1;
+                }
+                return 0;
+            }
+        });
+    }
 }

api/src/test/java/org/apache/cloudstack/api/command/admin/storage/FindStoragePoolsForMigrationCmdTest.java

@@ -0,0 +1,63 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.runners.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class FindStoragePoolsForMigrationCmdTest {
+
+    private FindStoragePoolsForMigrationCmd findStoragePoolsForMigrationCmd = new FindStoragePoolsForMigrationCmd();
+
+    @Test
+    public void sortPoolsBySuitability() {
+        List<StoragePoolResponse> storagePoolsResponse = new ArrayList<>();
+        StoragePoolResponse storagePoolResponse1 = new StoragePoolResponse();
+        storagePoolResponse1.setSuitableForMigration(true);
+        storagePoolResponse1.setId("1");
+        storagePoolResponse1.setName("1");
+
+        StoragePoolResponse storagePoolResponse2 = new StoragePoolResponse();
+        storagePoolResponse2.setSuitableForMigration(false);
+        storagePoolResponse2.setId("2");
+        storagePoolResponse2.setName("2");
+
+        StoragePoolResponse storagePoolResponse3 = new StoragePoolResponse();
+        storagePoolResponse3.setSuitableForMigration(true);
+        storagePoolResponse3.setId("3");
+        storagePoolResponse3.setName("3");
+
+        storagePoolsResponse.add(storagePoolResponse3);
+        storagePoolsResponse.add(storagePoolResponse2);
+        storagePoolsResponse.add(storagePoolResponse1);
+
+        findStoragePoolsForMigrationCmd.sortPoolsBySuitabilityAndName(storagePoolsResponse);
+
+        Assert.assertEquals("1", storagePoolsResponse.get(0).getId());
+        Assert.assertEquals("3", storagePoolsResponse.get(1).getId());
+        Assert.assertEquals("2", storagePoolsResponse.get(2).getId());
+
+    }
+
+}

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -23,9 +23,9 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -39,9 +39,6 @@ import java.util.concurrent.TimeUnit;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import org.apache.commons.collections.CollectionUtils;
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
 import org.apache.cloudstack.ca.CAManager;
 import org.apache.cloudstack.context.CallContext;
@@ -73,6 +70,9 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
 import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.collections.MapUtils;
+import org.apache.log4j.Logger;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.Listener;
@@ -424,8 +424,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
                 }
 
                 try {
-                    if (!vmProfile.getBootArgs().contains("ExternalLoadBalancerVm"))
+                    if (!vmProfile.getBootArgs().contains("ExternalLoadBalancerVm")) {
                         _networkMgr.allocate(vmProfile, auxiliaryNetworks, extraDhcpOptions);
+                    }
                 } catch (final ConcurrentOperationException e) {
                     throw new CloudRuntimeException("Concurrent operation while trying to allocate resources for the VM", e);
                 }
@@ -1880,6 +1881,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
         deleteVMSnapshots(vm, expunge);
 
         Transaction.execute(new TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {
+            @Override
             public void doInTransactionWithoutResult(final TransactionStatus status) throws CloudRuntimeException {
                 VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
                 try {
@@ -2292,97 +2294,119 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
         }
     }
 
-    private Map<Volume, StoragePool> getPoolListForVolumesForMigration(final VirtualMachineProfile profile, final Host host, final Map<Long, Long> volumeToPool) {
+    /**
-        final List<VolumeVO> allVolumes = _volsDao.findUsableVolumesForInstance(profile.getId());
+     * Create the mapping of volumes and storage pools. If the user did not enter a mapping on her/his own, we create one using {@link #getDefaultMappingOfVolumesAndStoragePoolForMigration(VirtualMachineProfile, Host)}.
-        final Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<>();
+     * If the user provided a mapping, we use whatever the user has provided (check the method {@link #createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile, Host, Map)}).
+     */
+    private Map<Volume, StoragePool> getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host targetHost, Map<Long, Long> volumeToPool) {
+        if (MapUtils.isEmpty(volumeToPool)) {
+            return getDefaultMappingOfVolumesAndStoragePoolForMigration(profile, targetHost);
+        }
 
-        for (final VolumeVO volume : allVolumes) {
+        return createMappingVolumeAndStoragePoolEnteredByUser(profile, targetHost, volumeToPool);
-            final Long poolId = volumeToPool.get(volume.getId());
+    }
-            final StoragePoolVO destPool = _storagePoolDao.findById(poolId);
-            final StoragePoolVO currentPool = _storagePoolDao.findById(volume.getPoolId());
-            final DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
 
-            if (destPool != null) {
+    /**
-                if (currentPool.isManaged()) {
+     * We create the mapping of volumes and storage pool to migrate the VMs according to the information sent by the user.
-                    if (destPool.getId() == currentPool.getId()) {
+     */
+    private Map<Volume, StoragePool> createMappingVolumeAndStoragePoolEnteredByUser(VirtualMachineProfile profile, Host host, Map<Long, Long> volumeToPool) {
+        Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<Volume, StoragePool>();
+        for(Long volumeId: volumeToPool.keySet()) {
+            VolumeVO volume = _volsDao.findById(volumeId);
+
+            Long poolId = volumeToPool.get(volumeId);
+            StoragePoolVO targetPool = _storagePoolDao.findById(poolId);
+            StoragePoolVO currentPool = _storagePoolDao.findById(volume.getPoolId());
+
+            if (_poolHostDao.findByPoolHost(targetPool.getId(), host.getId()) == null) {
+                throw new CloudRuntimeException(String.format("Cannot migrate the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host [%s]. The host does not have access to the storage pool entered.", volume.getUuid(), targetPool.getUuid(), profile.getUuid(), host.getUuid()));
+            }
+            if (currentPool.getId() == targetPool.getId()) {
+                s_logger.info(String.format("The volume [%s] is already allocated in storage pool [%s].", volume.getUuid(), targetPool.getUuid()));
+            }
+            volumeToPoolObjectMap.put(volume, targetPool);
+        }
+        return volumeToPoolObjectMap;
+    }
+
+    /**
+     * We create the default mapping of volumes and storage pools for the migration of the VM to the target host.
+     * If the current storage pool of one of the volumes is using local storage in the host, it then needs to be migrated to a local storage in the target host.
+     * Otherwise, we do not need to migrate, and the volume can be kept in its current storage pool.
+     */
+    private Map<Volume, StoragePool> getDefaultMappingOfVolumesAndStoragePoolForMigration(VirtualMachineProfile profile, Host targetHost) {
+        Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<Volume, StoragePool>();
+        List<VolumeVO> allVolumes = _volsDao.findUsableVolumesForInstance(profile.getId());
+        for (VolumeVO volume : allVolumes) {
+            StoragePoolVO currentPool = _storagePoolDao.findById(volume.getPoolId());
+            if (ScopeType.HOST.equals(currentPool.getScope())) {
+                createVolumeToStoragePoolMappingIfNeeded(profile, targetHost, volumeToPoolObjectMap, volume, currentPool);
+            } else {
                 volumeToPoolObjectMap.put(volume, currentPool);
             }
-                    else {
-                        throw new CloudRuntimeException("Currently, a volume on managed storage can only be 'migrated' to itself.");
-                    }
-                }
-                else {
-                    // Check if pool is accessible from the destination host and disk offering with which the volume was
-                    // created is compliant with the pool type.
-                    if (_poolHostDao.findByPoolHost(destPool.getId(), host.getId()) == null || destPool.isLocal() != diskOffering.getUseLocalStorage()) {
-                        // Cannot find a pool for the volume. Throw an exception.
-                        throw new CloudRuntimeException("Cannot migrate volume " + volume + " to storage pool " + destPool + " while migrating vm to host " + host +
-                                ". Either the pool is not accessible from the host or because of the offering with which the volume is created it cannot be placed on " +
-                                "the given pool.");
-                    } else if (destPool.getId() == currentPool.getId()) {
-                        // If the pool to migrate to is the same as current pool, the volume doesn't need to be migrated.
-                    } else {
-                        volumeToPoolObjectMap.put(volume, destPool);
-                    }
-                }
-            } else {
-                if (currentPool.isManaged()) {
-                    if (currentPool.getScope() == ScopeType.ZONE) {
-                        volumeToPoolObjectMap.put(volume, currentPool);
-                    }
-                    else {
-                        throw new CloudRuntimeException("Currently, you can only 'migrate' a volume on managed storage if its storage pool is zone wide.");
-                    }
-                } else {
-                    // Find a suitable pool for the volume. Call the storage pool allocator to find the list of pools.
-
-                    final DiskProfile diskProfile = new DiskProfile(volume, diskOffering, profile.getHypervisorType());
-                    final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(),
-                            host.getId(), null, null);
-
-                    final List<StoragePool> poolList = new ArrayList<>();
-                    final ExcludeList avoid = new ExcludeList();
-
-                    for (final StoragePoolAllocator allocator : _storagePoolAllocators) {
-                        final List<StoragePool> poolListFromAllocator = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL);
-
-                        if (poolListFromAllocator != null && !poolListFromAllocator.isEmpty()) {
-                            poolList.addAll(poolListFromAllocator);
         }
+        return volumeToPoolObjectMap;
     }
 
-                    boolean currentPoolAvailable = false;
+    /**
-
+     * We will add a mapping of volume to storage pool if needed. The conditions to add a mapping are the following:
-                    if (poolList != null && !poolList.isEmpty()) {
+     * <ul>
-                        // Volume needs to be migrated. Pick the first pool from the list. Add a mapping to migrate the
+     *  <li> The current storage pool where the volume is allocated can be accessed by the target host
-                        // volume to a pool only if it is required; that is the current pool on which the volume resides
+     *  <li> If not storage pool is found to allocate the volume we throw an exception.
-                        // is not available on the destination host.
+     * </ul>
-
+     *
-                        final Iterator<StoragePool> iter = poolList.iterator();
+     */
-
+    private void createVolumeToStoragePoolMappingIfNeeded(VirtualMachineProfile profile, Host targetHost, Map<Volume, StoragePool> volumeToPoolObjectMap, VolumeVO volume, StoragePoolVO currentPool) {
-                        while (iter.hasNext()) {
+        List<StoragePool> poolList = getCandidateStoragePoolsToMigrateLocalVolume(profile, targetHost, volume);
-                            if (currentPool.getId() == iter.next().getId()) {
-                                currentPoolAvailable = true;
 
+        Collections.shuffle(poolList);
+        boolean canTargetHostAccessVolumeStoragePool = false;
+        for (StoragePool storagePool : poolList) {
+            if (storagePool.getId() == currentPool.getId()) {
+                canTargetHostAccessVolumeStoragePool = true;
                 break;
             }
-                        }
 
-                        if (!currentPoolAvailable) {
+        }
+        if(!canTargetHostAccessVolumeStoragePool && CollectionUtils.isEmpty(poolList)) {
+            throw new CloudRuntimeException(String.format("There is not storage pools avaliable at the target host [%s] to migrate volume [%s]", targetHost.getUuid(), volume.getUuid()));
+        }
+        if (!canTargetHostAccessVolumeStoragePool) {
             volumeToPoolObjectMap.put(volume, _storagePoolDao.findByUuid(poolList.get(0).getUuid()));
         }
-                    }
+        if (!canTargetHostAccessVolumeStoragePool && !volumeToPoolObjectMap.containsKey(volume)) {
-
+            throw new CloudRuntimeException(String.format("Cannot find a storage pool which is available for volume [%s] while migrating virtual machine [%s] to host [%s]", volume.getUuid(),
-                    if (!currentPoolAvailable && !volumeToPoolObjectMap.containsKey(volume)) {
+                    profile.getUuid(), targetHost.getUuid()));
-                        // Cannot find a pool for the volume. Throw an exception.
-                        throw new CloudRuntimeException("Cannot find a storage pool which is available for volume " + volume + " while migrating virtual machine " +
-                                profile.getVirtualMachine() + " to host " + host);
-                    }
-                }
         }
     }
 
-        return volumeToPoolObjectMap;
+    /**
+     * We use {@link StoragePoolAllocator} objects to find local storage pools connected to the targetHost where we would be able to allocate the given volume.
+     */
+    private List<StoragePool> getCandidateStoragePoolsToMigrateLocalVolume(VirtualMachineProfile profile, Host targetHost, VolumeVO volume) {
+        List<StoragePool> poolList = new ArrayList<>();
+
+        DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
+        DiskProfile diskProfile = new DiskProfile(volume, diskOffering, profile.getHypervisorType());
+        DataCenterDeployment plan = new DataCenterDeployment(targetHost.getDataCenterId(), targetHost.getPodId(), targetHost.getClusterId(), targetHost.getId(), null, null);
+        ExcludeList avoid = new ExcludeList();
+
+        StoragePoolVO volumeStoragePool = _storagePoolDao.findById(volume.getPoolId());
+        if (volumeStoragePool.isLocal()) {
+            diskProfile.setUseLocalStorage(true);
+        }
+        for (StoragePoolAllocator allocator : _storagePoolAllocators) {
+            List<StoragePool> poolListFromAllocator = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL);
+            if (CollectionUtils.isEmpty(poolListFromAllocator)) {
+                continue;
+            }
+            for (StoragePool pool : poolListFromAllocator) {
+                if (pool.isLocal()) {
+                    poolList.add(pool);
+                }
+            }
+        }
+        return poolList;
     }
 
     private <T extends VMInstanceVO> void moveVmToMigratingState(final T vm, final Long hostId, final ItWorkVO work) throws ConcurrentOperationException {
@@ -2891,8 +2915,11 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
         if (vm.getType() == VirtualMachine.Type.ConsoleProxy || vm.getType() == VirtualMachine.Type.SecondaryStorageVm) {
             NicVO nic = _nicsDao.getControlNicForVM(vm.getId());
             return nic.getIPv4Address();
-        } else if (vm.getType() == VirtualMachine.Type.DomainRouter) return vm.getPrivateIpAddress();
+        } else if (vm.getType() == VirtualMachine.Type.DomainRouter) {
-        else return null;
+            return vm.getPrivateIpAddress();
+        } else {
+            return null;
+        }
     }
     public Command cleanup(final String vmName) {
         VirtualMachine vm = _vmDao.findVMByInstanceName(vmName);
@@ -2961,67 +2988,6 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
         _userVmDao.saveDetails(userVm);
     }
 
-    private void ensureVmRunningContext(final long hostId, VMInstanceVO vm, final Event cause) throws OperationTimedoutException, ResourceUnavailableException,
-    NoTransitionException, InsufficientAddressCapacityException {
-        final VirtualMachineGuru vmGuru = getVmGuru(vm);
-
-        s_logger.debug("VM state is starting on full sync so updating it to running");
-        vm = _vmDao.findById(vm.getId());
-
-        // grab outstanding work item if any
-        final ItWorkVO work = _workDao.findByOutstandingWork(vm.getId(), vm.getState());
-        if (work != null) {
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Found an outstanding work item for this vm " + vm + " in state:" + vm.getState() + ", work id:" + work.getId());
-            }
-        }
-
-        try {
-            stateTransitTo(vm, cause, hostId);
-        } catch (final NoTransitionException e1) {
-            s_logger.warn(e1.getMessage());
-        }
-
-        s_logger.debug("VM's " + vm + " state is starting on full sync so updating it to Running");
-        vm = _vmDao.findById(vm.getId()); // this should ensure vm has the most
-        // up to date info
-
-        final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
-        final List<NicVO> nics = _nicsDao.listByVmId(profile.getId());
-        for (final NicVO nic : nics) {
-            final Network network = _networkModel.getNetwork(nic.getNetworkId());
-            final NicProfile nicProfile =
-                    new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), null, _networkModel.isSecurityGroupSupportedInNetwork(network),
-                            _networkModel.getNetworkTag(profile.getHypervisorType(), network));
-            profile.addNic(nicProfile);
-        }
-
-        final Commands cmds = new Commands(Command.OnError.Stop);
-        s_logger.debug("Finalizing commands that need to be send to complete Start process for the vm " + vm);
-
-        if (vmGuru.finalizeCommandsOnStart(cmds, profile)) {
-            if (cmds.size() != 0) {
-                _agentMgr.send(vm.getHostId(), cmds);
-            }
-
-            if (vmGuru.finalizeStart(profile, vm.getHostId(), cmds, null)) {
-                stateTransitTo(vm, cause, vm.getHostId());
-            } else {
-                s_logger.error("Unable to finish finialization for running vm: " + vm);
-            }
-        } else {
-            s_logger.error("Unable to finalize commands on start for vm: " + vm);
-        }
-
-        if (work != null) {
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Updating outstanding work item to Done, id:" + work.getId());
-            }
-            work.setStep(Step.Done);
-            _workDao.update(work.getId(), work);
-        }
-    }
-
     @Override
     public boolean isRecurring() {
         return true;

engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java

@@ -20,455 +20,77 @@ package com.cloud.vm;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyLong;
-import static org.mockito.Matchers.isA;
-import static org.mockito.Mockito.doNothing;
-import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
 import java.util.ArrayList;
 
-import com.cloud.service.dao.ServiceOfferingDao;
+import org.junit.Assert;
-import junit.framework.Assert;
-
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.Matchers;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
 import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
 import org.mockito.Spy;
-
+import org.mockito.runners.MockitoJUnitRunner;
-import org.apache.cloudstack.api.command.user.vm.RestoreVMCmd;
-import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
-import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
-import org.apache.cloudstack.framework.config.ConfigDepot;
-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 com.cloud.agent.AgentManager;
-import com.cloud.agent.api.CheckVirtualMachineAnswer;
-import com.cloud.agent.api.CheckVirtualMachineCommand;
 import com.cloud.agent.api.Command;
-import com.cloud.agent.api.MigrateWithStorageAnswer;
-import com.cloud.agent.api.MigrateWithStorageCommand;
-import com.cloud.agent.api.MigrateWithStorageCompleteAnswer;
-import com.cloud.agent.api.MigrateWithStorageCompleteCommand;
-import com.cloud.agent.api.MigrateWithStorageReceiveAnswer;
-import com.cloud.agent.api.MigrateWithStorageReceiveCommand;
-import com.cloud.agent.api.MigrateWithStorageSendAnswer;
-import com.cloud.agent.api.MigrateWithStorageSendCommand;
-import com.cloud.agent.api.PrepareForMigrationAnswer;
-import com.cloud.agent.api.PrepareForMigrationCommand;
-import com.cloud.agent.api.ScaleVmAnswer;
-import com.cloud.agent.api.ScaleVmCommand;
 import com.cloud.agent.api.StopAnswer;
 import com.cloud.agent.api.StopCommand;
-import com.cloud.capacity.CapacityManager;
-import com.cloud.dc.dao.ClusterDao;
-import com.cloud.dc.dao.DataCenterDao;
-import com.cloud.dc.dao.HostPodDao;
-import com.cloud.deploy.DeployDestination;
 import com.cloud.deploy.DeploymentPlanner;
-import com.cloud.exception.ConcurrentOperationException;
-import com.cloud.exception.ManagementServerException;
-import com.cloud.exception.OperationTimedoutException;
-import com.cloud.exception.ResourceUnavailableException;
-import com.cloud.exception.VirtualMachineMigrationException;
 import com.cloud.host.HostVO;
-import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.hypervisor.HypervisorGuru;
-import com.cloud.hypervisor.HypervisorGuruManager;
-import com.cloud.offering.ServiceOffering;
 import com.cloud.service.ServiceOfferingVO;
-import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.service.dao.ServiceOfferingDao;
-import com.cloud.storage.dao.StoragePoolHostDao;
-import com.cloud.storage.dao.VMTemplateDao;
 import com.cloud.storage.dao.VolumeDao;
-import com.cloud.storage.Storage.ProvisioningType;
-import com.cloud.storage.DiskOfferingVO;
-import com.cloud.storage.StoragePoolHostVO;
-import com.cloud.storage.VolumeVO;
-import com.cloud.storage.VMTemplateVO;
-import com.cloud.user.Account;
-import com.cloud.user.AccountVO;
-import com.cloud.user.UserVO;
-import com.cloud.user.dao.AccountDao;
-import com.cloud.user.dao.UserDao;
-import com.cloud.utils.Pair;
-import com.cloud.utils.db.EntityManager;
 import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.vm.VirtualMachine.Event;
 import com.cloud.vm.VirtualMachine.State;
-import com.cloud.vm.VirtualMachine.PowerState;
-import com.cloud.vm.dao.UserVmDao;
-import com.cloud.vm.dao.UserVmDetailsDao;
 import com.cloud.vm.dao.VMInstanceDao;
-import com.cloud.vm.snapshot.VMSnapshotManager;
 
+@RunWith(MockitoJUnitRunner.class)
 public class VirtualMachineManagerImplTest {
 
     @Spy
-    VirtualMachineManagerImpl _vmMgr = new VirtualMachineManagerImpl();
+    @InjectMocks
+    private VirtualMachineManagerImpl virtualMachineManagerImpl;
     @Mock
-    VolumeOrchestrationService _storageMgr;
+    private AgentManager agentManagerMock;
     @Mock
-    Account _account;
+    private VMInstanceDao vmInstanceDao;
     @Mock
-    CapacityManager _capacityMgr;
+    private ServiceOfferingDao serviceOfferingDao;
     @Mock
-    AgentManager _agentMgr;
+    private VolumeDao volumeDao;
     @Mock
-    AccountDao _accountDao;
+    private VMInstanceVO vmInstanceMock;
     @Mock
-    ConfigurationDao _configDao;
+    private ServiceOfferingVO serviceOfferingMock;
     @Mock
-    HostDao _hostDao;
+    private HostVO hostMock;
     @Mock
-    UserDao _userDao;
+    private VirtualMachineProfile virtualMachineProfileMock;
-    @Mock
-    UserVmDao _vmDao;
-    @Mock
-    ItWorkDao _workDao;
-    @Mock
-    VMInstanceDao _vmInstanceDao;
-    @Mock
-    ServiceOfferingDao _offeringDao;
-    @Mock
-    VMTemplateDao _templateDao;
-    @Mock
-    VolumeDao _volsDao;
-    @Mock
-    RestoreVMCmd _restoreVMCmd;
-    @Mock
-    AccountVO _accountMock;
-    @Mock
-    UserVO _userMock;
-    @Mock
-    UserVmVO _vmMock;
-    @Mock
-    VMInstanceVO _vmInstance;
-    @Mock
-    ServiceOfferingVO _serviceOfferingMock;
-    @Mock
-    HostVO _host;
-    @Mock
-    VMTemplateVO _templateMock;
-    @Mock
-    VolumeVO _volumeMock;
-    @Mock
-    List<VolumeVO> _rootVols;
-    @Mock
-    ItWorkVO _work;
-    @Mock
-    HostVO hostVO;
-    @Mock
-    UserVmDetailVO _vmDetailVO;
-
-    @Mock
-    ClusterDao _clusterDao;
-    @Mock
-    HostPodDao _podDao;
-    @Mock
-    DataCenterDao _dcDao;
-    @Mock
-    DiskOfferingDao _diskOfferingDao;
-    @Mock
-    PrimaryDataStoreDao _storagePoolDao;
-    @Mock
-    UserVmDetailsDao _vmDetailsDao;
-    @Mock
-    StoragePoolHostDao _poolHostDao;
-    @Mock
-    NetworkOrchestrationService _networkMgr;
-    @Mock
-    HypervisorGuruManager _hvGuruMgr;
-    @Mock
-    VMSnapshotManager _vmSnapshotMgr;
-
-    // Mock objects for vm migration with storage test.
-    @Mock
-    DiskOfferingVO _diskOfferingMock;
-    @Mock
-    StoragePoolVO _srcStoragePoolMock;
-    @Mock
-    StoragePoolVO _destStoragePoolMock;
-    @Mock
-    HostVO _srcHostMock;
-    @Mock
-    HostVO _destHostMock;
-    @Mock
-    Map<Long, Long> _volumeToPoolMock;
-    @Mock
-    EntityManager _entityMgr;
-    @Mock
-    ConfigDepot _configDepot;
 
     @Before
     public void setup() {
-        MockitoAnnotations.initMocks(this);
+        virtualMachineManagerImpl.hostAllocators = new ArrayList<>();
-
-        _vmMgr._templateDao = _templateDao;
-        _vmMgr._volsDao = _volsDao;
-        _vmMgr.volumeMgr = _storageMgr;
-        _vmMgr._capacityMgr = _capacityMgr;
-        _vmMgr._hostDao = _hostDao;
-        _vmMgr._nodeId = 1L;
-        _vmMgr._workDao = _workDao;
-        _vmMgr._agentMgr = _agentMgr;
-        _vmMgr._podDao = _podDao;
-        _vmMgr._clusterDao = _clusterDao;
-        _vmMgr._dcDao = _dcDao;
-        _vmMgr._diskOfferingDao = _diskOfferingDao;
-        _vmMgr._storagePoolDao = _storagePoolDao;
-        _vmMgr._poolHostDao = _poolHostDao;
-        _vmMgr._networkMgr = _networkMgr;
-        _vmMgr._hvGuruMgr = _hvGuruMgr;
-        _vmMgr._vmSnapshotMgr = _vmSnapshotMgr;
-        _vmMgr._vmDao = _vmInstanceDao;
-        _vmMgr._uservmDetailsDao = _vmDetailsDao;
-        _vmMgr._entityMgr = _entityMgr;
-        _vmMgr._configDepot = _configDepot;
-        _vmMgr._offeringDao = _offeringDao;
-        _vmMgr.hostAllocators = new ArrayList<>();
-
-        when(_vmMock.getId()).thenReturn(314l);
-        when(_vmInstance.getId()).thenReturn(1L);
-        when(_vmInstance.getServiceOfferingId()).thenReturn(2L);
-        when(_vmInstance.getInstanceName()).thenReturn("myVm");
-        when(_vmInstance.getHostId()).thenReturn(2L);
-        when(_vmInstance.getType()).thenReturn(VirtualMachine.Type.User);
-        when(_host.getId()).thenReturn(1L);
-        when(_hostDao.findById(anyLong())).thenReturn(null);
-        when(_entityMgr.findById(Matchers.eq(ServiceOffering.class), anyLong())).thenReturn(getSvcoffering(512));
-        when(_workDao.persist(_work)).thenReturn(_work);
-        when(_workDao.update("1", _work)).thenReturn(true);
-        when(_work.getId()).thenReturn("1");
-        doNothing().when(_work).setStep(ItWorkVO.Step.Done);
-        when(_vmInstanceDao.findByUuid(any(String.class))).thenReturn(_vmMock);
-        //doNothing().when(_volsDao).detachVolume(anyLong());
-        //when(_work.setStep(ItWorkVO.Step.Done)).thenReturn("1");
-
-    }
-
-    @Test(expected = CloudRuntimeException.class)
-    public void testScaleVM1() throws Exception {
-
-        DeployDestination dest = new DeployDestination(null, null, null, _host);
-        long l = 1L;
-
-        when(_vmInstanceDao.findById(anyLong())).thenReturn(_vmInstance);
-        _vmMgr.migrateForScale(_vmInstance.getUuid(), l, dest, l);
-
-    }
-
-    @Test(expected = CloudRuntimeException.class)
-    public void testScaleVM2() throws Exception {
-
-        new DeployDestination(null, null, null, _host);
-        doReturn(3L).when(_vmInstance).getId();
-        when(_vmInstanceDao.findById(anyLong())).thenReturn(_vmInstance);
-        ServiceOfferingVO newServiceOffering = getSvcoffering(512);
-        doReturn(1L).when(_vmInstance).getHostId();
-        doReturn(hostVO).when(_hostDao).findById(1L);
-        doReturn(1L).when(_vmInstance).getDataCenterId();
-        doReturn(1L).when(hostVO).getClusterId();
-        when(CapacityManager.CpuOverprovisioningFactor.valueIn(1L)).thenReturn(1.0f);
-        ScaleVmCommand reconfigureCmd =
-                new ScaleVmCommand("myVmName", newServiceOffering.getCpu(), newServiceOffering.getSpeed(), newServiceOffering.getSpeed(), newServiceOffering.getRamSize(),
-                        newServiceOffering.getRamSize(), newServiceOffering.getLimitCpuUse());
-        new ScaleVmAnswer(reconfigureCmd, true, "details");
-        when(_agentMgr.send(2l, reconfigureCmd)).thenReturn(null);
-        _vmMgr.reConfigureVm(_vmInstance.getUuid(), getSvcoffering(256), false);
 
+        when(vmInstanceMock.getId()).thenReturn(1L);
+        when(vmInstanceMock.getServiceOfferingId()).thenReturn(2L);
+        when(vmInstanceMock.getInstanceName()).thenReturn("myVm");
+        when(vmInstanceMock.getHostId()).thenReturn(2L);
+        when(vmInstanceMock.getType()).thenReturn(VirtualMachine.Type.User);
+        when(hostMock.getId()).thenReturn(1L);
     }
 
     @Test(expected = CloudRuntimeException.class)
     public void testScaleVM3() throws Exception {
-
+        when(vmInstanceMock.getHostId()).thenReturn(null);
-        /*VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
+        when(vmInstanceDao.findById(anyLong())).thenReturn(vmInstanceMock);
-
+        when(vmInstanceDao.findByUuid(any(String.class))).thenReturn(vmInstanceMock);
-        Long srcHostId = vm.getHostId();
-        Long oldSvcOfferingId = vm.getServiceOfferingId();
-        if (srcHostId == null) {
-            throw new CloudRuntimeException("Unable to scale the vm because it doesn't have a host id");
-        }*/
-
-        when(_vmInstance.getHostId()).thenReturn(null);
-        when(_vmInstanceDao.findById(anyLong())).thenReturn(_vmInstance);
-        when(_vmInstanceDao.findByUuid(any(String.class))).thenReturn(_vmInstance);
         DeploymentPlanner.ExcludeList excludeHostList = new DeploymentPlanner.ExcludeList();
-        _vmMgr.findHostAndMigrate(_vmInstance.getUuid(), 2l, excludeHostList);
+        virtualMachineManagerImpl.findHostAndMigrate(vmInstanceMock.getUuid(), 2l, excludeHostList);
-
-    }
-
-    private ServiceOfferingVO getSvcoffering(int ramSize) {
-
-        String name = "name";
-        String displayText = "displayText";
-        int cpu = 1;
-        //int ramSize = 256;
-        int speed = 128;
-
-        boolean ha = false;
-        boolean useLocalStorage = false;
-
-        ServiceOfferingVO serviceOffering =
-                new ServiceOfferingVO(name, cpu, ramSize, speed, null, null, ha, displayText, ProvisioningType.THIN, useLocalStorage, false, null, false, null, false);
-        return serviceOffering;
-    }
-
-    private void initializeMockConfigForMigratingVmWithVolumes() throws OperationTimedoutException, ResourceUnavailableException {
-
-        // Mock the source and destination hosts.
-        when(_srcHostMock.getId()).thenReturn(5L);
-        when(_destHostMock.getId()).thenReturn(6L);
-        when(_hostDao.findById(5L)).thenReturn(_srcHostMock);
-        when(_hostDao.findById(6L)).thenReturn(_destHostMock);
-
-        // Mock the vm being migrated.
-        when(_vmMock.getId()).thenReturn(1L);
-        when(_vmMock.getHypervisorType()).thenReturn(HypervisorType.XenServer);
-        when(_vmMock.getState()).thenReturn(State.Running).thenReturn(State.Running).thenReturn(State.Migrating).thenReturn(State.Migrating);
-        when(_vmMock.getHostId()).thenReturn(5L);
-        when(_vmInstance.getId()).thenReturn(1L);
-        when(_vmInstance.getServiceOfferingId()).thenReturn(2L);
-        when(_vmInstance.getInstanceName()).thenReturn("myVm");
-        when(_vmInstance.getHostId()).thenReturn(5L);
-        when(_vmInstance.getType()).thenReturn(VirtualMachine.Type.User);
-        when(_vmInstance.getState()).thenReturn(State.Running).thenReturn(State.Running).thenReturn(State.Migrating).thenReturn(State.Migrating);
-
-        // Mock the work item.
-        when(_workDao.persist(any(ItWorkVO.class))).thenReturn(_work);
-        when(_workDao.update("1", _work)).thenReturn(true);
-        when(_work.getId()).thenReturn("1");
-        doNothing().when(_work).setStep(ItWorkVO.Step.Done);
-
-        // Mock the vm guru and the user vm object that gets returned.
-        _vmMgr._vmGurus = new HashMap<VirtualMachine.Type, VirtualMachineGuru>();
-//        UserVmManagerImpl userVmManager = mock(UserVmManagerImpl.class);
-//        _vmMgr.registerGuru(VirtualMachine.Type.User, userVmManager);
-
-        // Mock the iteration over all the volumes of an instance.
-        Iterator<VolumeVO> volumeIterator = mock(Iterator.class);
-        when(_volsDao.findUsableVolumesForInstance(anyLong())).thenReturn(_rootVols);
-        when(_rootVols.iterator()).thenReturn(volumeIterator);
-        when(volumeIterator.hasNext()).thenReturn(true, false);
-        when(volumeIterator.next()).thenReturn(_volumeMock);
-
-        // Mock the disk offering and pool objects for a volume.
-        when(_volumeMock.getDiskOfferingId()).thenReturn(5L);
-        when(_volumeMock.getPoolId()).thenReturn(200L);
-        when(_volumeMock.getId()).thenReturn(5L);
-        when(_diskOfferingDao.findById(anyLong())).thenReturn(_diskOfferingMock);
-        when(_storagePoolDao.findById(200L)).thenReturn(_srcStoragePoolMock);
-        when(_storagePoolDao.findById(201L)).thenReturn(_destStoragePoolMock);
-
-        // Mock the volume to pool mapping.
-        when(_volumeToPoolMock.get(5L)).thenReturn(201L);
-        when(_destStoragePoolMock.getId()).thenReturn(201L);
-        when(_srcStoragePoolMock.getId()).thenReturn(200L);
-        when(_destStoragePoolMock.isLocal()).thenReturn(false);
-        when(_diskOfferingMock.getUseLocalStorage()).thenReturn(false);
-        when(_poolHostDao.findByPoolHost(anyLong(), anyLong())).thenReturn(mock(StoragePoolHostVO.class));
-
-        // Mock hypervisor guru.
-        HypervisorGuru guruMock = mock(HypervisorGuru.class);
-        when(_hvGuruMgr.getGuru(HypervisorType.XenServer)).thenReturn(guruMock);
-
-        when(_srcHostMock.getClusterId()).thenReturn(3L);
-        when(_destHostMock.getClusterId()).thenReturn(3L);
-
-        // Mock the commands and answers to the agent.
-        PrepareForMigrationAnswer prepAnswerMock = mock(PrepareForMigrationAnswer.class);
-        when(prepAnswerMock.getResult()).thenReturn(true);
-        when(_agentMgr.send(anyLong(), isA(PrepareForMigrationCommand.class))).thenReturn(prepAnswerMock);
-
-        MigrateWithStorageAnswer migAnswerMock = mock(MigrateWithStorageAnswer.class);
-        when(migAnswerMock.getResult()).thenReturn(true);
-        when(_agentMgr.send(anyLong(), isA(MigrateWithStorageCommand.class))).thenReturn(migAnswerMock);
-
-        MigrateWithStorageReceiveAnswer migRecAnswerMock = mock(MigrateWithStorageReceiveAnswer.class);
-        when(migRecAnswerMock.getResult()).thenReturn(true);
-        when(_agentMgr.send(anyLong(), isA(MigrateWithStorageReceiveCommand.class))).thenReturn(migRecAnswerMock);
-
-        MigrateWithStorageSendAnswer migSendAnswerMock = mock(MigrateWithStorageSendAnswer.class);
-        when(migSendAnswerMock.getResult()).thenReturn(true);
-        when(_agentMgr.send(anyLong(), isA(MigrateWithStorageSendCommand.class))).thenReturn(migSendAnswerMock);
-
-        MigrateWithStorageCompleteAnswer migCompleteAnswerMock = mock(MigrateWithStorageCompleteAnswer.class);
-        when(migCompleteAnswerMock.getResult()).thenReturn(true);
-        when(_agentMgr.send(anyLong(), isA(MigrateWithStorageCompleteCommand.class))).thenReturn(migCompleteAnswerMock);
-
-        CheckVirtualMachineAnswer checkVmAnswerMock = mock(CheckVirtualMachineAnswer.class);
-        when(checkVmAnswerMock.getResult()).thenReturn(true);
-        when(checkVmAnswerMock.getState()).thenReturn(PowerState.PowerOn);
-        when(_agentMgr.send(anyLong(), isA(CheckVirtualMachineCommand.class))).thenReturn(checkVmAnswerMock);
-
-        // Mock the state transitions of vm.
-        Pair<Long, Long> opaqueMock = new Pair<Long, Long>(_vmMock.getHostId(), _destHostMock.getId());
-        when(_vmSnapshotMgr.hasActiveVMSnapshotTasks(anyLong())).thenReturn(false);
-        when(_vmInstanceDao.updateState(State.Running, Event.MigrationRequested, State.Migrating, _vmMock, opaqueMock)).thenReturn(true);
-        when(_vmInstanceDao.updateState(State.Migrating, Event.OperationSucceeded, State.Running, _vmMock, opaqueMock)).thenReturn(true);
-    }
-
-    // Check migration of a vm with its volumes within a cluster.
-    @Test
-    public void testMigrateWithVolumeWithinCluster() throws ResourceUnavailableException, ConcurrentOperationException, ManagementServerException,
-    VirtualMachineMigrationException, OperationTimedoutException {
-
-        initializeMockConfigForMigratingVmWithVolumes();
-        when(_srcHostMock.getClusterId()).thenReturn(3L);
-        when(_destHostMock.getClusterId()).thenReturn(3L);
-
-        _vmMgr.migrateWithStorage(_vmInstance.getUuid(), _srcHostMock.getId(), _destHostMock.getId(), _volumeToPoolMock);
-    }
-
-    // Check migration of a vm with its volumes across a cluster.
-    @Test
-    public void testMigrateWithVolumeAcrossCluster() throws ResourceUnavailableException, ConcurrentOperationException, ManagementServerException,
-    VirtualMachineMigrationException, OperationTimedoutException {
-
-        initializeMockConfigForMigratingVmWithVolumes();
-        when(_srcHostMock.getClusterId()).thenReturn(3L);
-        when(_destHostMock.getClusterId()).thenReturn(4L);
-
-        _vmMgr.migrateWithStorage(_vmInstance.getUuid(), _srcHostMock.getId(), _destHostMock.getId(), _volumeToPoolMock);
-    }
-
-    // Check migration of a vm fails when src and destination pool are not of same type; that is, one is shared and
-    // other is local.
-    @Test(expected = CloudRuntimeException.class)
-    public void testMigrateWithVolumeFail1() throws ResourceUnavailableException, ConcurrentOperationException, ManagementServerException,
-    VirtualMachineMigrationException, OperationTimedoutException {
-
-        initializeMockConfigForMigratingVmWithVolumes();
-        when(_srcHostMock.getClusterId()).thenReturn(3L);
-        when(_destHostMock.getClusterId()).thenReturn(3L);
-
-        when(_destStoragePoolMock.isLocal()).thenReturn(true);
-        when(_diskOfferingMock.getUseLocalStorage()).thenReturn(false);
-
-        _vmMgr.migrateWithStorage(_vmInstance.getUuid(), _srcHostMock.getId(), _destHostMock.getId(), _volumeToPoolMock);
-    }
-
-    // Check migration of a vm fails when vm is not in Running state.
-    @Test(expected = ConcurrentOperationException.class)
-    public void testMigrateWithVolumeFail2() throws ResourceUnavailableException, ConcurrentOperationException, ManagementServerException,
-    VirtualMachineMigrationException, OperationTimedoutException {
-
-        initializeMockConfigForMigratingVmWithVolumes();
-        when(_srcHostMock.getClusterId()).thenReturn(3L);
-        when(_destHostMock.getClusterId()).thenReturn(3L);
-
-        when(_vmMock.getState()).thenReturn(State.Stopped);
-
-        _vmMgr.migrateWithStorage(_vmInstance.getUuid(), _srcHostMock.getId(), _destHostMock.getId(), _volumeToPoolMock);
     }
 
     @Test
@@ -479,9 +101,9 @@ public class VirtualMachineManagerImplTest {
         StopAnswer answer = new StopAnswer(new StopCommand(vm, false, false), "ok", true);
         when(profile.getVirtualMachine()).thenReturn(vm);
         when(vm.getHostId()).thenReturn(1L);
-        when(_agentMgr.send(anyLong(), (Command)any())).thenReturn(answer);
+        when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer);
 
-        boolean actual = _vmMgr.sendStop(guru, profile, false, false);
+        boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false);
 
         Assert.assertTrue(actual);
     }
@@ -494,9 +116,9 @@ public class VirtualMachineManagerImplTest {
         StopAnswer answer = new StopAnswer(new StopCommand(vm, false, false), "fail", false);
         when(profile.getVirtualMachine()).thenReturn(vm);
         when(vm.getHostId()).thenReturn(1L);
-        when(_agentMgr.send(anyLong(), (Command)any())).thenReturn(answer);
+        when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer);
 
-        boolean actual = _vmMgr.sendStop(guru, profile, false, false);
+        boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false);
 
         Assert.assertFalse(actual);
     }
@@ -508,38 +130,38 @@ public class VirtualMachineManagerImplTest {
         VirtualMachineProfile profile = mock(VirtualMachineProfile.class);
         when(profile.getVirtualMachine()).thenReturn(vm);
         when(vm.getHostId()).thenReturn(1L);
-        when(_agentMgr.send(anyLong(), (Command)any())).thenReturn(null);
+        when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(null);
 
-        boolean actual = _vmMgr.sendStop(guru, profile, false, false);
+        boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false);
 
         Assert.assertFalse(actual);
     }
 
     @Test
     public void testExeceuteInSequence() {
-        assertTrue(_vmMgr.getExecuteInSequence(HypervisorType.XenServer) == false);
+        assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.XenServer) == false);
-        assertTrue(_vmMgr.getExecuteInSequence(HypervisorType.KVM) == false);
+        assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.KVM) == false);
-        assertTrue(_vmMgr.getExecuteInSequence(HypervisorType.VMware) == HypervisorGuru.VmwareFullClone.value());
+        assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.VMware) == HypervisorGuru.VmwareFullClone.value());
-        assertTrue(_vmMgr.getExecuteInSequence(HypervisorType.Ovm3) == VirtualMachineManager.ExecuteInSequence.value());
+        assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.Ovm3) == VirtualMachineManager.ExecuteInSequence.value());
     }
 
     @Test
     public void testCheckIfCanUpgrade() throws Exception {
-        when(_vmInstance.getState()).thenReturn(State.Stopped);
+        when(vmInstanceMock.getState()).thenReturn(State.Stopped);
-        when(_serviceOfferingMock.isDynamic()).thenReturn(true);
+        when(serviceOfferingMock.isDynamic()).thenReturn(true);
-        when(_vmInstance.getServiceOfferingId()).thenReturn(1l);
+        when(vmInstanceMock.getServiceOfferingId()).thenReturn(1l);
-        when(_serviceOfferingMock.getId()).thenReturn(2l);
+        when(serviceOfferingMock.getId()).thenReturn(2l);
 
         ServiceOfferingVO mockCurrentServiceOffering = mock(ServiceOfferingVO.class);
 
-        when(_offeringDao.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(mockCurrentServiceOffering);
+        when(serviceOfferingDao.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(mockCurrentServiceOffering);
         when(mockCurrentServiceOffering.getUseLocalStorage()).thenReturn(true);
-        when(_serviceOfferingMock.getUseLocalStorage()).thenReturn(true);
+        when(serviceOfferingMock.getUseLocalStorage()).thenReturn(true);
         when(mockCurrentServiceOffering.getSystemUse()).thenReturn(true);
-        when(_serviceOfferingMock.getSystemUse()).thenReturn(true);
+        when(serviceOfferingMock.getSystemUse()).thenReturn(true);
         when(mockCurrentServiceOffering.getTags()).thenReturn("x,y");
-        when(_serviceOfferingMock.getTags()).thenReturn("z,x,y");
+        when(serviceOfferingMock.getTags()).thenReturn("z,x,y");
 
-        _vmMgr.checkIfCanUpgrade(_vmInstance, _serviceOfferingMock);
+        virtualMachineManagerImpl.checkIfCanUpgrade(vmInstanceMock, serviceOfferingMock);
     }
 }

engine/schema/src/main/java/com/cloud/hypervisor/dao/HypervisorCapabilitiesDaoImpl.java

@@ -18,7 +18,7 @@ package com.cloud.hypervisor.dao;
 
 import java.util.List;
 
-
+import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
@@ -66,6 +66,9 @@ public class HypervisorCapabilitiesDaoImpl extends GenericDaoBase<HypervisorCapa
 
     @Override
     public HypervisorCapabilitiesVO findByHypervisorTypeAndVersion(HypervisorType hypervisorType, String hypervisorVersion) {
+        if (StringUtils.isBlank(hypervisorVersion)) {
+            hypervisorVersion = DEFAULT_VERSION;
+        }
         SearchCriteria<HypervisorCapabilitiesVO> sc = HypervisorTypeAndVersionSearch.create();
         sc.setParameters("hypervisorType", hypervisorType);
         sc.setParameters("hypervisorVersion", hypervisorVersion);
@@ -76,11 +79,13 @@ public class HypervisorCapabilitiesDaoImpl extends GenericDaoBase<HypervisorCapa
     public Long getMaxGuestsLimit(HypervisorType hypervisorType, String hypervisorVersion) {
         Long defaultLimit = new Long(50);
         HypervisorCapabilitiesVO result = getCapabilities(hypervisorType, hypervisorVersion);
-        if (result == null)
+        if (result == null) {
             return defaultLimit;
+        }
         Long limit = result.getMaxGuestsLimit();
-        if (limit == null)
+        if (limit == null) {
             return defaultLimit;
+        }
         return limit;
     }
 

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

@@ -231,23 +231,21 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
 
     @Override
     public List<StoragePoolVO> listBy(long datacenterId, Long podId, Long clusterId, ScopeType scope) {
+        SearchCriteria<StoragePoolVO> sc = null;
         if (clusterId != null) {
-            SearchCriteria<StoragePoolVO> sc = DcPodSearch.create();
+            sc = DcPodSearch.create();
-            sc.setParameters("datacenterId", datacenterId);
-            sc.setParameters("podId", podId);
-            sc.setParameters("status", Status.Up);
-            sc.setParameters("scope", scope);
-
             sc.setParameters("cluster", clusterId);
-            return listBy(sc);
         } else {
-            SearchCriteria<StoragePoolVO> sc = DcPodAnyClusterSearch.create();
+            sc = DcPodAnyClusterSearch.create();
+
+        }
         sc.setParameters("datacenterId", datacenterId);
         sc.setParameters("podId", podId);
         sc.setParameters("status", Status.Up);
+        if (scope != null) {
             sc.setParameters("scope", scope);
-            return listBy(sc);
         }
+        return listBy(sc);
     }
 
     @Override

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

@@ -48,11 +48,6 @@ public class ClusterScopeStoragePoolAllocator extends AbstractStoragePoolAllocat
     protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
         s_logger.debug("ClusterScopeStoragePoolAllocator looking for storage pool");
 
-        if (dskCh.useLocalStorage()) {
-            // cluster wide allocator should bail out in case of local disk
-            return null;
-        }
-
         List<StoragePool> suitablePools = new ArrayList<StoragePool>();
 
         long dcId = plan.getDataCenterId();

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

@@ -140,15 +140,15 @@ import org.apache.cloudstack.api.command.admin.offering.DeleteDiskOfferingCmd;
 import org.apache.cloudstack.api.command.admin.offering.DeleteServiceOfferingCmd;
 import org.apache.cloudstack.api.command.admin.offering.UpdateDiskOfferingCmd;
 import org.apache.cloudstack.api.command.admin.offering.UpdateServiceOfferingCmd;
+import org.apache.cloudstack.api.command.admin.outofbandmanagement.ChangeOutOfBandManagementPasswordCmd;
+import org.apache.cloudstack.api.command.admin.outofbandmanagement.ConfigureOutOfBandManagementCmd;
 import org.apache.cloudstack.api.command.admin.outofbandmanagement.DisableOutOfBandManagementForClusterCmd;
 import org.apache.cloudstack.api.command.admin.outofbandmanagement.DisableOutOfBandManagementForHostCmd;
 import org.apache.cloudstack.api.command.admin.outofbandmanagement.DisableOutOfBandManagementForZoneCmd;
 import org.apache.cloudstack.api.command.admin.outofbandmanagement.EnableOutOfBandManagementForClusterCmd;
 import org.apache.cloudstack.api.command.admin.outofbandmanagement.EnableOutOfBandManagementForHostCmd;
 import org.apache.cloudstack.api.command.admin.outofbandmanagement.EnableOutOfBandManagementForZoneCmd;
-import org.apache.cloudstack.api.command.admin.outofbandmanagement.ConfigureOutOfBandManagementCmd;
 import org.apache.cloudstack.api.command.admin.outofbandmanagement.IssueOutOfBandManagementPowerActionCmd;
-import org.apache.cloudstack.api.command.admin.outofbandmanagement.ChangeOutOfBandManagementPasswordCmd;
 import org.apache.cloudstack.api.command.admin.pod.CreatePodCmd;
 import org.apache.cloudstack.api.command.admin.pod.DeletePodCmd;
 import org.apache.cloudstack.api.command.admin.pod.ListPodsByCmd;
@@ -519,7 +519,6 @@ import org.apache.cloudstack.api.command.user.zone.ListZonesCmd;
 import org.apache.cloudstack.config.Configuration;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
 import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
 import org.apache.cloudstack.framework.config.ConfigDepot;
 import org.apache.cloudstack.framework.config.ConfigKey;
@@ -535,6 +534,8 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.utils.identity.ManagementServerNode;
 import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.agent.AgentManager;
@@ -794,8 +795,6 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     @Inject
     private HighAvailabilityManager _haMgr;
     @Inject
-    private DataStoreManager dataStoreMgr;
-    @Inject
     private HostTagsDao _hostTagsDao;
     @Inject
     private ConfigDepot _configDepot;
@@ -809,12 +808,12 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     private DeploymentPlanningManager _dpMgr;
     @Inject
     private GuestOsDetailsDao _guestOsDetailsDao;
+    @Inject
+    private KeystoreManager _ksMgr;
 
     private LockMasterListener _lockMasterListener;
     private final ScheduledExecutorService _eventExecutor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("EventChecker"));
     private final ScheduledExecutorService _alertExecutor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("AlertChecker"));
-    @Inject
-    private KeystoreManager _ksMgr;
 
     private Map<String, String> _configs;
 
@@ -1224,14 +1223,14 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
             allHosts.remove(srcHost);
 
             for (final VolumeVO volume : volumes) {
-                final StoragePool storagePool = _poolDao.findById(volume.getPoolId());
+                StoragePool storagePool = _poolDao.findById(volume.getPoolId());
-                final Long volClusterId = storagePool.getClusterId();
+                Long volClusterId = storagePool.getClusterId();
 
-                for (final Iterator<HostVO> iterator = allHosts.iterator(); iterator.hasNext();) {
+                for (Iterator<HostVO> iterator = allHosts.iterator(); iterator.hasNext();) {
                     final Host host = iterator.next();
 
                     if (volClusterId != null) {
-                        if (!host.getClusterId().equals(volClusterId) || usesLocal) {
+                        if (storagePool.isLocal() || !host.getClusterId().equals(volClusterId) || usesLocal) {
                             if (storagePool.isManaged()) {
                                 // At the time being, we do not support storage migration of a volume from managed storage unless the managed storage
                                 // is at the zone level and the source and target storage pool is the same.
@@ -1346,8 +1345,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         }
 
         // Volume must be attached to an instance for live migration.
-        final List<StoragePool> allPools = new ArrayList<StoragePool>();
+        List<? extends StoragePool> allPools = new ArrayList<StoragePool>();
-        final List<StoragePool> suitablePools = new ArrayList<StoragePool>();
+        List<? extends StoragePool> suitablePools = new ArrayList<StoragePool>();
 
         // Volume must be in Ready state to be migrated.
         if (!Volume.State.Ready.equals(volume.getState())) {
@@ -1355,11 +1354,6 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
             return new Pair<List<? extends StoragePool>, List<? extends StoragePool>>(allPools, suitablePools);
         }
 
-        if (!_volumeMgr.volumeOnSharedStoragePool(volume)) {
-            s_logger.info("Volume " + volume + " is on local storage. It cannot be migrated to another pool.");
-            return new Pair<List<? extends StoragePool>, List<? extends StoragePool>>(allPools, suitablePools);
-        }
-
         final Long instanceId = volume.getInstanceId();
         VMInstanceVO vm = null;
         if (instanceId != null) {
@@ -1397,55 +1391,78 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
             }
         }
 
-        // Source pool of the volume.
+        StoragePool srcVolumePool = _poolDao.findById(volume.getPoolId());
-        final StoragePoolVO srcVolumePool = _poolDao.findById(volume.getPoolId());
+        allPools = getAllStoragePoolCompatileWithVolumeSourceStoragePool(srcVolumePool);
-        // Get all the pools available. Only shared pools are considered because only a volume on a shared pools
+        allPools.remove(srcVolumePool);
-        // can be live migrated while the virtual machine stays on the same host.
+        suitablePools = findAllSuitableStoragePoolsForVm(volume, vm, srcVolumePool);
-
-        List<StoragePoolVO> storagePools;
-
-        if (srcVolumePool.getClusterId() != null) {
-            storagePools = _poolDao.findPoolsByTags(volume.getDataCenterId(), srcVolumePool.getPodId(), srcVolumePool.getClusterId(), null);
-        }
-        else {
-            storagePools = new ArrayList<>();
-        }
-
-        List<StoragePoolVO> zoneWideStoragePools = _poolDao.findZoneWideStoragePoolsByTags(volume.getDataCenterId(), null);
-
-        storagePools.addAll(zoneWideStoragePools);
-
-        storagePools.remove(srcVolumePool);
-        for (final StoragePoolVO pool : storagePools) {
-            if (pool.isShared()) {
-                allPools.add((StoragePool)dataStoreMgr.getPrimaryDataStore(pool.getId()));
-            }
-        }
-
-        // Get all the suitable pools.
-        // Exclude the current pool from the list of pools to which the volume can be migrated.
-        final ExcludeList avoid = new ExcludeList();
-        avoid.addPool(srcVolumePool.getId());
-
-        // Volume stays in the same cluster after migration.
-        final DataCenterDeployment plan = new DataCenterDeployment(volume.getDataCenterId(), srcVolumePool.getPodId(), srcVolumePool.getClusterId(), null, null, null);
-        final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
-
-        final DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
-        final DiskProfile diskProfile = new DiskProfile(volume, diskOffering, profile.getHypervisorType());
-
-        // Call the storage pool allocator to find the list of storage pools.
-        for (final StoragePoolAllocator allocator : _storagePoolAllocators) {
-            final List<StoragePool> pools = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL);
-            if (pools != null && !pools.isEmpty()) {
-                suitablePools.addAll(pools);
-                break;
-            }
-        }
 
         return new Pair<List<? extends StoragePool>, List<? extends StoragePool>>(allPools, suitablePools);
     }
 
+
+    /**
+     * This method looks for all storage pools that are compatible with the given volume.
+     * <ul>
+     *  <li>We will look for storage systems that are zone wide.</li>
+     *  <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) {
+        List<StoragePoolVO> storagePools = new ArrayList<>();
+        List<StoragePoolVO> zoneWideStoragePools = _poolDao.findZoneWideStoragePoolsByTags(srcVolumePool.getDataCenterId(), null);
+        if(CollectionUtils.isNotEmpty(zoneWideStoragePools)) {
+            storagePools.addAll(zoneWideStoragePools);
+        }
+        List<StoragePoolVO> clusterAndLocalStoragePools = _poolDao.listBy(srcVolumePool.getDataCenterId(), srcVolumePool.getPodId(), srcVolumePool.getClusterId(), null);
+        if(CollectionUtils.isNotEmpty(clusterAndLocalStoragePools)) {
+            storagePools.addAll(clusterAndLocalStoragePools);
+        }
+        return storagePools;
+    }
+
+    /**
+     *  Looks for all suitable storage pools to allocate the given volume.
+     *  We take into account the service offering of the VM and volume to find suitable storage pools. It is also excluded from the search the current storage pool used by the volume.
+     *  We use {@link StoragePoolAllocator} to look for possible storage pools to allocate the given volume. We will look for possible local storage poosl even if the volume is using a shared storage disk offering.
+     *
+     *  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) {
+        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());
+
+        DataCenterDeployment plan = new DataCenterDeployment(volume.getDataCenterId(), srcVolumePool.getPodId(), srcVolumePool.getClusterId(), null, null, null);
+        VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
+
+        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, profile.getHypervisorType());
+        diskProfile.setUseLocalStorage(true);
+
+        for (StoragePoolAllocator allocator : _storagePoolAllocators) {
+            List<StoragePool> pools = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL);
+            if (CollectionUtils.isEmpty(pools)) {
+                continue;
+            }
+            for (StoragePool pool : pools) {
+                boolean isLocalPoolSameHostAsSourcePool = pool.isLocal() && StringUtils.equals(host.getPrivateIpAddress(), pool.getHostAddress());
+                if (isLocalPoolSameHostAsSourcePool || pool.isShared()) {
+                    suitablePools.add(pool);
+                }
+
+            }
+        }
+        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 Filter searchFilter = new Filter(HostVO.class, "id", Boolean.TRUE, startIndex, pageSize);
@@ -1888,10 +1905,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         vlanSearch.and("vlanType", vlanSearch.entity().getVlanType(), SearchCriteria.Op.EQ);
         sb.join("vlanSearch", vlanSearch, sb.entity().getVlanId(), vlanSearch.entity().getId(), JoinBuilder.JoinType.INNER);
 
-        boolean allocatedOnly = false;
         if (isAllocated != null && isAllocated == true) {
             sb.and("allocated", sb.entity().getAllocatedTime(), SearchCriteria.Op.NNULL);
-            allocatedOnly = true;
         }
 
         VlanType vlanType = null;
@@ -2149,7 +2164,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         if(cmd.getDetails() != null && !cmd.getDetails().isEmpty()){
             Map<String, String> detailsMap = cmd.getDetails();
             for(Object key: detailsMap.keySet()){
-                _guestOsDetailsDao.addDetail(guestOsPersisted.getId(),(String) key,detailsMap.get((String) key), false);
+                _guestOsDetailsDao.addDetail(guestOsPersisted.getId(),(String) key,detailsMap.get(key), false);
             }
         }
 
@@ -2182,7 +2197,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         if(cmd.getDetails() != null && !cmd.getDetails().isEmpty()){
             Map<String, String> detailsMap = cmd.getDetails();
             for(Object key: detailsMap.keySet()){
-                _guestOsDetailsDao.addDetail(id,(String) key,detailsMap.get((String) key), false);
+                _guestOsDetailsDao.addDetail(id,(String) key,detailsMap.get(key), false);
             }
         }
 

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

@@ -16,6 +16,70 @@
 // under the License.
 package com.cloud.storage;
 
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.ExecutionException;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.ExtractVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.GetUploadParamsForVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
+import org.apache.cloudstack.api.command.user.volume.UploadVolumeCmd;
+import org.apache.cloudstack.api.response.GetUploadParamsResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
+import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
+import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
+import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService.VolumeApiResult;
+import org.apache.cloudstack.framework.async.AsyncCallFuture;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.jobs.AsyncJob;
+import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext;
+import org.apache.cloudstack.framework.jobs.AsyncJobManager;
+import org.apache.cloudstack.framework.jobs.Outcome;
+import org.apache.cloudstack.framework.jobs.dao.VmWorkJobDao;
+import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
+import org.apache.cloudstack.framework.jobs.impl.OutcomeImpl;
+import org.apache.cloudstack.framework.jobs.impl.VmWorkJobVO;
+import org.apache.cloudstack.jobs.JobInfo;
+import org.apache.cloudstack.storage.command.AttachAnswer;
+import org.apache.cloudstack.storage.command.AttachCommand;
+import org.apache.cloudstack.storage.command.DettachCommand;
+import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
+import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
+import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
+import org.apache.log4j.Logger;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.ModifyTargetsCommand;
@@ -26,7 +90,6 @@ import com.cloud.configuration.Config;
 import com.cloud.configuration.ConfigurationManager;
 import com.cloud.configuration.Resource.ResourceType;
 import com.cloud.dc.ClusterDetailsDao;
-import com.cloud.dc.ClusterVO;
 import com.cloud.dc.DataCenter;
 import com.cloud.dc.DataCenterVO;
 import com.cloud.dc.dao.DataCenterDao;
@@ -53,7 +116,6 @@ import com.cloud.storage.dao.DiskOfferingDao;
 import com.cloud.storage.dao.SnapshotDao;
 import com.cloud.storage.dao.VMTemplateDao;
 import com.cloud.storage.dao.VolumeDao;
-import com.cloud.storage.dao.VolumeDetailsDao;
 import com.cloud.storage.snapshot.SnapshotApiService;
 import com.cloud.storage.snapshot.SnapshotManager;
 import com.cloud.template.TemplateManager;
@@ -111,150 +173,82 @@ import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonParseException;
 
-import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.ExtractVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.GetUploadParamsForVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
-import org.apache.cloudstack.api.command.user.volume.UploadVolumeCmd;
-import org.apache.cloudstack.api.response.GetUploadParamsResponse;
-import org.apache.cloudstack.context.CallContext;
-import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
-import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
-import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
-import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
-import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
-import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService.VolumeApiResult;
-import org.apache.cloudstack.framework.async.AsyncCallFuture;
-import org.apache.cloudstack.framework.config.ConfigKey;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.framework.jobs.AsyncJob;
-import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext;
-import org.apache.cloudstack.framework.jobs.AsyncJobManager;
-import org.apache.cloudstack.framework.jobs.Outcome;
-import org.apache.cloudstack.framework.jobs.dao.VmWorkJobDao;
-import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
-import org.apache.cloudstack.framework.jobs.impl.OutcomeImpl;
-import org.apache.cloudstack.framework.jobs.impl.VmWorkJobVO;
-import org.apache.cloudstack.jobs.JobInfo;
-import org.apache.cloudstack.storage.command.AttachAnswer;
-import org.apache.cloudstack.storage.command.AttachCommand;
-import org.apache.cloudstack.storage.command.DettachCommand;
-import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
-import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
-import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
-import org.apache.cloudstack.utils.identity.ManagementServerNode;
-import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
-import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
-import org.apache.log4j.Logger;
-import org.joda.time.DateTime;
-import org.joda.time.DateTimeZone;
-
-import javax.inject.Inject;
-
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
-import java.util.concurrent.ExecutionException;
-
 public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiService, VmWorkJobHandler {
     private final static Logger s_logger = Logger.getLogger(VolumeApiServiceImpl.class);
     public static final String VM_WORK_JOB_HANDLER = VolumeApiServiceImpl.class.getSimpleName();
 
     @Inject
-    VolumeOrchestrationService _volumeMgr;
+    private UserVmManager _userVmMgr;
     @Inject
-    EntityManager _entityMgr;
+    private VolumeOrchestrationService _volumeMgr;
     @Inject
-    AgentManager _agentMgr;
+    private EntityManager _entityMgr;
     @Inject
-    TemplateManager _tmpltMgr;
+    private AgentManager _agentMgr;
     @Inject
-    SnapshotManager _snapshotMgr;
+    private TemplateManager _tmpltMgr;
     @Inject
-    AccountManager _accountMgr;
+    private SnapshotManager _snapshotMgr;
     @Inject
-    ConfigurationManager _configMgr;
+    private AccountManager _accountMgr;
     @Inject
-    VolumeDao _volsDao;
+    private ConfigurationManager _configMgr;
     @Inject
-    VolumeDetailsDao _volDetailDao;
+    private VolumeDao _volsDao;
     @Inject
-    HostDao _hostDao;
+    private HostDao _hostDao;
     @Inject
-    SnapshotDao _snapshotDao;
+    private SnapshotDao _snapshotDao;
     @Inject
-    ServiceOfferingDetailsDao _serviceOfferingDetailsDao;
+    private ServiceOfferingDetailsDao _serviceOfferingDetailsDao;
     @Inject
-    StoragePoolDetailsDao storagePoolDetailsDao;
+    private UserVmDao _userVmDao;
     @Inject
-    UserVmDao _userVmDao;
+    private UserVmService _userVmService;
     @Inject
-    UserVmService _userVmService;
+    private VolumeDataStoreDao _volumeStoreDao;
     @Inject
-    VolumeDataStoreDao _volumeStoreDao;
+    private VMInstanceDao _vmInstanceDao;
     @Inject
-    VMInstanceDao _vmInstanceDao;
+    private PrimaryDataStoreDao _storagePoolDao;
     @Inject
-    PrimaryDataStoreDao _storagePoolDao;
+    private DiskOfferingDao _diskOfferingDao;
     @Inject
-    DiskOfferingDao _diskOfferingDao;
+    private AccountDao _accountDao;
     @Inject
-    AccountDao _accountDao;
+    private DataCenterDao _dcDao;
     @Inject
-    DataCenterDao _dcDao = null;
+    private VMTemplateDao _templateDao;
     @Inject
-    VMTemplateDao _templateDao;
+    private ResourceLimitService _resourceLimitMgr;
     @Inject
-    ResourceLimitService _resourceLimitMgr;
+    private VmDiskStatisticsDao _vmDiskStatsDao;
     @Inject
-    VmDiskStatisticsDao _vmDiskStatsDao;
+    private VMSnapshotDao _vmSnapshotDao;
     @Inject
-    VMSnapshotDao _vmSnapshotDao;
+    private ConfigurationDao _configDao;
     @Inject
-    ConfigurationDao _configDao;
+    private DataStoreManager dataStoreMgr;
     @Inject
-    DataStoreManager dataStoreMgr;
+    private VolumeService volService;
     @Inject
-    VolumeService volService;
+    private VolumeDataFactory volFactory;
     @Inject
-    VolumeDataFactory volFactory;
+    private SnapshotApiService snapshotMgr;
     @Inject
-    SnapshotApiService snapshotMgr;
+    private UUIDManager _uuidMgr;
     @Inject
-    UUIDManager _uuidMgr;
+    private HypervisorCapabilitiesDao _hypervisorCapabilitiesDao;
     @Inject
-    HypervisorCapabilitiesDao _hypervisorCapabilitiesDao;
+    private AsyncJobManager _jobMgr;
     @Inject
-    AsyncJobManager _jobMgr;
+    private VmWorkJobDao _workJobDao;
     @Inject
-    VmWorkJobDao _workJobDao;
+    private ClusterDetailsDao _clusterDetailsDao;
     @Inject
-    ClusterDetailsDao _clusterDetailsDao;
+    private StorageManager storageMgr;
-    @Inject
+
-    UserVmManager _userVmMgr;
     protected Gson _gson;
-    @Inject
-    StorageManager storageMgr;
 
     private List<StoragePoolAllocator> _storagePoolAllocators;
 
@@ -1586,15 +1580,15 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
             Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob());
             if (jobResult != null) {
-                if (jobResult instanceof ConcurrentOperationException)
+                if (jobResult instanceof ConcurrentOperationException) {
                     throw (ConcurrentOperationException)jobResult;
-                else if (jobResult instanceof InvalidParameterValueException)
+                } else if (jobResult instanceof InvalidParameterValueException) {
                     throw (InvalidParameterValueException)jobResult;
-                else if (jobResult instanceof RuntimeException)
+                } else if (jobResult instanceof RuntimeException) {
                     throw (RuntimeException)jobResult;
-                else if (jobResult instanceof Throwable)
+                } else if (jobResult instanceof Throwable) {
                     throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
-                else if (jobResult instanceof Long) {
+                } else if (jobResult instanceof Long) {
                     vol = _volsDao.findById((Long)jobResult);
                 }
             }
@@ -1608,8 +1602,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
         VolumeVO volume = _volsDao.findById(volumeId);
 
-        if(volume == null)
+        if(volume == null) {
             throw new InvalidParameterValueException("The volume id doesn't exist");
+        }
 
         if (path != null) {
             volume.setPath(path);
@@ -1690,8 +1685,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     }
 
     private boolean isVolumeDestroyed(Volume volume){
-        if(volume.getState() == Volume.State.Destroy || volume.getState() == Volume.State.Expunging && volume.getState() == Volume.State.Expunged)
+        if(volume.getState() == Volume.State.Destroy || volume.getState() == Volume.State.Expunging && volume.getState() == Volume.State.Expunged) {
             return true;
+        }
         return false;
     }
 
@@ -1792,13 +1788,13 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
             Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob());
             if (jobResult != null) {
-                if (jobResult instanceof ConcurrentOperationException)
+                if (jobResult instanceof ConcurrentOperationException) {
                     throw (ConcurrentOperationException)jobResult;
-                else if (jobResult instanceof RuntimeException)
+                } else if (jobResult instanceof RuntimeException) {
                     throw (RuntimeException)jobResult;
-                else if (jobResult instanceof Throwable)
+                } else if (jobResult instanceof Throwable) {
                     throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
-                else if (jobResult instanceof Long) {
+                } else if (jobResult instanceof Long) {
                     vol = _volsDao.findById((Long) jobResult);
                 }
             }
@@ -2033,12 +2029,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                     " as the storage pool is in maintenance mode.");
         }
 
-        if (_volumeMgr.volumeOnSharedStoragePool(vol)) {
-            if (destPool.isLocal()) {
-                throw new InvalidParameterValueException("Migration of volume from shared to local storage pool is not supported");
-            } else {
-                // If the volume is attached to a running vm and the volume is on a shared storage pool, check
-                // to make sure that the destination storage pool is in the same cluster as the vm.
         if (liveMigrateVolume && destPool.getClusterId() != null && srcClusterId != null) {
             if (!srcClusterId.equals(destPool.getClusterId())) {
                 throw new InvalidParameterValueException("Cannot migrate a volume of a virtual machine to a storage pool in a different cluster");
@@ -2049,8 +2039,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
             if (!liveMigrateVolume && vol.volumeType.equals(Volume.Type.ROOT)) {
                 Long hostId = vm.getHostId() != null ? vm.getHostId() : vm.getLastHostId();
                 HostVO host = _hostDao.findById(hostId);
-                        if (host != null)
+                if (host != null) {
                     srcClusterId = host.getClusterId();
+                }
                 if (srcClusterId != null && destPool.getClusterId() != null && !srcClusterId.equals(destPool.getClusterId())) {
                     String srcDcName = _clusterDetailsDao.getVmwareDcName(srcClusterId);
                     String destDcName = _clusterDetailsDao.getVmwareDcName(destPool.getClusterId());
@@ -2061,10 +2052,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 updateMissingRootDiskController(vm, vol.getChainInfo());
             }
         }
-            }
-        } else {
-            throw new InvalidParameterValueException("Migration of volume from local storage pool is not supported");
-        }
 
         if (vm != null) {
             // serialize VM operation
@@ -2093,13 +2080,14 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
                 Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob());
                 if (jobResult != null) {
-                    if (jobResult instanceof ConcurrentOperationException)
+                    if (jobResult instanceof ConcurrentOperationException) {
                         throw (ConcurrentOperationException)jobResult;
-                    else if (jobResult instanceof RuntimeException)
+                    } else if (jobResult instanceof RuntimeException) {
                         throw (RuntimeException)jobResult;
-                    else if (jobResult instanceof Throwable)
+                    } else if (jobResult instanceof Throwable) {
                         throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
                     }
+                }
 
                 // retrieve the migrated new volume from job result
                 if (jobResult != null && jobResult instanceof Long) {
@@ -2175,8 +2163,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         }
 
         VMInstanceVO vm = null;
-        if (volume.getInstanceId() != null)
+        if (volume.getInstanceId() != null) {
             vm = _vmInstanceDao.findById(volume.getInstanceId());
+        }
 
         if (vm != null) {
             // serialize VM operation
@@ -2205,13 +2194,14 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
                 Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob());
                 if (jobResult != null) {
-                    if (jobResult instanceof ConcurrentOperationException)
+                    if (jobResult instanceof ConcurrentOperationException) {
                         throw (ConcurrentOperationException)jobResult;
-                    else if (jobResult instanceof ResourceAllocationException)
+                    } else if (jobResult instanceof ResourceAllocationException) {
                         throw (ResourceAllocationException)jobResult;
-                    else if (jobResult instanceof Throwable)
+                    } else if (jobResult instanceof Throwable) {
                         throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
                     }
+                }
 
                 return _snapshotDao.findById(snapshotId);
             }
@@ -2449,13 +2439,14 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
                 Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob());
                 if (jobResult != null) {
-                    if (jobResult instanceof ConcurrentOperationException)
+                    if (jobResult instanceof ConcurrentOperationException) {
                         throw (ConcurrentOperationException)jobResult;
-                    else if (jobResult instanceof RuntimeException)
+                    } else if (jobResult instanceof RuntimeException) {
                         throw (RuntimeException)jobResult;
-                    else if (jobResult instanceof Throwable)
+                    } else if (jobResult instanceof Throwable) {
                         throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
                     }
+                }
 
                 // retrieve the entity url from job result
                 if (jobResult != null && jobResult instanceof String) {
@@ -2523,24 +2514,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         return volume.isDisplayVolume();
     }
 
-    private String getFormatForPool(StoragePool pool) {
-        ClusterVO cluster = ApiDBUtils.findClusterById(pool.getClusterId());
-
-        if (cluster.getHypervisorType() == HypervisorType.XenServer) {
-            return "vhd";
-        } else if (cluster.getHypervisorType() == HypervisorType.KVM) {
-            return "qcow2";
-        } else if (cluster.getHypervisorType() == HypervisorType.Hyperv) {
-            return "vhdx";
-        } else if (cluster.getHypervisorType() == HypervisorType.VMware) {
-            return "ova";
-        } else if (cluster.getHypervisorType() == HypervisorType.Ovm) {
-            return "raw";
-        } else {
-            return null;
-        }
-    }
-
     private boolean needMoveVolume(VolumeVO existingVolume, VolumeInfo newVolume) {
         if (existingVolume == null || existingVolume.getPoolId() == null || newVolume.getPoolId() == null) {
             return false;
@@ -2832,7 +2805,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
     @Override
     public boolean configure(String name, Map<String, Object> params) {
-
         String maxVolumeSizeInGbString = _configDao.getValue(Config.MaxVolumeSize.toString());
         _maxVolumeSizeInGb = NumbersUtil.parseLong(maxVolumeSizeInGbString, 2000);
         return true;
@@ -2848,15 +2820,15 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     }
 
     public class VmJobVolumeUrlOutcome extends OutcomeImpl<String> {
-
         public VmJobVolumeUrlOutcome(final AsyncJob job) {
             super(String.class, job, VmJobCheckInterval.value(), new Predicate() {
                 @Override
                 public boolean checkCondition() {
                     AsyncJobVO jobVo = _entityMgr.findById(AsyncJobVO.class, job.getId());
                     assert (jobVo != null);
-                    if (jobVo == null || jobVo.getStatus() != JobInfo.Status.IN_PROGRESS)
+                    if (jobVo == null || jobVo.getStatus() != JobInfo.Status.IN_PROGRESS) {
                         return true;
+                    }
 
                     return false;
                 }
@@ -2873,8 +2845,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 public boolean checkCondition() {
                     AsyncJobVO jobVo = _entityMgr.findById(AsyncJobVO.class, job.getId());
                     assert (jobVo != null);
-                    if (jobVo == null || jobVo.getStatus() != JobInfo.Status.IN_PROGRESS)
+                    if (jobVo == null || jobVo.getStatus() != JobInfo.Status.IN_PROGRESS) {
                         return true;
+                    }
 
                     return false;
                 }
@@ -2897,8 +2870,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 public boolean checkCondition() {
                     AsyncJobVO jobVo = _entityMgr.findById(AsyncJobVO.class, job.getId());
                     assert (jobVo != null);
-                    if (jobVo == null || jobVo.getStatus() != JobInfo.Status.IN_PROGRESS)
+                    if (jobVo == null || jobVo.getStatus() != JobInfo.Status.IN_PROGRESS) {
                         return true;
+                    }
 
                     return false;
                 }

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -4995,17 +4995,18 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
 
     private boolean isVMUsingLocalStorage(VMInstanceVO vm) {
         boolean usesLocalStorage = false;
-        ServiceOfferingVO svcOffering = _serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId());
+
-        if (svcOffering.getUseLocalStorage()) {
+        List<VolumeVO> volumes = _volsDao.findByInstance(vm.getId());
-            usesLocalStorage = true;
-        } else {
-            List<VolumeVO> volumes = _volsDao.findByInstanceAndType(vm.getId(), Volume.Type.DATADISK);
         for (VolumeVO vol : volumes) {
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(vol.getDiskOfferingId());
             if (diskOffering.getUseLocalStorage()) {
                 usesLocalStorage = true;
                 break;
             }
+            StoragePoolVO storagePool = _storagePoolDao.findById(vol.getPoolId());
+            if (storagePool.isLocal()) {
+                usesLocalStorage = true;
+                break;
             }
         }
         return usesLocalStorage;

server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java

@@ -16,32 +16,21 @@
 // under the License.
 package com.cloud.storage;
 
-import com.cloud.dc.DataCenterVO;
+import static org.mockito.Matchers.any;
-import com.cloud.dc.dao.DataCenterDao;
+import static org.mockito.Matchers.anyLong;
-import com.cloud.exception.InvalidParameterValueException;
+import static org.mockito.Matchers.anyString;
-import com.cloud.exception.ResourceAllocationException;
+import static org.mockito.Matchers.eq;
-import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import static org.mockito.Mockito.doNothing;
-import com.cloud.org.Grouping;
+import static org.mockito.Mockito.doThrow;
-import com.cloud.serializer.GsonHelper;
+import static org.mockito.Mockito.times;
-import com.cloud.storage.dao.VolumeDao;
+import static org.mockito.Mockito.verify;
-import com.cloud.user.Account;
+import static org.mockito.Mockito.when;
-import com.cloud.user.AccountManager;
+
-import com.cloud.user.AccountVO;
+import java.lang.reflect.Field;
-import com.cloud.user.User;
+import java.util.ArrayList;
-import com.cloud.user.UserVO;
+import java.util.List;
-import com.cloud.utils.db.TransactionLegacy;
+import java.util.UUID;
-import com.cloud.vm.UserVmManager;
+
-import com.cloud.vm.UserVmVO;
-import com.cloud.vm.VirtualMachine;
-import com.cloud.vm.VirtualMachine.State;
-import com.cloud.vm.dao.UserVmDao;
-import com.cloud.vm.dao.VMInstanceDao;
-import com.cloud.vm.snapshot.VMSnapshotVO;
-import com.cloud.vm.snapshot.dao.VMSnapshotDao;
-import com.cloud.user.dao.AccountDao;
-import com.cloud.user.ResourceLimitService;
-import com.cloud.configuration.Resource;
-import com.cloud.host.dao.HostDao;
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
@@ -58,103 +47,103 @@ import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
-import org.junit.Assert;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.Mockito;
-import org.mockito.MockitoAnnotations;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
 
-import javax.inject.Inject;
+import com.cloud.configuration.Resource;
-import java.lang.reflect.Field;
+import com.cloud.dc.DataCenterVO;
-import java.util.ArrayList;
+import com.cloud.dc.dao.DataCenterDao;
-import java.util.List;
+import com.cloud.exception.InvalidParameterValueException;
-import java.util.UUID;
+import com.cloud.exception.ResourceAllocationException;
-
+import com.cloud.host.dao.HostDao;
-import static org.mockito.Matchers.any;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import static org.mockito.Matchers.anyLong;
+import com.cloud.org.Grouping;
-import static org.mockito.Matchers.anyString;
+import com.cloud.serializer.GsonHelper;
-import static org.mockito.Matchers.eq;
+import com.cloud.storage.dao.VolumeDao;
-import static org.mockito.Mockito.doNothing;
+import com.cloud.user.Account;
-import static org.mockito.Mockito.doThrow;
+import com.cloud.user.AccountManager;
-import static org.mockito.Mockito.times;
+import com.cloud.user.AccountVO;
-import static org.mockito.Mockito.verify;
+import com.cloud.user.ResourceLimitService;
-import static org.mockito.Mockito.when;
+import com.cloud.user.User;
+import com.cloud.user.UserVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.vm.UserVmManager;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
 
+@RunWith(MockitoJUnitRunner.class)
 public class VolumeApiServiceImplTest {
-    @Inject
-    VolumeApiServiceImpl _svc = new VolumeApiServiceImpl();
-    @Mock
-    VolumeDao _volumeDao;
-    @Mock
-    AccountManager _accountMgr;
-    @Mock
-    UserVmDao _userVmDao;
-    @Mock
-    PrimaryDataStoreDao _storagePoolDao;
-    @Mock
-    VMSnapshotDao _vmSnapshotDao;
-    @Mock
-    AsyncJobManager _jobMgr;
-    @Mock
-    AsyncJobJoinMapDao _joinMapDao;
-    @Mock
-    VolumeDataFactory _volFactory;
 
+    @Spy
+    @InjectMocks
+    private VolumeApiServiceImpl volumeApiServiceImpl;
     @Mock
-    VMInstanceDao _vmInstanceDao;
+    private VolumeDao _volumeDao;
     @Mock
-    VolumeInfo volumeInfoMock;
+    private AccountManager _accountMgr;
     @Mock
-    SnapshotInfo snapshotInfoMock;
+    private UserVmDao _userVmDao;
     @Mock
-    VolumeService volService;
+    private PrimaryDataStoreDao _storagePoolDao;
     @Mock
-    CreateVolumeCmd createVol;
+    private VMSnapshotDao _vmSnapshotDao;
     @Mock
-    UserVmManager _userVmMgr;
+    private AsyncJobManager _jobMgr;
     @Mock
-    DataCenterDao _dcDao;
+    private AsyncJobJoinMapDao _joinMapDao;
     @Mock
-    ResourceLimitService _resourceLimitMgr;
+    private VolumeDataFactory _volFactory;
     @Mock
-    AccountDao _accountDao;
+    private VMInstanceDao _vmInstanceDao;
     @Mock
-    HostDao _hostDao;
+    private VolumeInfo volumeInfoMock;
+    @Mock
+    private SnapshotInfo snapshotInfoMock;
+    @Mock
+    private VolumeService volService;
+    @Mock
+    private CreateVolumeCmd createVol;
+    @Mock
+    private UserVmManager userVmManager;
+    @Mock
+    private DataCenterDao _dcDao;
+    @Mock
+    private ResourceLimitService _resourceLimitMgr;
+    @Mock
+    private AccountDao _accountDao;
+    @Mock
+    private HostDao _hostDao;
 
-    DetachVolumeCmd detachCmd = new DetachVolumeCmd();
+    private DetachVolumeCmd detachCmd = new DetachVolumeCmd();
-    Class<?> _detachCmdClass = detachCmd.getClass();
+    private Class<?> _detachCmdClass = detachCmd.getClass();
 
 
     @Before
     public void setup() throws Exception {
-        MockitoAnnotations.initMocks(this);
+        volumeApiServiceImpl._gson = GsonHelper.getGsonLogger();
-        _svc._volsDao = _volumeDao;
-        _svc._accountMgr = _accountMgr;
-        _svc._userVmDao = _userVmDao;
-        _svc._storagePoolDao = _storagePoolDao;
-        _svc._vmSnapshotDao = _vmSnapshotDao;
-        _svc._vmInstanceDao = _vmInstanceDao;
-        _svc._jobMgr = _jobMgr;
-        _svc.volFactory = _volFactory;
-        _svc.volService = volService;
-        _svc._userVmMgr = _userVmMgr;
-        _svc._dcDao = _dcDao;
-        _svc._resourceLimitMgr = _resourceLimitMgr;
-        _svc._accountDao = _accountDao;
-        _svc._hostDao = _hostDao;
-        _svc._gson = GsonHelper.getGsonLogger();
 
         // mock caller context
         AccountVO account = new AccountVO("admin", 1L, "networkDomain", Account.ACCOUNT_TYPE_NORMAL, "uuid");
-        AccountVO account2 = new AccountVO("Account2", 2L, "networkDomain", Account.ACCOUNT_TYPE_NORMAL, "uuid");
         UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN);
         CallContext.register(user, account);
         // mock async context
         AsyncJobExecutionContext context = new AsyncJobExecutionContext();
-        AsyncJobExecutionContext.init(_svc._jobMgr, _joinMapDao);
+        AsyncJobExecutionContext.init(_jobMgr, _joinMapDao);
         AsyncJobVO job = new AsyncJobVO();
         context.setJob(job);
         AsyncJobExecutionContext.setCurrentExecutionContext(context);
@@ -164,25 +153,25 @@ public class VolumeApiServiceImplTest {
             // volume of running vm id=1
             VolumeVO volumeOfRunningVm = new VolumeVO("root", 1L, 1L, 1L, 1L, 1L, "root", "root", Storage.ProvisioningType.THIN, 1, null,
                     null, "root", Volume.Type.ROOT);
-            when(_svc._volsDao.findById(1L)).thenReturn(volumeOfRunningVm);
+            when(_volumeDao.findById(1L)).thenReturn(volumeOfRunningVm);
 
             UserVmVO runningVm = new UserVmVO(1L, "vm", "vm", 1, HypervisorType.XenServer, 1L, false,
                     false, 1L, 1L, 1, 1L, null, "vm", null);
             runningVm.setState(State.Running);
             runningVm.setDataCenterId(1L);
-            when(_svc._userVmDao.findById(1L)).thenReturn(runningVm);
+            when(_userVmDao.findById(1L)).thenReturn(runningVm);
 
             // volume of stopped vm id=2
             VolumeVO volumeOfStoppedVm = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", Storage.ProvisioningType.THIN, 1, null,
                     null, "root", Volume.Type.ROOT);
             volumeOfStoppedVm.setPoolId(1L);
-            when(_svc._volsDao.findById(2L)).thenReturn(volumeOfStoppedVm);
+            when(_volumeDao.findById(2L)).thenReturn(volumeOfStoppedVm);
 
             UserVmVO stoppedVm = new UserVmVO(2L, "vm", "vm", 1, HypervisorType.XenServer, 1L, false,
                     false, 1L, 1L, 1, 1L, null, "vm", null);
             stoppedVm.setState(State.Stopped);
             stoppedVm.setDataCenterId(1L);
-            when(_svc._userVmDao.findById(2L)).thenReturn(stoppedVm);
+            when(_userVmDao.findById(2L)).thenReturn(stoppedVm);
 
 
             // volume of hyperV vm id=3
@@ -190,31 +179,31 @@ public class VolumeApiServiceImplTest {
                     false, 1L, 1L, 1, 1L, null, "vm", null);
             hyperVVm.setState(State.Stopped);
             hyperVVm.setDataCenterId(1L);
-            when(_svc._userVmDao.findById(3L)).thenReturn(hyperVVm);
+            when(_userVmDao.findById(3L)).thenReturn(hyperVVm);
 
             VolumeVO volumeOfStoppeHyperVVm = new VolumeVO("root", 1L, 1L, 1L, 1L, 3L, "root", "root", Storage.ProvisioningType.THIN, 1, null,
                     null, "root", Volume.Type.ROOT);
             volumeOfStoppeHyperVVm.setPoolId(1L);
-            when(_svc._volsDao.findById(3L)).thenReturn(volumeOfStoppeHyperVVm);
+            when(_volumeDao.findById(3L)).thenReturn(volumeOfStoppeHyperVVm);
 
             StoragePoolVO unmanagedPool = new StoragePoolVO();
 
-            when(_svc._storagePoolDao.findById(1L)).thenReturn(unmanagedPool);
+            when(_storagePoolDao.findById(1L)).thenReturn(unmanagedPool);
 
             // volume of managed pool id=4
             StoragePoolVO managedPool = new StoragePoolVO();
             managedPool.setManaged(true);
-            when(_svc._storagePoolDao.findById(2L)).thenReturn(managedPool);
+            when(_storagePoolDao.findById(2L)).thenReturn(managedPool);
             VolumeVO managedPoolVolume = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", Storage.ProvisioningType.THIN, 1, null,
                     null, "root", Volume.Type.ROOT);
             managedPoolVolume.setPoolId(2L);
-            when(_svc._volsDao.findById(4L)).thenReturn(managedPoolVolume);
+            when(_volumeDao.findById(4L)).thenReturn(managedPoolVolume);
 
             // non-root non-datadisk volume
             VolumeInfo volumeWithIncorrectVolumeType = Mockito.mock(VolumeInfo.class);
             when(volumeWithIncorrectVolumeType.getId()).thenReturn(5L);
             when(volumeWithIncorrectVolumeType.getVolumeType()).thenReturn(Volume.Type.ISO);
-            when(_svc.volFactory.getVolume(5L)).thenReturn(volumeWithIncorrectVolumeType);
+            when(_volFactory.getVolume(5L)).thenReturn(volumeWithIncorrectVolumeType);
 
             // correct root volume
             VolumeInfo correctRootVolume = Mockito.mock(VolumeInfo.class);
@@ -225,11 +214,11 @@ public class VolumeApiServiceImplTest {
             when(correctRootVolume.getState()).thenReturn(Volume.State.Ready);
             when(correctRootVolume.getTemplateId()).thenReturn(null);
             when(correctRootVolume.getPoolId()).thenReturn(1L);
-            when(_svc.volFactory.getVolume(6L)).thenReturn(correctRootVolume);
+            when(_volFactory.getVolume(6L)).thenReturn(correctRootVolume);
 
             VolumeVO correctRootVolumeVO = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", Storage.ProvisioningType.THIN, 1, null,
                     null, "root", Volume.Type.ROOT);
-            when(_svc._volsDao.findById(6L)).thenReturn(correctRootVolumeVO);
+            when(_volumeDao.findById(6L)).thenReturn(correctRootVolumeVO);
 
             // managed root volume
             VolumeInfo managedVolume = Mockito.mock(VolumeInfo.class);
@@ -238,23 +227,23 @@ public class VolumeApiServiceImplTest {
             when(managedVolume.getVolumeType()).thenReturn(Volume.Type.ROOT);
             when(managedVolume.getInstanceId()).thenReturn(null);
             when(managedVolume.getPoolId()).thenReturn(2L);
-            when(_svc.volFactory.getVolume(7L)).thenReturn(managedVolume);
+            when(_volFactory.getVolume(7L)).thenReturn(managedVolume);
 
             VolumeVO managedVolume1 = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", Storage.ProvisioningType.THIN, 1, null,
                     null, "root", Volume.Type.ROOT);
             managedVolume1.setPoolId(2L);
             managedVolume1.setDataCenterId(1L);
-            when(_svc._volsDao.findById(7L)).thenReturn(managedVolume1);
+            when(_volumeDao.findById(7L)).thenReturn(managedVolume1);
 
             // vm having root volume
             UserVmVO vmHavingRootVolume = new UserVmVO(4L, "vm", "vm", 1, HypervisorType.XenServer, 1L, false,
                     false, 1L, 1L, 1, 1L, null, "vm", null);
             vmHavingRootVolume.setState(State.Stopped);
             vmHavingRootVolume.setDataCenterId(1L);
-            when(_svc._userVmDao.findById(4L)).thenReturn(vmHavingRootVolume);
+            when(_userVmDao.findById(4L)).thenReturn(vmHavingRootVolume);
             List<VolumeVO> vols = new ArrayList<VolumeVO>();
             vols.add(new VolumeVO());
-            when(_svc._volsDao.findByInstanceAndDeviceId(4L, 0L)).thenReturn(vols);
+            when(_volumeDao.findByInstanceAndDeviceId(4L, 0L)).thenReturn(vols);
 
             // volume in uploaded state
             VolumeInfo uploadedVolume = Mockito.mock(VolumeInfo.class);
@@ -264,32 +253,32 @@ public class VolumeApiServiceImplTest {
             when(uploadedVolume.getInstanceId()).thenReturn(null);
             when(uploadedVolume.getPoolId()).thenReturn(1L);
             when(uploadedVolume.getState()).thenReturn(Volume.State.Uploaded);
-            when(_svc.volFactory.getVolume(8L)).thenReturn(uploadedVolume);
+            when(_volFactory.getVolume(8L)).thenReturn(uploadedVolume);
 
             VolumeVO upVolume = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", Storage.ProvisioningType.THIN, 1, null,
                     null, "root", Volume.Type.ROOT);
             upVolume.setPoolId(1L);
             upVolume.setDataCenterId(1L);
             upVolume.setState(Volume.State.Uploaded);
-            when(_svc._volsDao.findById(8L)).thenReturn(upVolume);
+            when(_volumeDao.findById(8L)).thenReturn(upVolume);
 
             // helper dao methods mock
-            when(_svc._vmSnapshotDao.findByVm(any(Long.class))).thenReturn(new ArrayList<VMSnapshotVO>());
+            when(_vmSnapshotDao.findByVm(any(Long.class))).thenReturn(new ArrayList<VMSnapshotVO>());
-            when(_svc._vmInstanceDao.findById(any(Long.class))).thenReturn(stoppedVm);
+            when(_vmInstanceDao.findById(any(Long.class))).thenReturn(stoppedVm);
 
             DataCenterVO enabledZone = Mockito.mock(DataCenterVO.class);
             when(enabledZone.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled);
 
-            when(_svc._dcDao.findById(anyLong())).thenReturn(enabledZone);
+            when(_dcDao.findById(anyLong())).thenReturn(enabledZone);
 
         } finally {
             txn.close("runVolumeDaoImplTest");
         }
 
         // helper methods mock
-        doNothing().when(_svc._accountMgr).checkAccess(any(Account.class), any(AccessType.class), any(Boolean.class), any(ControlledEntity.class));
+        doNothing().when(_accountMgr).checkAccess(any(Account.class), any(AccessType.class), any(Boolean.class), any(ControlledEntity.class));
-        doNothing().when(_svc._jobMgr).updateAsyncJobAttachment(any(Long.class), any(String.class), any(Long.class));
+        doNothing().when(_jobMgr).updateAsyncJobAttachment(any(Long.class), any(String.class), any(Long.class));
-        when(_svc._jobMgr.submitAsyncJob(any(AsyncJobVO.class), any(String.class), any(Long.class))).thenReturn(1L);
+        when(_jobMgr.submitAsyncJob(any(AsyncJobVO.class), any(String.class), any(Long.class))).thenReturn(1L);
     }
 
     /**
@@ -302,7 +291,7 @@ public class VolumeApiServiceImplTest {
         Field dedicateIdField = _detachCmdClass.getDeclaredField("id");
         dedicateIdField.setAccessible(true);
         dedicateIdField.set(detachCmd, 1L);
-        _svc.detachVolumeFromVM(detachCmd);
+        volumeApiServiceImpl.detachVolumeFromVM(detachCmd);
     }
 
     @Test(expected = InvalidParameterValueException.class)
@@ -310,7 +299,7 @@ public class VolumeApiServiceImplTest {
         Field dedicateIdField = _detachCmdClass.getDeclaredField("id");
         dedicateIdField.setAccessible(true);
         dedicateIdField.set(detachCmd, 3L);
-        _svc.detachVolumeFromVM(detachCmd);
+        volumeApiServiceImpl.detachVolumeFromVM(detachCmd);
     }
 
     @Test(expected = InvalidParameterValueException.class)
@@ -318,7 +307,7 @@ public class VolumeApiServiceImplTest {
         Field dedicateIdField = _detachCmdClass.getDeclaredField("id");
         dedicateIdField.setAccessible(true);
         dedicateIdField.set(detachCmd, 4L);
-        _svc.detachVolumeFromVM(detachCmd);
+        volumeApiServiceImpl.detachVolumeFromVM(detachCmd);
     }
 
     @Rule
@@ -330,7 +319,7 @@ public class VolumeApiServiceImplTest {
         Field dedicateIdField = _detachCmdClass.getDeclaredField("id");
         dedicateIdField.setAccessible(true);
         dedicateIdField.set(detachCmd, 2L);
-        _svc.detachVolumeFromVM(detachCmd);
+        volumeApiServiceImpl.detachVolumeFromVM(detachCmd);
     }
 
     /**
@@ -340,44 +329,44 @@ public class VolumeApiServiceImplTest {
     // Negative test - try to attach non-root non-datadisk volume
     @Test(expected = InvalidParameterValueException.class)
     public void attachIncorrectDiskType() throws NoSuchFieldException, IllegalAccessException {
-        _svc.attachVolumeToVM(1L, 5L, 0L);
+        volumeApiServiceImpl.attachVolumeToVM(1L, 5L, 0L);
     }
 
     // Negative test - attach root volume to running vm
     @Test(expected = InvalidParameterValueException.class)
     public void attachRootDiskToRunningVm() throws NoSuchFieldException, IllegalAccessException {
-        _svc.attachVolumeToVM(1L, 6L, 0L);
+        volumeApiServiceImpl.attachVolumeToVM(1L, 6L, 0L);
     }
 
     // Negative test - attach root volume to non-xen vm
     @Test(expected = InvalidParameterValueException.class)
     public void attachRootDiskToHyperVm() throws NoSuchFieldException, IllegalAccessException {
-        _svc.attachVolumeToVM(3L, 6L, 0L);
+        volumeApiServiceImpl.attachVolumeToVM(3L, 6L, 0L);
     }
 
     // Negative test - attach root volume from the managed data store
     @Test(expected = InvalidParameterValueException.class)
     public void attachRootDiskOfManagedDataStore() throws NoSuchFieldException, IllegalAccessException {
-        _svc.attachVolumeToVM(2L, 7L, 0L);
+        volumeApiServiceImpl.attachVolumeToVM(2L, 7L, 0L);
     }
 
     // Negative test - root volume can't be attached to the vm already having a root volume attached
     @Test(expected = InvalidParameterValueException.class)
     public void attachRootDiskToVmHavingRootDisk() throws NoSuchFieldException, IllegalAccessException {
-        _svc.attachVolumeToVM(4L, 6L, 0L);
+        volumeApiServiceImpl.attachVolumeToVM(4L, 6L, 0L);
     }
 
     // Negative test - root volume in uploaded state can't be attached
     @Test(expected = InvalidParameterValueException.class)
     public void attachRootInUploadedState() throws NoSuchFieldException, IllegalAccessException {
-        _svc.attachVolumeToVM(2L, 8L, 0L);
+        volumeApiServiceImpl.attachVolumeToVM(2L, 8L, 0L);
     }
 
     // Positive test - attach ROOT volume in correct state, to the vm not having root volume attached
     @Test
     public void attachRootVolumePositive() throws NoSuchFieldException, IllegalAccessException {
         thrown.expect(NullPointerException.class);
-        _svc.attachVolumeToVM(2L, 6L, 0L);
+        volumeApiServiceImpl.attachVolumeToVM(2L, 6L, 0L);
     }
 
     // volume not Ready
@@ -386,7 +375,7 @@ public class VolumeApiServiceImplTest {
         when(_volFactory.getVolume(anyLong())).thenReturn(volumeInfoMock);
         when(volumeInfoMock.getState()).thenReturn(Volume.State.Allocated);
         when(volumeInfoMock.getPoolId()).thenReturn(1L);
-        _svc.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false, null, false);
+        volumeApiServiceImpl.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false, null, false);
     }
 
     @Test
@@ -396,45 +385,46 @@ public class VolumeApiServiceImplTest {
         when(volumeInfoMock.getInstanceId()).thenReturn(null);
         when(volumeInfoMock.getPoolId()).thenReturn(1L);
         when (volService.takeSnapshot(Mockito.any(VolumeInfo.class))).thenReturn(snapshotInfoMock);
-        _svc.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false, null, false);
+        volumeApiServiceImpl.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false, null, false);
     }
 
     @Test
     public void testNullGetVolumeNameFromCmd() {
         when(createVol.getVolumeName()).thenReturn(null);
-        Assert.assertNotNull(_svc.getVolumeNameFromCommand(createVol));
+        Assert.assertNotNull(volumeApiServiceImpl.getVolumeNameFromCommand(createVol));
     }
 
     @Test
     public void testEmptyGetVolumeNameFromCmd() {
         when(createVol.getVolumeName()).thenReturn("");
-        Assert.assertNotNull(_svc.getVolumeNameFromCommand(createVol));
+        Assert.assertNotNull(volumeApiServiceImpl.getVolumeNameFromCommand(createVol));
     }
 
     @Test
     public void testBlankGetVolumeNameFromCmd() {
         when(createVol.getVolumeName()).thenReturn("   ");
-        Assert.assertNotNull(_svc.getVolumeNameFromCommand(createVol));
+        Assert.assertNotNull(volumeApiServiceImpl.getVolumeNameFromCommand(createVol));
     }
 
     @Test
     public void testNonEmptyGetVolumeNameFromCmd() {
         when(createVol.getVolumeName()).thenReturn("abc");
-        Assert.assertSame(_svc.getVolumeNameFromCommand(createVol), "abc");
+        Assert.assertSame(volumeApiServiceImpl.getVolumeNameFromCommand(createVol), "abc");
     }
 
     @Test
     public void testUpdateMissingRootDiskControllerWithNullChainInfo() {
-        _svc.updateMissingRootDiskController(null, null);
+        volumeApiServiceImpl.updateMissingRootDiskController(null, null);
-        verify(_svc._userVmMgr, times(0)).persistDeviceBusInfo(any(UserVmVO.class), anyString());
+        verify(userVmManager, times(0)).persistDeviceBusInfo(any(UserVmVO.class), anyString());
     }
 
     @Test
     public void testUpdateMissingRootDiskControllerWithValidChainInfo() {
-        UserVmVO vm = _svc._userVmDao.findById(1L);
+        UserVmVO vm = _userVmDao.findById(1L);
-        assert vm.getType() == VirtualMachine.Type.User;
+
-        _svc.updateMissingRootDiskController(vm, "{\"diskDeviceBusName\":\"scsi0:0\",\"diskChain\":[\"[somedatastore] i-3-VM-somePath/ROOT-1.vmdk\"]}");
+        Mockito.doNothing().when(userVmManager).persistDeviceBusInfo(any(UserVmVO.class), eq("scsi"));
-        verify(_svc._userVmMgr, times(1)).persistDeviceBusInfo(any(UserVmVO.class), eq("scsi"));
+        volumeApiServiceImpl.updateMissingRootDiskController(vm, "{\"diskDeviceBusName\":\"scsi0:0\",\"diskChain\":[\"[somedatastore] i-3-VM-somePath/ROOT-1.vmdk\"]}");
+        verify(userVmManager, times(1)).persistDeviceBusInfo(any(UserVmVO.class), eq("scsi"));
     }
 
     @Test
@@ -443,7 +433,7 @@ public class VolumeApiServiceImplTest {
      */
     public void testAllocSnapshotNonManagedStorageArchive() {
         try {
-            _svc.allocSnapshot(6L, 1L, "test", Snapshot.LocationType.SECONDARY);
+            volumeApiServiceImpl.allocSnapshot(6L, 1L, "test", Snapshot.LocationType.SECONDARY);
         } catch (InvalidParameterValueException e) {
             Assert.assertEquals(e.getMessage(), "VolumeId: 6 LocationType is supported only for managed storage");
             return;
@@ -462,7 +452,8 @@ public class VolumeApiServiceImplTest {
      */
     @Test
     public void testResourceLimitCheckForUploadedVolume() throws NoSuchFieldException, IllegalAccessException, ResourceAllocationException {
-        doThrow(new ResourceAllocationException("primary storage resource limit check failed", Resource.ResourceType.primary_storage)).when(_svc._resourceLimitMgr).checkResourceLimit(any(AccountVO.class), any(Resource.ResourceType.class), any(Long.class));
+        doThrow(new ResourceAllocationException("primary storage resource limit check failed", Resource.ResourceType.primary_storage)).when(_resourceLimitMgr).checkResourceLimit(any(AccountVO.class),
+                any(Resource.ResourceType.class), any(Long.class));
         UserVmVO vm = Mockito.mock(UserVmVO.class);
         VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class);
         when(volumeToAttach.getId()).thenReturn(9L);
@@ -473,14 +464,14 @@ public class VolumeApiServiceImplTest {
         when(vm.getType()).thenReturn(VirtualMachine.Type.User);
         when(vm.getState()).thenReturn(State.Running);
         when(vm.getDataCenterId()).thenReturn(34L);
-        when(_svc._volsDao.findByInstanceAndType(anyLong(), any(Volume.Type.class))).thenReturn(new ArrayList(10));
+        when(_volumeDao.findByInstanceAndType(anyLong(), any(Volume.Type.class))).thenReturn(new ArrayList<>(10));
-        when(_svc.volFactory.getVolume(9L)).thenReturn(volumeToAttach);
+        when(_volFactory.getVolume(9L)).thenReturn(volumeToAttach);
         when(volumeToAttach.getState()).thenReturn(Volume.State.Uploaded);
         DataCenterVO zoneWithDisabledLocalStorage = Mockito.mock(DataCenterVO.class);
-        when(_svc._dcDao.findById(anyLong())).thenReturn(zoneWithDisabledLocalStorage);
+        when(_dcDao.findById(anyLong())).thenReturn(zoneWithDisabledLocalStorage);
         when(zoneWithDisabledLocalStorage.isLocalStorageEnabled()).thenReturn(true);
         try {
-            _svc.attachVolumeToVM(2L, 9L, null);
+            volumeApiServiceImpl.attachVolumeToVM(2L, 9L, null);
         } catch (InvalidParameterValueException e) {
             Assert.assertEquals(e.getMessage(), ("primary storage resource limit check failed"));
         }

setup/db/hypervisor_capabilities.simulator.sql

@@ -16,4 +16,4 @@
 -- under the License.
 
 
-INSERT INTO `cloud`.`hypervisor_capabilities` (uuid, hypervisor_type, hypervisor_version, max_guests_limit, security_group_enabled, max_data_volumes_limit, max_hosts_per_cluster, storage_motion_supported, vm_snapshot_enabled) values (UUID(), 'Simulator', 'default', 50, 1, 6, NULL, 0, 1);
+INSERT INTO `cloud`.`hypervisor_capabilities` (uuid, hypervisor_type, hypervisor_version, max_guests_limit, security_group_enabled, max_data_volumes_limit, max_hosts_per_cluster, storage_motion_supported, vm_snapshot_enabled) values (UUID(), 'Simulator', 'default', 50, 1, 6, NULL, 1, 1);

test/integration/smoke/test_primary_storage.py

@@ -43,11 +43,7 @@ class TestPrimaryStorageServices(cloudstackTestCase):
         self.pod = get_pod(self.apiclient, self.zone.id)
         self.hypervisor = self.testClient.getHypervisorInfo()
         self.domain = get_domain(self.apiclient)
-        self.template = get_template(
+        self.template = get_template(self.apiclient, self.zone.id, self.services["ostype"])
-            self.apiclient ,
-            self.zone.id ,
-            self.services["ostype"]
-        )
 
         return
 
@@ -383,11 +379,7 @@ class TestStorageTags(cloudstackTestCase):
         cls.pod = get_pod(cls.apiclient, cls.zone.id)
         cls.hypervisor = testClient.getHypervisorInfo()
         cls.domain = get_domain(cls.apiclient)
-        cls.template = get_template(
+        cls.template = get_template(cls.apiclient, cls.zone.id, cls.services["ostype"])
-            cls.apiclient,
-            cls.zone.id,
-            cls.services["ostype"]
-        )
         cls.services["virtual_machine"]["zoneid"] = cls.zone.id
         cls.services["virtual_machine"]["template"] = cls.template.id
         cls.services["storage_tags"] = StorageTagsServices().storage_tags
@@ -477,17 +469,18 @@ class TestStorageTags(cloudstackTestCase):
         try:
             # First expunge vm, so PS can be cleaned up
             cls.virtual_machine_1.delete(cls.apiclient)
+            time.sleep(60)
 
             # Force delete primary storage
             cmd = enableStorageMaintenance.enableStorageMaintenanceCmd()
             cmd.id = cls.storage_pool_1.id
             cls.apiclient.enableStorageMaintenance(cmd)
-            time.sleep(30)
+            time.sleep(45)
             cmd = deleteStoragePool.deleteStoragePoolCmd()
             cmd.id = cls.storage_pool_1.id
             cmd.forced = True
             cls.apiclient.deleteStoragePool(cmd)
-
+            time.sleep(30)
             cleanup_resources(cls.apiclient, cls._cleanup)
         except Exception as e:
             raise Exception("Cleanup failed with %s" % e)
@@ -660,12 +653,23 @@ class TestStorageTags(cloudstackTestCase):
         if self.hypervisor.lower() not in ["vmware", "xenserver"]:
             self.virtual_machine_1.stop(self.apiclient)
 
+        volumePool = StoragePool.list(
+            self.apiclient,
+            id=vol.storageid
+        )
+        self.debug("Volume %s is on storage: %s" % (vol.id, volumePool))
+        allStoragePools = StoragePool.list(
+            self.apiclient
+        )
+        self.debug("All storage pools in the system: %s" % (allStoragePools))
         # Check migration options for volume
         pools_response = StoragePool.listForMigration(
             self.apiclient,
             id=vol.id
         )
         pools_suitable = filter(lambda p : p.suitableformigration, pools_response)
+        
+        self.debug("Suitable storage pools found: %s" % len(pools_suitable))
         self.assertEquals(1, len(pools_suitable), "Check that there is only one item on the list")
         self.assertEquals(pools_suitable[0].id, storage_pool_2.id, "Check that PS-2 is the migration option for volume")
         
@@ -682,6 +686,8 @@ class TestStorageTags(cloudstackTestCase):
             id=vol.id
         )
         pools_suitable = filter(lambda p : p.suitableformigration, pools_response)
+        
+        self.debug("Suitable storage pools found: %s" % len(pools_suitable))
         self.assertEquals(0, len(pools_suitable), "Check that there is no migration option for volume")
         
         return