Skip to content

Commit 9fa2e58

Browse files
author
Mike Tutkowski
committed
Implemented review comments
1 parent c401f37 commit 9fa2e58

5 files changed

Lines changed: 49 additions & 51 deletions

File tree

‎core/src/com/cloud/agent/api/MigrateCommand.java‎

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,6 @@ public Map<String, MigrateDiskInfo> getMigrateStorage() {
5353
return migrateStorage != null ? new HashMap<>(migrateStorage) : new HashMap<String, MigrateDiskInfo>();
5454
}
5555

56-
public boolean isMigrateStorage() {
57-
return migrateStorage != null && !migrateStorage.isEmpty();
58-
}
59-
6056
public void setAutoConvergence(boolean autoConvergence) {
6157
this.autoConvergence = autoConvergence;
6258
}
@@ -100,11 +96,7 @@ public enum DiskType {
10096

10197
@Override
10298
public String toString() {
103-
switch(this) {
104-
case FILE: return "file";
105-
case BLOCK: return "block";
106-
default: throw new IllegalArgumentException();
107-
}
99+
return name().toLowerCase();
108100
}
109101
}
110102

@@ -113,11 +105,7 @@ public enum DriverType {
113105

114106
@Override
115107
public String toString() {
116-
switch(this) {
117-
case QCOW2: return "qcow2";
118-
case RAW: return "raw";
119-
default: throw new IllegalArgumentException();
120-
}
108+
return name().toLowerCase();
121109
}
122110
}
123111

@@ -126,11 +114,7 @@ public enum Source {
126114

127115
@Override
128116
public String toString() {
129-
switch(this) {
130-
case FILE: return "file";
131-
case DEV: return "dev";
132-
default: throw new IllegalArgumentException();
133-
}
117+
return name().toLowerCase();
134118
}
135119
}
136120

