Skip to content

Commit 9ea3d8c

Browse files
author
Pearl Dsilva
committed
Address comments - add early return guard to reduce indentation
1 parent 90843cb commit 9ea3d8c

4 files changed

Lines changed: 67 additions & 59 deletions

File tree

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

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3159,9 +3159,7 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy
31593159

31603160
final VirtualMachineTO to = toVmTO(profile);
31613161

3162-
if (vm.getHypervisorType() == HypervisorType.KVM && hasClvmVolumes(vm.getId())) {
3163-
executePreMigrationCommand(to, vm.getInstanceName(), srcHostId);
3164-
}
3162+
executePreMigrationCommand(vm, to, srcHostId);
31653163

31663164
final PrepareForMigrationCommand pfmc = new PrepareForMigrationCommand(to);
31673165
setVmNetworkDetails(vm, to);
@@ -3335,25 +3333,26 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy
33353333
}
33363334

33373335
private void executePostMigrationCommand(VMInstanceVO vm, VirtualMachineTO to, long dstHostId) {
3338-
if (vm.getHypervisorType() == HypervisorType.KVM && hasClvmVolumes(vm.getId())) {
3339-
try {
3340-
logger.info("Executing post-migration tasks for VM {} with CLVM volumes on destination host {}", vm.getInstanceName(), dstHostId);
3341-
final PostMigrationCommand postMigrationCommand = new PostMigrationCommand(to, vm.getInstanceName());
3342-
final Answer postMigrationAnswer = _agentMgr.send(dstHostId, postMigrationCommand);
3343-
3344-
if (postMigrationAnswer == null || !postMigrationAnswer.getResult()) {
3345-
final String details = postMigrationAnswer != null ? postMigrationAnswer.getDetails() : "null answer returned";
3346-
logger.warn("Post-migration tasks failed for VM {} on destination host {}: {}. Migration completed but some cleanup may be needed.",
3347-
vm.getInstanceName(), dstHostId, details);
3348-
} else {
3349-
logger.info("Successfully completed post-migration tasks for VM {} on destination host {}", vm.getInstanceName(), dstHostId);
3350-
}
3351-
} catch (Exception e) {
3352-
logger.warn("Exception during post-migration tasks for VM {} on destination host {}: {}. Migration completed but some cleanup may be needed.",
3353-
vm.getInstanceName(), dstHostId, e.getMessage(), e);
3336+
if (!(vm.getHypervisorType() == HypervisorType.KVM && hasClvmVolumes(vm.getId()))) {
3337+
return;
3338+
}
3339+
final String dstHostUuid = _hostDao.findById(dstHostId).getUuid();
3340+
try {
3341+
logger.info("Executing post-migration tasks for VM {} with CLVM volumes on destination host {}", vm.getInstanceName(), dstHostUuid);
3342+
final PostMigrationCommand postMigrationCommand = new PostMigrationCommand(to, vm.getInstanceName());
3343+
final Answer postMigrationAnswer = _agentMgr.send(dstHostId, postMigrationCommand);
3344+
3345+
if (postMigrationAnswer == null || !postMigrationAnswer.getResult()) {
3346+
final String details = postMigrationAnswer != null ? postMigrationAnswer.getDetails() : "null answer returned";
3347+
logger.warn("Post-migration tasks failed for VM {} on destination host {}: {}. Migration completed but some cleanup may be needed.",
3348+
vm.getInstanceName(), dstHostUuid, details);
3349+
} else {
3350+
logger.info("Successfully completed post-migration tasks for VM {} on destination host {}", vm.getInstanceName(), dstHostUuid);
33543351
}
3352+
} catch (Exception e) {
3353+
logger.warn("Exception during post-migration tasks for VM {} on destination host {}: {}. Migration completed but some cleanup may be needed.",
3354+
vm.getInstanceName(), dstHostUuid, e.getMessage(), e);
33553355
}
3356-
33573356
updateClvmLockHostForVmVolumes(vm.getId(), dstHostId);
33583357
}
33593358

@@ -4983,9 +4982,7 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
49834982

49844983
// Step 1: Send PreMigrationCommand to source host to convert CLVM volumes to shared mode
49854984
// This must happen BEFORE PrepareForMigrationCommand on destination to avoid lock conflicts
4986-
if (vm.getHypervisorType() == HypervisorType.KVM && hasClvmVolumes(vm.getId())) {
4987-
executePreMigrationCommand(to, vm.getInstanceName(), srcHostId);
4988-
}
4985+
executePreMigrationCommand(vm, to, srcHostId);
49894986

49904987
// Step 2: Send PrepareForMigrationCommand to destination host
49914988
final PrepareForMigrationCommand pfmc = new PrepareForMigrationCommand(to);
@@ -5072,6 +5069,7 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
50725069
}
50735070

50745071
migrated = true;
5072+
executePostMigrationCommand(vm, to, dstHostId);
50755073
} finally {
50765074
if (!migrated) {
50775075
logger.info("Migration was unsuccessful. Cleaning up: {}", vm);
@@ -6514,21 +6512,26 @@ private boolean hasClvmVolumes(long vmId) {
65146512
.anyMatch(pool -> pool != null && ClvmPoolManager.isClvmPoolType(pool.getPoolType()));
65156513
}
65166514

6517-
private void executePreMigrationCommand(VirtualMachineTO to, String vmInstanceName, long srcHostId) {
6518-
logger.info("Sending PreMigrationCommand to source host {} for VM {} with CLVM volumes", srcHostId, vmInstanceName);
6515+
private void executePreMigrationCommand(VMInstanceVO vm, VirtualMachineTO to, long srcHostId) {
6516+
if (!(vm.getHypervisorType() == HypervisorType.KVM && hasClvmVolumes(vm.getId()))) {
6517+
return;
6518+
}
6519+
final String vmInstanceName = vm.getInstanceName();
6520+
final String srcHostUuid = _hostDao.findById(srcHostId).getUuid();
6521+
logger.info("Sending PreMigrationCommand to source host {} for VM {} with CLVM volumes", srcHostUuid, vmInstanceName);
65196522
final PreMigrationCommand preMigCmd = new PreMigrationCommand(to, vmInstanceName);
65206523
Answer preMigAnswer = null;
65216524
try {
65226525
preMigAnswer = _agentMgr.send(srcHostId, preMigCmd);
65236526
if (preMigAnswer == null || !preMigAnswer.getResult()) {
65246527
final String details = preMigAnswer != null ? preMigAnswer.getDetails() : "null answer returned";
65256528
final String msg = "Failed to prepare source host for migration: " + details;
6526-
logger.error("Failed to prepare source host {} for migration of VM {}: {}", srcHostId, vmInstanceName, details);
6529+
logger.error("Failed to prepare source host {} for migration of VM {}: {}", srcHostUuid, vmInstanceName, details);
65276530
throw new CloudRuntimeException(msg);
65286531
}
6529-
logger.info("Successfully prepared source host {} for migration of VM {}", srcHostId, vmInstanceName);
6532+
logger.info("Successfully prepared source host {} for migration of VM {}", srcHostUuid, vmInstanceName);
65306533
} catch (final AgentUnavailableException | OperationTimedoutException e) {
6531-
logger.error("Failed to send PreMigrationCommand to source host {}: {}", srcHostId, e.getMessage(), e);
6534+
logger.error("Failed to send PreMigrationCommand to source host {}: {}", srcHostUuid, e.getMessage(), e);
65326535
throw new CloudRuntimeException("Failed to prepare source host for migration: " + e.getMessage(), e);
65336536
}
65346537
}

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -862,19 +862,20 @@ private Long getClvmLockHostFromVmVolumes(Long vmId) {
862862
}
863863

864864
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
865-
if (pool != null && ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
866-
Long lockHostId = clvmPoolManager.getClvmLockHostId(
867-
volume.getId(),
868-
volume.getUuid(),
869-
volume.getPath(),
870-
pool,
871-
true
872-
);
873-
if (lockHostId != null) {
874-
logger.debug("Found actual CLVM lock host {} from volume {} of VM {} via LVM query",
875-
lockHostId, volume.getUuid(), vmId);
876-
return lockHostId;
877-
}
865+
if (pool == null || !ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
866+
continue;
867+
}
868+
Long lockHostId = clvmPoolManager.getClvmLockHostId(
869+
volume.getId(),
870+
volume.getUuid(),
871+
volume.getPath(),
872+
pool,
873+
true
874+
);
875+
if (lockHostId != null) {
876+
logger.debug("Found actual CLVM lock host {} from volume {} of VM {} via LVM query",
877+
lockHostId, volume.getUuid(), vmId);
878+
return lockHostId;
878879
}
879880
}
880881

engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -337,23 +337,25 @@ protected Answer copyVolumeFromSnapshot(DataObject snapObj, DataObject volObj) {
337337
}
338338

339339
private void updateLockHostForVolume(EndPoint ep, DataObject volObj) {
340-
if (ep != null && volObj instanceof VolumeInfo) {
341-
VolumeInfo volumeInfo = (VolumeInfo) volObj;
342-
StoragePool destPool = (StoragePool) volObj.getDataStore();
343-
if (destPool != null && ClvmPoolManager.isClvmPoolType(destPool.getPoolType())) {
344-
Long hostId = ep.getId();
345-
Long existingHostId = clvmPoolManager.getClvmLockHostId(
346-
volumeInfo.getId(),
347-
volumeInfo.getUuid(),
348-
volumeInfo.getPath(),
349-
destPool,
350-
true
351-
);
352-
if (existingHostId == null) {
353-
clvmPoolManager.setClvmLockHostId(volumeInfo.getId(), hostId);
354-
logger.debug("Set lock host ID {} for CLVM volume {} being created from snapshot", hostId, volumeInfo.getId());
355-
}
356-
}
340+
if (ep == null || !(volObj instanceof VolumeInfo)) {
341+
return;
342+
}
343+
VolumeInfo volumeInfo = (VolumeInfo) volObj;
344+
StoragePool destPool = (StoragePool) volObj.getDataStore();
345+
if (destPool == null || !ClvmPoolManager.isClvmPoolType(destPool.getPoolType())) {
346+
return;
347+
}
348+
Long hostId = ep.getId();
349+
Long existingHostId = clvmPoolManager.getClvmLockHostId(
350+
volumeInfo.getId(),
351+
volumeInfo.getUuid(),
352+
volumeInfo.getPath(),
353+
destPool,
354+
true
355+
);
356+
if (existingHostId == null) {
357+
clvmPoolManager.setClvmLockHostId(volumeInfo.getId(), hostId);
358+
logger.debug("Set lock host ID {} for CLVM volume {} being created from snapshot", hostId, volumeInfo.getId());
357359
}
358360
}
359361

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2160,7 +2160,9 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
21602160
for (VolumeInfo vol : samePoolClvmVolumes) {
21612161
StoragePoolVO samePoolClvmPool = _storagePoolDao.findById(vol.getPoolId());
21622162
String vgName = samePoolClvmPool.getPath();
2163-
if (vgName.startsWith("/")) vgName = vgName.substring(1);
2163+
if (vgName.startsWith("/")) {
2164+
vgName = vgName.substring(1);
2165+
}
21642166
String lvPath = String.format("/dev/%s/%s", vgName, vol.getPath());
21652167
logger.info("Activating CLVM volume [{}] in shared mode on dest host [{}] for same-pool migration.",
21662168
vol.getUuid(), destHost.getId());

0 commit comments

Comments
 (0)