Repository: cloudstack Updated Branches: refs/heads/master b8adb96ae -> 245b7f4c3
CLOUDSTACK-6510: Fix gson serialization exception in storage migration. Gson couldn't serialize a map with volume and storagepool objects for logging. Fixed by using volume and storage pool ids instead of objects in the map. Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/245b7f4c Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/245b7f4c Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/245b7f4c Branch: refs/heads/master Commit: 245b7f4c39ab8bdb656b733afc4cfc55782d36eb Parents: b8adb96 Author: Devdeep Singh <devd...@gmail.com> Authored: Mon May 5 16:59:39 2014 +0530 Committer: Devdeep Singh <devd...@gmail.com> Committed: Thu May 8 12:23:46 2014 +0530 ---------------------------------------------------------------------- .../src/com/cloud/vm/VirtualMachineManager.java | 3 +- .../com/cloud/vm/VirtualMachineManagerImpl.java | 61 ++++++++++++-------- .../com/cloud/vm/VmWorkMigrateWithStorage.java | 9 +-- .../cloud/vm/VirtualMachineManagerImplTest.java | 10 ++-- server/src/com/cloud/vm/UserVmManagerImpl.java | 4 +- 5 files changed, 48 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/245b7f4c/engine/api/src/com/cloud/vm/VirtualMachineManager.java ---------------------------------------------------------------------- diff --git a/engine/api/src/com/cloud/vm/VirtualMachineManager.java b/engine/api/src/com/cloud/vm/VirtualMachineManager.java index f070210..b1e5258 100644 --- a/engine/api/src/com/cloud/vm/VirtualMachineManager.java +++ b/engine/api/src/com/cloud/vm/VirtualMachineManager.java @@ -39,7 +39,6 @@ import com.cloud.network.Network; import com.cloud.offering.DiskOfferingInfo; import com.cloud.offering.ServiceOffering; import com.cloud.storage.StoragePool; -import com.cloud.storage.Volume; import com.cloud.template.VirtualMachineTemplate; import com.cloud.utils.component.Manager; import com.cloud.utils.fsm.NoTransitionException; @@ -113,7 +112,7 @@ public interface VirtualMachineManager extends Manager { void migrate(String vmUuid, long srcHostId, DeployDestination dest) throws ResourceUnavailableException, ConcurrentOperationException; - void migrateWithStorage(String vmUuid, long srcId, long destId, Map<Volume, StoragePool> volumeToPool) throws ResourceUnavailableException, ConcurrentOperationException; + void migrateWithStorage(String vmUuid, long srcId, long destId, Map<Long, Long> volumeToPool) throws ResourceUnavailableException, ConcurrentOperationException; void reboot(String vmUuid, Map<VirtualMachineProfile.Param, Object> params) throws InsufficientCapacityException, ResourceUnavailableException; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/245b7f4c/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ---------------------------------------------------------------------- diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java index e15d287..8ca7d1e 100755 --- a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -1963,24 +1964,26 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } - private Map<Volume, StoragePool> getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host host, Map<Volume, StoragePool> volumeToPool) { + private Map<Volume, StoragePool> getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host host, Map<Long, Long> volumeToPool) { List<VolumeVO> allVolumes = _volsDao.findUsableVolumesForInstance(profile.getId()); + Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<Volume, StoragePool> (); for (VolumeVO volume : allVolumes) { - StoragePool pool = volumeToPool.get(volume); - DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId()); + Long poolId = volumeToPool.get(Long.valueOf(volume.getId())); + StoragePoolVO pool = _storagePoolDao.findById(poolId); StoragePoolVO currentPool = _storagePoolDao.findById(volume.getPoolId()); + DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId()); if (pool != null) { // 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(pool.getId(), host.getId()) == null || pool.isLocal() != diskOffering.getUseLocalStorage()) { // Cannot find a pool for the volume. Throw an exception. throw new CloudRuntimeException("Cannot migrate volume " + volume + " to storage pool " + pool + " 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 " + + ". 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 (pool.getId() == currentPool.getId()) { - // If the pool to migrate too is the same as current pool, remove the volume from the list of - // volumes to be migrated. - volumeToPool.remove(volume); + // If the pool to migrate too is the same as current pool, the volume doesn't need to be migrated. + } else { + volumeToPoolObjectMap.put(volume, pool); } } else { // Find a suitable pool for the volume. Call the storage pool allocator to find the list of pools. @@ -1989,23 +1992,33 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac ExcludeList avoid = new ExcludeList(); boolean currentPoolAvailable = false; + List<StoragePool> poolList = new ArrayList<StoragePool>(); for (StoragePoolAllocator allocator : _storagePoolAllocators) { - List<StoragePool> poolList = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL); - if (poolList != null && !poolList.isEmpty()) { - // Volume needs to be migrated. Pick the first pool from the list. Add a mapping to migrate the - // volume to a pool only if it is required; that is the current pool on which the volume resides - // is not available on the destination host. - if (poolList.contains(currentPool)) { + List<StoragePool> poolListFromAllocator = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL); + if (poolListFromAllocator != null && !poolListFromAllocator.isEmpty()) { + poolList.addAll(poolListFromAllocator); + } + } + + if (poolList != null && !poolList.isEmpty()) { + // Volume needs to be migrated. Pick the first pool from the list. Add a mapping to migrate the + // volume to a pool only if it is required; that is the current pool on which the volume resides + // is not available on the destination host. + Iterator<StoragePool> iter = poolList.iterator(); + while (iter.hasNext()) { + if (currentPool.getId() == iter.next().getId()) { currentPoolAvailable = true; - } else { - volumeToPool.put(volume, _storagePoolDao.findByUuid(poolList.get(0).getUuid())); + break; } + } - break; + if (!currentPoolAvailable) { + volumeToPoolObjectMap.put(volume, _storagePoolDao.findByUuid(poolList.get(0).getUuid())); } } - if (!currentPoolAvailable && !volumeToPool.containsKey(volume)) { + + if (!currentPoolAvailable && !volumeToPoolObjectMap.containsKey(volume)) { // 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); @@ -2013,7 +2026,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } - return volumeToPool; + return volumeToPoolObjectMap; } private <T extends VMInstanceVO> void moveVmToMigratingState(T vm, Long hostId, ItWorkVO work) throws ConcurrentOperationException { @@ -2043,7 +2056,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } @Override - public void migrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map<Volume, StoragePool> volumeToPool) + public void migrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map<Long, Long> volumeToPool) throws ResourceUnavailableException, ConcurrentOperationException { AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext(); @@ -2087,7 +2100,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } - private void orchestrateMigrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map<Volume, StoragePool> volumeToPool) throws ResourceUnavailableException, + private void orchestrateMigrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map<Long, Long> volumeToPool) throws ResourceUnavailableException, ConcurrentOperationException { VMInstanceVO vm = _vmDao.findByUuid(vmUuid); @@ -2103,11 +2116,11 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac // Create a map of which volume should go in which storage pool. VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); - volumeToPool = getPoolListForVolumesForMigration(profile, destHost, volumeToPool); + Map<Volume, StoragePool> volumeToPoolMap = getPoolListForVolumesForMigration(profile, destHost, volumeToPool); // If none of the volumes have to be migrated, fail the call. Administrator needs to make a call for migrating // a vm and not migrating a vm with storage. - if (volumeToPool.isEmpty()) { + if (volumeToPoolMap == null || volumeToPoolMap.isEmpty()) { throw new InvalidParameterValueException("Migration of the vm " + vm + "from host " + srcHost + " to destination host " + destHost + " doesn't involve migrating the volumes."); } @@ -2137,7 +2150,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac boolean migrated = false; try { // Migrate the vm and its volume. - volumeMgr.migrateVolumes(vm, to, srcHost, destHost, volumeToPool); + volumeMgr.migrateVolumes(vm, to, srcHost, destHost, volumeToPoolMap); // Put the vm back to running state. moveVmOutofMigratingStateOnSuccess(vm, destHost.getId(), work); @@ -4717,7 +4730,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac public Outcome<VirtualMachine> migrateVmWithStorageThroughJobQueue( final String vmUuid, final long srcHostId, final long destHostId, - final Map<Volume, StoragePool> volumeToPool) { + final Map<Long, Long> volumeToPool) { final CallContext context = CallContext.current(); final User user = context.getCallingUser(); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/245b7f4c/engine/orchestration/src/com/cloud/vm/VmWorkMigrateWithStorage.java ---------------------------------------------------------------------- diff --git a/engine/orchestration/src/com/cloud/vm/VmWorkMigrateWithStorage.java b/engine/orchestration/src/com/cloud/vm/VmWorkMigrateWithStorage.java index ee30c74..52fe9f2 100644 --- a/engine/orchestration/src/com/cloud/vm/VmWorkMigrateWithStorage.java +++ b/engine/orchestration/src/com/cloud/vm/VmWorkMigrateWithStorage.java @@ -18,18 +18,15 @@ package com.cloud.vm; import java.util.Map; -import com.cloud.storage.StoragePool; -import com.cloud.storage.Volume; - public class VmWorkMigrateWithStorage extends VmWork { private static final long serialVersionUID = -5626053872453569165L; long srcHostId; long destHostId; - Map<Volume, StoragePool> volumeToPool; + Map<Long, Long> volumeToPool; public VmWorkMigrateWithStorage(long userId, long accountId, long vmId, String handlerName, long srcHostId, - long destHostId, Map<Volume, StoragePool> volumeToPool) { + long destHostId, Map<Long, Long> volumeToPool) { super(userId, accountId, vmId, handlerName); @@ -46,7 +43,7 @@ public class VmWorkMigrateWithStorage extends VmWork { return destHostId; } - public Map<Volume, StoragePool> getVolumeToPool() { + public Map<Long, Long> getVolumeToPool() { return volumeToPool; } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/245b7f4c/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java ---------------------------------------------------------------------- diff --git a/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java index f6422f4..ae07455 100644 --- a/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -89,9 +89,7 @@ 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.StoragePool; import com.cloud.storage.StoragePoolHostVO; -import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.VMTemplateVO; import com.cloud.user.Account; @@ -197,7 +195,7 @@ public class VirtualMachineManagerImplTest { @Mock HostVO _destHostMock; @Mock - Map<Volume, StoragePool> _volumeToPoolMock; + Map<Long, Long> _volumeToPoolMock; @Mock EntityManager _entityMgr; @Mock @@ -356,11 +354,13 @@ public class VirtualMachineManagerImplTest { // 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(anyLong())).thenReturn(_srcStoragePoolMock); + when(_storagePoolDao.findById(200L)).thenReturn(_srcStoragePoolMock); + when(_storagePoolDao.findById(201L)).thenReturn(_destStoragePoolMock); // Mock the volume to pool mapping. - when(_volumeToPoolMock.get(_volumeMock)).thenReturn(_destStoragePoolMock); + when(_volumeToPoolMock.get(5L)).thenReturn(201L); when(_destStoragePoolMock.getId()).thenReturn(201L); when(_srcStoragePoolMock.getId()).thenReturn(200L); when(_destStoragePoolMock.isLocal()).thenReturn(false); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/245b7f4c/server/src/com/cloud/vm/UserVmManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index 5da1875..d323e98 100755 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -4092,7 +4092,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } List<VolumeVO> vmVolumes = _volsDao.findUsableVolumesForInstance(vm.getId()); - Map<Volume, StoragePool> volToPoolObjectMap = new HashMap<Volume, StoragePool>(); + Map<Long, Long> volToPoolObjectMap = new HashMap<Long, Long>(); if (!isVMUsingLocalStorage(vm) && destinationHost.getClusterId().equals(srcHost.getClusterId())) { if (volumeToPool.isEmpty()) { // If the destination host is in the same cluster and volumes do not have to be migrated across pools @@ -4116,7 +4116,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (!vmVolumes.contains(volume)) { throw new InvalidParameterValueException("There volume " + volume + " doesn't belong to " + "the virtual machine " + vm + " that has to be migrated"); } - volToPoolObjectMap.put(volume, pool); + volToPoolObjectMap.put(Long.valueOf(volume.getId()), Long.valueOf(pool.getId())); } } }