‎core/src/com/cloud/agent/api/StartAnswer.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ public class StartAnswer extends Answer {
2929

3030
VirtualMachineTO vm;
3131
String hostGuid;
32+
// key = an applicable IQN (ex. iqn.1998-01.com.vmware.iscsi:name1)
33+
// value = a Map with the following data:
34+
// key = PATH or IMAGE_FORMAT (defined above)
35+
// value = Example if PATH is key: UUID of VDI; Example if IMAGE_FORMAT is key: DiskTO.VHD
3236
private Map<String, Map<String, String>> _iqnToData;
3337

3438
protected StartAnswer() {

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

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import javax.inject.Inject;
4040
import javax.naming.ConfigurationException;
4141

42+
import org.apache.commons.collections.CollectionUtils;
4243
import org.apache.log4j.Logger;
4344

4445
import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
@@ -567,7 +568,7 @@ protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableExcepti
567568
// Clean up volumes based on the vm's instance id
568569
volumeMgr.cleanupVolumes(vm.getId());
569570

570-
if (hostId != null && targets != null && targets.size() > 0) {
571+
if (hostId != null && CollectionUtils.isNotEmpty(targets)) {
571572
removeDynamicTargets(hostId, targets);
572573
}
573574

@@ -612,23 +613,27 @@ private List<Map<String, String>> getTargets(Long hostId, long vmId) {
612613

613614
HostVO hostVO = _hostDao.findById(hostId);
614615

615-
if (hostVO != null && hostVO.getHypervisorType() == HypervisorType.VMware) {
616-
List<VolumeVO> volumes = _volsDao.findByInstance(vmId);
616+
if (hostVO == null || hostVO.getHypervisorType() != HypervisorType.VMware) {
617+
return targets;
618+
}
617619

618-
if (volumes != null) {
619-
for (VolumeVO volume : volumes) {
620-
StoragePoolVO storagePoolVO = _storagePoolDao.findById(volume.getPoolId());
620+
List<VolumeVO> volumes = _volsDao.findByInstance(vmId);
621621

622-
if (storagePoolVO != null && storagePoolVO.isManaged()) {
623-
Map<String, String> target = new HashMap<>();
622+
if (CollectionUtils.isEmpty(volumes)) {
623+
return targets;
624+
}
624625

625-
target.put(ModifyTargetsCommand.STORAGE_HOST, storagePoolVO.getHostAddress());
626-
target.put(ModifyTargetsCommand.STORAGE_PORT, String.valueOf(storagePoolVO.getPort()));
627-
target.put(ModifyTargetsCommand.IQN, volume.get_iScsiName());
626+
for (VolumeVO volume : volumes) {
627+
StoragePoolVO storagePoolVO = _storagePoolDao.findById(volume.getPoolId());
628628

629-
targets.add(target);
630-
}
631-
}
629+
if (storagePoolVO != null && storagePoolVO.isManaged()) {
630+
Map<String, String> target = new HashMap<>();
631+
632+
target.put(ModifyTargetsCommand.STORAGE_HOST, storagePoolVO.getHostAddress());
633+
target.put(ModifyTargetsCommand.STORAGE_PORT, String.valueOf(storagePoolVO.getPort()));
634+
target.put(ModifyTargetsCommand.IQN, volume.get_iScsiName());
635+
636+
targets.add(target);
632637
}
633638
}
634639

@@ -1419,19 +1424,21 @@ private List<Map<String, String>> getVolumesToDisconnect(VirtualMachine vm) {
14191424

14201425
List<VolumeVO> volumes = _volsDao.findByInstance(vm.getId());
14211426

1422-
if (volumes != null) {
1423-
for (VolumeVO volume : volumes) {
1424-
StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId());
1427+
if (CollectionUtils.isEmpty(volumes)) {
1428+
return volumesToDisconnect;
1429+
}
1430+
1431+
for (VolumeVO volume : volumes) {
1432+
StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId());
14251433

1426-
if (storagePool != null && storagePool.isManaged()) {
1427-
Map<String, String> info = new HashMap<>(3);
1434+
if (storagePool != null && storagePool.isManaged()) {
1435+
Map<String, String> info = new HashMap<>(3);
14281436

1429-
info.put(DiskTO.STORAGE_HOST, storagePool.getHostAddress());
1430-
info.put(DiskTO.STORAGE_PORT, String.valueOf(storagePool.getPort()));
1431-
info.put(DiskTO.IQN, volume.get_iScsiName());
1437+
info.put(DiskTO.STORAGE_HOST, storagePool.getHostAddress());
1438+
info.put(DiskTO.STORAGE_PORT, String.valueOf(storagePool.getPort()));
1439+
info.put(DiskTO.IQN, volume.get_iScsiName());
14321440

1433-
volumesToDisconnect.add(info);
1434-
}
1441+
volumesToDisconnect.add(info);
14351442
}
14361443
}
14371444

‎engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -859,7 +859,7 @@ else if (HypervisorType.VMware.equals(snapshotInfo.getHypervisorType()) || Hyper
859859

860860
LOGGER.warn(msg, ex);
861861

862-
throw new CloudRuntimeException(msg + ex.getMessage());
862+
throw new CloudRuntimeException(msg + ex.getMessage(), ex);
863863
} finally {
864864
_volumeService.revokeAccess(snapshotInfo, hostVO, srcDataStore);
865865

@@ -1336,7 +1336,7 @@ private CopyCmdAnswer copyImageToVolume(DataObject srcDataObject, VolumeInfo des
13361336

13371337
LOGGER.warn(msg, ex);
13381338

1339-
throw new CloudRuntimeException(msg + ex.getMessage());
1339+
throw new CloudRuntimeException(msg + ex.getMessage(), ex);
13401340
}
13411341
finally {
13421342
_volumeService.revokeAccess(destVolumeInfo, hostVO, destVolumeInfo.getDataStore());
@@ -1798,7 +1798,7 @@ private void handleCreateTemplateFromVolume(VolumeInfo volumeInfo, TemplateInfo
17981798

17991799
LOGGER.warn(msg, ex);
18001800

1801-
throw new CloudRuntimeException(msg + ex.getMessage());
1801+
throw new CloudRuntimeException(msg + ex.getMessage(), ex);
18021802
}
18031803
finally {
18041804
try {
@@ -2147,7 +2147,7 @@ private String migrateVolume(VolumeInfo srcVolumeInfo, VolumeInfo destVolumeInfo
21472147

21482148
LOGGER.warn(msg, ex);
21492149

2150-
throw new CloudRuntimeException(msg + ex.getMessage());
2150+
throw new CloudRuntimeException(msg + ex.getMessage(), ex);
21512151
}
21522152
}
21532153

@@ -2262,7 +2262,7 @@ private CopyCmdAnswer performCopyOfVdi(VolumeInfo volumeInfo, SnapshotInfo snaps
22622262

22632263
LOGGER.warn(msg, ex);
22642264

2265-
throw new CloudRuntimeException(msg + ex.getMessage());
2265+
throw new CloudRuntimeException(msg + ex.getMessage(), ex);
22662266
}
22672267
finally {
22682268
if (Snapshot.LocationType.PRIMARY.equals(locationType)) {

‎plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import javax.xml.transform.dom.DOMSource;
4444
import javax.xml.transform.stream.StreamResult;
4545

46+
import org.apache.commons.collections.MapUtils;
4647
import org.apache.commons.io.IOUtils;
4748
import org.apache.log4j.Logger;
4849

@@ -130,16 +131,18 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0.
130131
// delete the metadata of vm snapshots before migration
131132
vmsnapshots = libvirtComputingResource.cleanVMSnapshotMetadata(dm);
132133

133-
if (command.isMigrateStorage()) {
134-
xmlDesc = replaceStorage(xmlDesc, command.getMigrateStorage());
134+
Map<String, MigrateCommand.MigrateDiskInfo> mapMigrateStorage = command.getMigrateStorage();
135+
136+
if (MapUtils.isNotEmpty(mapMigrateStorage)) {
137+
xmlDesc = replaceStorage(xmlDesc, mapMigrateStorage);
135138
}
136139

137140
dconn = libvirtUtilitiesHelper.retrieveQemuConnection("qemu+tcp://" + command.getDestinationIp() + "/system");
138141

139142
//run migration in thread so we can monitor it
140143
s_logger.info("Live migration of instance " + vmName + " initiated");
141144
final ExecutorService executor = Executors.newFixedThreadPool(1);
142-
final Callable<Domain> worker = new MigrateKVMAsync(libvirtComputingResource, dm, dconn, xmlDesc, command.isMigrateStorage(),
145+
final Callable<Domain> worker = new MigrateKVMAsync(libvirtComputingResource, dm, dconn, xmlDesc, MapUtils.isNotEmpty(mapMigrateStorage),
143146
command.isAutoConvergence(), vmName, command.getDestinationIp());
144147
final Future<Domain> migrateThread = executor.submit(worker);
145148
executor.shutdown();

0 commit comments

Comments
 (0)