Skip to content

Commit 245b7f4

Browse files
devdeepdevdeep
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 b8adb96 commit 245b7f4

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;
@@ -1963,24 +1964,26 @@ protected void migrate(VMInstanceVO vm, long srcHostId, DeployDestination dest)
19631964
}
19641965
}
19651966

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

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

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

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

2016-
return volumeToPool;
2029+
return volumeToPoolObjectMap;
20172030
}
20182031

20192032
private <T extends VMInstanceVO> void moveVmToMigratingState(T vm, Long hostId, ItWorkVO work) throws ConcurrentOperationException {
@@ -2043,7 +2056,7 @@ private <T extends VMInstanceVO> void moveVmOutofMigratingStateOnSuccess(T vm, L
20432056
}
20442057

20452058
@Override
2046-
public void migrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map<Volume, StoragePool> volumeToPool)
2059+
public void migrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map<Long, Long> volumeToPool)
20472060
throws ResourceUnavailableException, ConcurrentOperationException {
20482061

20492062
AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
@@ -2087,7 +2100,7 @@ else if (jobException instanceof Throwable)
20872100
}
20882101
}
20892102

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

20932106
VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
@@ -2103,11 +2116,11 @@ private void orchestrateMigrateWithStorage(String vmUuid, long srcHostId, long d
21032116

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

21082121
// If none of the volumes have to be migrated, fail the call. Administrator needs to make a call for migrating
21092122
// a vm and not migrating a vm with storage.
2110-
if (volumeToPool.isEmpty()) {
2123+
if (volumeToPoolMap == null || volumeToPoolMap.isEmpty()) {
21112124
throw new InvalidParameterValueException("Migration of the vm " + vm + "from host " + srcHost + " to destination host " + destHost +
21122125
" doesn't involve migrating the volumes.");
21132126
}
@@ -2137,7 +2150,7 @@ private void orchestrateMigrateWithStorage(String vmUuid, long srcHostId, long d
21372150
boolean migrated = false;
21382151
try {
21392152
// Migrate the vm and its volume.
2140-
volumeMgr.migrateVolumes(vm, to, srcHost, destHost, volumeToPool);
2153+
volumeMgr.migrateVolumes(vm, to, srcHost, destHost, volumeToPoolMap);
21412154

21422155
// Put the vm back to running state.
21432156
moveVmOutofMigratingStateOnSuccess(vm, destHost.getId(), work);
@@ -4717,7 +4730,7 @@ public Object[] doInTransaction(TransactionStatus status) {
47174730

47184731
public Outcome<VirtualMachine> migrateVmWithStorageThroughJobQueue(
47194732
final String vmUuid, final long srcHostId, final long destHostId,
4720-
final Map<Volume, StoragePool> volumeToPool) {
4733+
final Map<Long, Long> volumeToPool) {
47214734

47224735
final CallContext context = CallContext.current();
47234736
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
@@ -89,9 +89,7 @@
8989
import com.cloud.storage.dao.VolumeDao;
9090
import com.cloud.storage.Storage.ProvisioningType;
9191
import com.cloud.storage.DiskOfferingVO;
92-
import com.cloud.storage.StoragePool;
9392
import com.cloud.storage.StoragePoolHostVO;
94-
import com.cloud.storage.Volume;
9593
import com.cloud.storage.VolumeVO;
9694
import com.cloud.storage.VMTemplateVO;
9795
import com.cloud.user.Account;
@@ -197,7 +195,7 @@ public class VirtualMachineManagerImplTest {
197195
@Mock
198196
HostVO _destHostMock;
199197
@Mock
200-
Map<Volume, StoragePool> _volumeToPoolMock;
198+
Map<Long, Long> _volumeToPoolMock;
201199
@Mock
202200
EntityManager _entityMgr;
203201
@Mock
@@ -356,11 +354,13 @@ private void initializeMockConfigForMigratingVmWithVolumes() throws OperationTim
356354
// Mock the disk offering and pool objects for a volume.
357355
when(_volumeMock.getDiskOfferingId()).thenReturn(5L);
358356
when(_volumeMock.getPoolId()).thenReturn(200L);
357+
when(_volumeMock.getId()).thenReturn(5L);
359358
when(_diskOfferingDao.findById(anyLong())).thenReturn(_diskOfferingMock);
360-
when(_storagePoolDao.findById(anyLong())).thenReturn(_srcStoragePoolMock);
359+
when(_storagePoolDao.findById(200L)).thenReturn(_srcStoragePoolMock);
360+
when(_storagePoolDao.findById(201L)).thenReturn(_destStoragePoolMock);
361361

362362
// Mock the volume to pool mapping.
363-
when(_volumeToPoolMock.get(_volumeMock)).thenReturn(_destStoragePoolMock);
363+
when(_volumeToPoolMock.get(5L)).thenReturn(201L);
364364
when(_destStoragePoolMock.getId()).thenReturn(201L);
365365
when(_srcStoragePoolMock.getId()).thenReturn(200L);
366366
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
@@ -4092,7 +4092,7 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio
40924092
}
40934093

40944094
List<VolumeVO> vmVolumes = _volsDao.findUsableVolumesForInstance(vm.getId());
4095-
Map<Volume, StoragePool> volToPoolObjectMap = new HashMap<Volume, StoragePool>();
4095+
Map<Long, Long> volToPoolObjectMap = new HashMap<Long, Long>();
40964096
if (!isVMUsingLocalStorage(vm) && destinationHost.getClusterId().equals(srcHost.getClusterId())) {
40974097
if (volumeToPool.isEmpty()) {
40984098
// If the destination host is in the same cluster and volumes do not have to be migrated across pools
@@ -4116,7 +4116,7 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio
41164116
if (!vmVolumes.contains(volume)) {
41174117
throw new InvalidParameterValueException("There volume " + volume + " doesn't belong to " + "the virtual machine " + vm + " that has to be migrated");
41184118
}
4119-
volToPoolObjectMap.put(volume, pool);
4119+
volToPoolObjectMap.put(Long.valueOf(volume.getId()), Long.valueOf(pool.getId()));
41204120
}
41214121
}
41224122
}

0 commit comments

Comments
 (0)