Skip to content

Commit f0dcf47

Browse files
devdeepDaanHoogland
devdeep
authored and
DaanHoogland
committed
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.
1 parent 7bd0b92 commit f0dcf47

5 files changed

Lines changed: 48 additions & 39 deletions

File tree

‎engine/api/src/com/cloud/vm/VirtualMachineManager.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import com.cloud.offering.DiskOfferingInfo;
4040
import com.cloud.offering.ServiceOffering;
4141
import com.cloud.storage.StoragePool;
42-
import com.cloud.storage.Volume;
4342
import com.cloud.template.VirtualMachineTemplate;
4443
import com.cloud.utils.component.Manager;
4544
import com.cloud.utils.fsm.NoTransitionException;
@@ -113,7 +112,7 @@ void orchestrateStart(String vmUuid, Map<VirtualMachineProfile.Param, Object> pa
113112

114113
void migrate(String vmUuid, long srcHostId, DeployDestination dest) throws ResourceUnavailableException, ConcurrentOperationException;
115114

116-
void migrateWithStorage(String vmUuid, long srcId, long destId, Map<Volume, StoragePool> volumeToPool) throws ResourceUnavailableException, ConcurrentOperationException;
115+
void migrateWithStorage(String vmUuid, long srcId, long destId, Map<Long, Long> volumeToPool) throws ResourceUnavailableException, ConcurrentOperationException;
117116

118117
void reboot(String vmUuid, Map<VirtualMachineProfile.Param, Object> params) throws InsufficientCapacityException, ResourceUnavailableException;
119118

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

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.Date;
2727
import java.util.HashMap;
2828
import java.util.HashSet;
29+
import java.util.Iterator;
2930
import java.util.LinkedHashMap;
3031
import java.util.List;
3132
import java.util.Map;
@@ -1964,24 +1965,26 @@ protected void migrate(VMInstanceVO vm, long srcHostId, DeployDestination dest)
19641965
}
19651966
}
19661967

1967-
private Map<Volume, StoragePool> getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host host, Map<Volume, StoragePool> volumeToPool) {
1968+
private Map<Volume, StoragePool> getPoolListForVolumesForMigration(VirtualMachineProfile profile, Host host, Map<Long, Long> volumeToPool) {
19681969
List<VolumeVO> allVolumes = _volsDao.findUsableVolumesForInstance(profile.getId());
1970+
Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<Volume, StoragePool> ();
19691971
for (VolumeVO volume : allVolumes) {
1970-
StoragePool pool = volumeToPool.get(volume);
1971-
DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
1972+
Long poolId = volumeToPool.get(Long.valueOf(volume.getId()));
1973+
StoragePoolVO pool = _storagePoolDao.findById(poolId);
19721974
StoragePoolVO currentPool = _storagePoolDao.findById(volume.getPoolId());
1975+
DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
19731976
if (pool != null) {
19741977
// Check if pool is accessible from the destination host and disk offering with which the volume was
19751978
// created is compliant with the pool type.
19761979
if (_poolHostDao.findByPoolHost(pool.getId(), host.getId()) == null || pool.isLocal() != diskOffering.getUseLocalStorage()) {
19771980
// Cannot find a pool for the volume. Throw an exception.
19781981
throw new CloudRuntimeException("Cannot migrate volume " + volume + " to storage pool " + pool + " while migrating vm to host " + host +
1979-
". 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 " +
1982+
". 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 " +
19801983
"the given pool.");
19811984
} else if (pool.getId() == currentPool.getId()) {
1982-
// If the pool to migrate too is the same as current pool, remove the volume from the list of
1983-
// volumes to be migrated.
1984-
volumeToPool.remove(volume);
1985+
// If the pool to migrate too is the same as current pool, the volume doesn't need to be migrated.
1986+
} else {
1987+
volumeToPoolObjectMap.put(volume, pool);
19851988
}
19861989
} else {
19871990
// Find a suitable pool for the volume. Call the storage pool allocator to find the list of pools.
@@ -1990,31 +1993,41 @@ private Map<Volume, StoragePool> getPoolListForVolumesForMigration(VirtualMachin
19901993
ExcludeList avoid = new ExcludeList();
19911994
boolean currentPoolAvailable = false;
19921995

1996+
List<StoragePool> poolList = new ArrayList<StoragePool>();
19931997
for (StoragePoolAllocator allocator : _storagePoolAllocators) {
1994-
List<StoragePool> poolList = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL);
1995-
if (poolList != null && !poolList.isEmpty()) {
1996-
// Volume needs to be migrated. Pick the first pool from the list. Add a mapping to migrate the
1997-
// volume to a pool only if it is required; that is the current pool on which the volume resides
1998-
// is not available on the destination host.
1999-
if (poolList.contains(currentPool)) {
1998+
List<StoragePool> poolListFromAllocator = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL);
1999+
if (poolListFromAllocator != null && !poolListFromAllocator.isEmpty()) {
2000+
poolList.addAll(poolListFromAllocator);
2001+
}
2002+
}
2003+
2004+
if (poolList != null && !poolList.isEmpty()) {
2005+
// Volume needs to be migrated. Pick the first pool from the list. Add a mapping to migrate the
2006+
// volume to a pool only if it is required; that is the current pool on which the volume resides
2007+
// is not available on the destination host.
2008+
Iterator<StoragePool> iter = poolList.iterator();
2009+
while (iter.hasNext()) {
2010+
if (currentPool.getId() == iter.next().getId()) {
20002011
currentPoolAvailable = true;
2001-
} else {
2002-
volumeToPool.put(volume, _storagePoolDao.findByUuid(poolList.get(0).getUuid()));
2012+
break;
20032013
}
2014+
}
20042015

2005-
break;
2016+
if (!currentPoolAvailable) {
2017+
volumeToPoolObjectMap.put(volume, _storagePoolDao.findByUuid(poolList.get(0).getUuid()));
20062018
}
20072019
}
20082020

2009-
if (!currentPoolAvailable && !volumeToPool.containsKey(volume)) {
2021+
2022+
if (!currentPoolAvailable && !volumeToPoolObjectMap.containsKey(volume)) {
20102023
// Cannot find a pool for the volume. Throw an exception.
20112024
throw new CloudRuntimeException("Cannot find a storage pool which is available for volume " + volume + " while migrating virtual machine " +
20122025
profile.getVirtualMachine() + " to host " + host);
20132026
}
20142027
}
20152028
}
20162029

2017-
return volumeToPool;
2030+
return volumeToPoolObjectMap;
20182031
}
20192032

20202033
private <T extends VMInstanceVO> void moveVmToMigratingState(T vm, Long hostId, ItWorkVO work) throws ConcurrentOperationException {
@@ -2044,7 +2057,7 @@ private <T extends VMInstanceVO> void moveVmOutofMigratingStateOnSuccess(T vm, L
20442057
}
20452058

20462059
@Override
2047-
public void migrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map<Volume, StoragePool> volumeToPool)
2060+
public void migrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map<Long, Long> volumeToPool)
20482061
throws ResourceUnavailableException, ConcurrentOperationException {
20492062

20502063
AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
@@ -2088,7 +2101,7 @@ else if (jobException instanceof Throwable)
20882101
}
20892102
}
20902103

2091-
private void orchestrateMigrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map<Volume, StoragePool> volumeToPool) throws ResourceUnavailableException,
2104+
private void orchestrateMigrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map<Long, Long> volumeToPool) throws ResourceUnavailableException,
20922105
ConcurrentOperationException {
20932106

20942107
VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
@@ -2104,11 +2117,11 @@ private void orchestrateMigrateWithStorage(String vmUuid, long srcHostId, long d
21042117

21052118
// Create a map of which volume should go in which storage pool.
21062119
VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
2107-
volumeToPool = getPoolListForVolumesForMigration(profile, destHost, volumeToPool);
2120+
Map<Volume, StoragePool> volumeToPoolMap = getPoolListForVolumesForMigration(profile, destHost, volumeToPool);
21082121

21092122
// If none of the volumes have to be migrated, fail the call. Administrator needs to make a call for migrating
21102123
// a vm and not migrating a vm with storage.
2111-
if (volumeToPool.isEmpty()) {
2124+
if (volumeToPoolMap == null || volumeToPoolMap.isEmpty()) {
21122125
throw new InvalidParameterValueException("Migration of the vm " + vm + "from host " + srcHost + " to destination host " + destHost +
21132126
" doesn't involve migrating the volumes.");
21142127
}
@@ -2138,7 +2151,7 @@ private void orchestrateMigrateWithStorage(String vmUuid, long srcHostId, long d
21382151
boolean migrated = false;
21392152
try {
21402153
// Migrate the vm and its volume.
2141-
volumeMgr.migrateVolumes(vm, to, srcHost, destHost, volumeToPool);
2154+
volumeMgr.migrateVolumes(vm, to, srcHost, destHost, volumeToPoolMap);
21422155

21432156
// Put the vm back to running state.
21442157
moveVmOutofMigratingStateOnSuccess(vm, destHost.getId(), work);
@@ -4768,7 +4781,7 @@ public Object[] doInTransaction(TransactionStatus status) {
47684781

47694782
public Outcome<VirtualMachine> migrateVmWithStorageThroughJobQueue(
47704783
final String vmUuid, final long srcHostId, final long destHostId,
4771-
final Map<Volume, StoragePool> volumeToPool) {
4784+
final Map<Long, Long> volumeToPool) {
47724785

47734786
final CallContext context = CallContext.current();
47744787
final User user = context.getCallingUser();

‎engine/orchestration/src/com/cloud/vm/VmWorkMigrateWithStorage.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,15 @@
1818

1919
import java.util.Map;
2020

21-
import com.cloud.storage.StoragePool;
22-
import com.cloud.storage.Volume;
23-
2421
public class VmWorkMigrateWithStorage extends VmWork {
2522
private static final long serialVersionUID = -5626053872453569165L;
2623

2724
long srcHostId;
2825
long destHostId;
29-
Map<Volume, StoragePool> volumeToPool;
26+
Map<Long, Long> volumeToPool;
3027

3128
public VmWorkMigrateWithStorage(long userId, long accountId, long vmId, String handlerName, long srcHostId,
32-
long destHostId, Map<Volume, StoragePool> volumeToPool) {
29+
long destHostId, Map<Long, Long> volumeToPool) {
3330

3431
super(userId, accountId, vmId, handlerName);
3532

@@ -46,7 +43,7 @@ public long getDestHostId() {
4643
return destHostId;
4744
}
4845

49-
public Map<Volume, StoragePool> getVolumeToPool() {
46+
public Map<Long, Long> getVolumeToPool() {
5047
return volumeToPool;
5148
}
5249
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,8 @@
8484
import com.cloud.offering.ServiceOffering;
8585
import com.cloud.service.ServiceOfferingVO;
8686
import com.cloud.storage.DiskOfferingVO;
87-
import com.cloud.storage.StoragePool;
8887
import com.cloud.storage.StoragePoolHostVO;
8988
import com.cloud.storage.VMTemplateVO;
90-
import com.cloud.storage.Volume;
9189
import com.cloud.storage.VolumeVO;
9290
import com.cloud.storage.dao.DiskOfferingDao;
9391
import com.cloud.storage.dao.StoragePoolHostDao;
@@ -196,7 +194,7 @@ public class VirtualMachineManagerImplTest {
196194
@Mock
197195
HostVO _destHostMock;
198196
@Mock
199-
Map<Volume, StoragePool> _volumeToPoolMock;
197+
Map<Long, Long> _volumeToPoolMock;
200198
@Mock
201199
EntityManager _entityMgr;
202200
@Mock
@@ -355,11 +353,13 @@ private void initializeMockConfigForMigratingVmWithVolumes() throws OperationTim
355353
// Mock the disk offering and pool objects for a volume.
356354
when(_volumeMock.getDiskOfferingId()).thenReturn(5L);
357355
when(_volumeMock.getPoolId()).thenReturn(200L);
356+
when(_volumeMock.getId()).thenReturn(5L);
358357
when(_diskOfferingDao.findById(anyLong())).thenReturn(_diskOfferingMock);
359-
when(_storagePoolDao.findById(anyLong())).thenReturn(_srcStoragePoolMock);
358+
when(_storagePoolDao.findById(200L)).thenReturn(_srcStoragePoolMock);
359+
when(_storagePoolDao.findById(201L)).thenReturn(_destStoragePoolMock);
360360

361361
// Mock the volume to pool mapping.
362-
when(_volumeToPoolMock.get(_volumeMock)).thenReturn(_destStoragePoolMock);
362+
when(_volumeToPoolMock.get(5L)).thenReturn(201L);
363363
when(_destStoragePoolMock.getId()).thenReturn(201L);
364364
when(_srcStoragePoolMock.getId()).thenReturn(200L);
365365
when(_destStoragePoolMock.isLocal()).thenReturn(false);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4085,7 +4085,7 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio
40854085
}
40864086

40874087
List<VolumeVO> vmVolumes = _volsDao.findUsableVolumesForInstance(vm.getId());
4088-
Map<Volume, StoragePool> volToPoolObjectMap = new HashMap<Volume, StoragePool>();
4088+
Map<Long, Long> volToPoolObjectMap = new HashMap<Long, Long>();
40894089
if (!isVMUsingLocalStorage(vm) && destinationHost.getClusterId().equals(srcHost.getClusterId())) {
40904090
if (volumeToPool.isEmpty()) {
40914091
// If the destination host is in the same cluster and volumes do not have to be migrated across pools
@@ -4109,7 +4109,7 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio
41094109
if (!vmVolumes.contains(volume)) {
41104110
throw new InvalidParameterValueException("There volume " + volume + " doesn't belong to " + "the virtual machine " + vm + " that has to be migrated");
41114111
}
4112-
volToPoolObjectMap.put(volume, pool);
4112+
volToPoolObjectMap.put(Long.valueOf(volume.getId()), Long.valueOf(pool.getId()));
41134113
}
41144114
}
41154115
}

0 commit comments

Comments
 (0)