Skip to content

Commit 746baff

Browse files
author
Pearl Dsilva
committed
renamed and re-arranged classes; addressed comments part-1
1 parent ffb8698 commit 746baff

26 files changed

Lines changed: 305 additions & 317 deletions

File tree

core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferAnswer.java renamed to core/src/main/java/org/apache/cloudstack/storage/clvm/command/ClvmLockTransferAnswer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
package org.apache.cloudstack.storage.command;
18+
package org.apache.cloudstack.storage.clvm.command;
1919

2020
import com.cloud.agent.api.Answer;
2121

core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferCommand.java renamed to core/src/main/java/org/apache/cloudstack/storage/clvm/command/ClvmLockTransferCommand.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
package org.apache.cloudstack.storage.command;
18+
package org.apache.cloudstack.storage.clvm.command;
1919

2020
import com.cloud.agent.api.Command;
2121

engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,6 @@
3131

3232
public interface VolumeInfo extends DownloadableDataInfo, Volume {
3333

34-
/**
35-
* Constant for the volume detail key that stores the host ID currently holding the CLVM exclusive lock.
36-
* This is used during lightweight lock migration to determine the source host for lock transfer.
37-
*/
38-
String CLVM_LOCK_HOST_ID = "clvmLockHostId";
39-
4034
boolean isAttachedVM();
4135

4236
void addPayload(Object data);

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

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
import javax.persistence.EntityExistsException;
5252

5353
import com.cloud.agent.api.PostMigrationCommand;
54-
import com.cloud.storage.ClvmLockManager;
54+
import com.cloud.storage.clvm.ClvmPoolManager;
5555
import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
5656
import org.apache.cloudstack.annotation.AnnotationService;
5757
import org.apache.cloudstack.annotation.dao.AnnotationDao;
@@ -469,7 +469,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
469469
@Inject
470470
ExtensionDetailsDao extensionDetailsDao;
471471
@Inject
472-
ClvmLockManager clvmLockManager;
472+
ClvmPoolManager clvmPoolManager;
473473

474474

475475
VmWorkJobHandlerProxy _jobHandlerProxy = new VmWorkJobHandlerProxy(this);
@@ -3287,26 +3287,7 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy
32873287
logger.warn("Error while checking the vm {} on host {}", vm, dest.getHost(), e);
32883288
}
32893289
migrated = true;
3290-
if (vm.getHypervisorType() == HypervisorType.KVM && hasClvmVolumes(vm.getId())) {
3291-
try {
3292-
logger.info("Executing post-migration tasks for VM {} with CLVM volumes on destination host {}", vm.getInstanceName(), dstHostId);
3293-
final PostMigrationCommand postMigrationCommand = new PostMigrationCommand(to, vm.getInstanceName());
3294-
final Answer postMigrationAnswer = _agentMgr.send(dstHostId, postMigrationCommand);
3295-
3296-
if (postMigrationAnswer == null || !postMigrationAnswer.getResult()) {
3297-
final String details = postMigrationAnswer != null ? postMigrationAnswer.getDetails() : "null answer returned";
3298-
logger.warn("Post-migration tasks failed for VM {} on destination host {}: {}. Migration completed but some cleanup may be needed.",
3299-
vm.getInstanceName(), dstHostId, details);
3300-
} else {
3301-
logger.info("Successfully completed post-migration tasks for VM {} on destination host {}", vm.getInstanceName(), dstHostId);
3302-
}
3303-
} catch (Exception e) {
3304-
logger.warn("Exception during post-migration tasks for VM {} on destination host {}: {}. Migration completed but some cleanup may be needed.",
3305-
vm.getInstanceName(), dstHostId, e.getMessage(), e);
3306-
}
3307-
}
3308-
3309-
updateClvmLockHostForVmVolumes(vm.getId(), dstHostId);
3290+
executePostMigrationCommand(vm, to, dstHostId);
33103291
} finally {
33113292
if (!migrated) {
33123293
logger.info("Migration was unsuccessful. Cleaning up: {}", vm);
@@ -3346,6 +3327,29 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy
33463327
}
33473328
}
33483329

3330+
private void executePostMigrationCommand(VMInstanceVO vm, VirtualMachineTO to, long dstHostId) {
3331+
if (vm.getHypervisorType() == HypervisorType.KVM && hasClvmVolumes(vm.getId())) {
3332+
try {
3333+
logger.info("Executing post-migration tasks for VM {} with CLVM volumes on destination host {}", vm.getInstanceName(), dstHostId);
3334+
final PostMigrationCommand postMigrationCommand = new PostMigrationCommand(to, vm.getInstanceName());
3335+
final Answer postMigrationAnswer = _agentMgr.send(dstHostId, postMigrationCommand);
3336+
3337+
if (postMigrationAnswer == null || !postMigrationAnswer.getResult()) {
3338+
final String details = postMigrationAnswer != null ? postMigrationAnswer.getDetails() : "null answer returned";
3339+
logger.warn("Post-migration tasks failed for VM {} on destination host {}: {}. Migration completed but some cleanup may be needed.",
3340+
vm.getInstanceName(), dstHostId, details);
3341+
} else {
3342+
logger.info("Successfully completed post-migration tasks for VM {} on destination host {}", vm.getInstanceName(), dstHostId);
3343+
}
3344+
} catch (Exception e) {
3345+
logger.warn("Exception during post-migration tasks for VM {} on destination host {}: {}. Migration completed but some cleanup may be needed.",
3346+
vm.getInstanceName(), dstHostId, e.getMessage(), e);
3347+
}
3348+
}
3349+
3350+
updateClvmLockHostForVmVolumes(vm.getId(), dstHostId);
3351+
}
3352+
33493353
/**
33503354
* Create and set parameters for the {@link MigrateCommand} used in the migration and scaling of VMs.
33513355
*/
@@ -3401,14 +3405,14 @@ private void updateVmPod(VMInstanceVO vm, long dstHostId) {
34013405
*/
34023406
private void updateClvmLockHostForVmVolumes(long vmId, long destHostId) {
34033407
List<VolumeVO> volumes = _volsDao.findByInstance(vmId);
3404-
if (volumes == null || volumes.isEmpty()) {
3408+
if (CollectionUtils.isEmpty(volumes)) {
34053409
return;
34063410
}
34073411

34083412
for (VolumeVO volume : volumes) {
34093413
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
3410-
if (pool != null && ClvmLockManager.isClvmPoolType(pool.getPoolType())) {
3411-
clvmLockManager.setClvmLockHostId(volume.getId(), destHostId);
3414+
if (pool != null && ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
3415+
clvmPoolManager.setClvmLockHostId(volume.getId(), destHostId);
34123416
}
34133417
}
34143418
}
@@ -6507,7 +6511,7 @@ private boolean hasClvmVolumes(long vmId) {
65076511
List<VolumeVO> volumes = _volsDao.findByInstance(vmId);
65086512
return volumes.stream()
65096513
.map(v -> _storagePoolDao.findById(v.getPoolId()))
6510-
.anyMatch(pool -> pool != null && ClvmLockManager.isClvmPoolType(pool.getPoolType()));
6514+
.anyMatch(pool -> pool != null && ClvmPoolManager.isClvmPoolType(pool.getPoolType()));
65116515
}
65126516

65136517
private void executePreMigrationCommand(VirtualMachineTO to, String vmInstanceName, long srcHostId) {

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
import com.cloud.agent.AgentManager;
4242
import com.cloud.deploy.DeploymentClusterPlanner;
4343
import com.cloud.exception.ResourceAllocationException;
44-
import com.cloud.storage.ClvmLockManager;
44+
import com.cloud.storage.clvm.ClvmPoolManager;
4545
import com.cloud.resourcelimit.ReservationHelper;
4646
import com.cloud.storage.DiskOfferingVO;
4747
import com.cloud.storage.VMTemplateVO;
@@ -278,7 +278,7 @@ public enum UserVmCloneType {
278278
@Inject
279279
VMInstanceDao vmInstanceDao;
280280
@Inject
281-
ClvmLockManager clvmLockManager;
281+
ClvmPoolManager clvmPoolManager;
282282
@Inject
283283
AgentManager _agentMgr;
284284

@@ -756,12 +756,12 @@ public VolumeInfo createVolume(VolumeInfo volumeInfo, VirtualMachine vm, Virtual
756756

757757
// For CLVM pools, set the lock host hint so volume is created on the correct host
758758
// This avoids the need for shared mode activation and improves performance
759-
if (ClvmLockManager.isClvmPoolType(pool.getPoolType()) && hostId != null) {
759+
if (ClvmPoolManager.isClvmPoolType(pool.getPoolType()) && hostId != null) {
760760
logger.info("CLVM pool detected. Setting lock host {} for volume {} to route creation to correct host",
761761
hostId, volumeInfo.getUuid());
762762
volumeInfo.setDestinationHostId(hostId);
763763

764-
clvmLockManager.setClvmLockHostId(volumeInfo.getId(), hostId);
764+
clvmPoolManager.setClvmLockHostId(volumeInfo.getId(), hostId);
765765
}
766766

767767
for (int i = 0; i < 2; i++) {
@@ -820,7 +820,7 @@ private void updateClvmLockHostAfterMigration(Volume migratedVolume, StoragePool
820820
}
821821

822822
StoragePoolVO pool = _storagePoolDao.findById(destPool.getId());
823-
if (pool == null || !ClvmLockManager.isClvmPoolType(pool.getPoolType())) {
823+
if (pool == null || !ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
824824
return;
825825
}
826826

@@ -833,7 +833,7 @@ private void updateClvmLockHostAfterMigration(Volume migratedVolume, StoragePool
833833
return;
834834
}
835835

836-
clvmLockManager.setClvmLockHostId(migratedVolume.getId(), vm.getHostId());
836+
clvmPoolManager.setClvmLockHostId(migratedVolume.getId(), vm.getHostId());
837837
logger.debug("Updated CLVM_LOCK_HOST_ID for {} volume {} to host {} where VM {} is running",
838838
operationType, migratedVolume.getUuid(), vm.getHostId(), vm.getInstanceName());
839839
}
@@ -862,8 +862,8 @@ private Long getClvmLockHostFromVmVolumes(Long vmId) {
862862
}
863863

864864
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
865-
if (pool != null && ClvmLockManager.isClvmPoolType(pool.getPoolType())) {
866-
Long lockHostId = clvmLockManager.getClvmLockHostId(
865+
if (pool != null && ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
866+
Long lockHostId = clvmPoolManager.getClvmLockHostId(
867867
volume.getId(),
868868
volume.getUuid(),
869869
volume.getPath(),
@@ -892,11 +892,11 @@ private void transferClvmLocksForVmStart(List<VolumeVO> volumes, Long destHostId
892892
}
893893

894894
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
895-
if (pool == null || !ClvmLockManager.isClvmPoolType(pool.getPoolType())) {
895+
if (pool == null || !ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
896896
continue;
897897
}
898898

899-
Long currentLockHost = clvmLockManager.getClvmLockHostId(
899+
Long currentLockHost = clvmPoolManager.getClvmLockHostId(
900900
volume.getId(),
901901
volume.getUuid(),
902902
volume.getPath(),
@@ -905,12 +905,12 @@ private void transferClvmLocksForVmStart(List<VolumeVO> volumes, Long destHostId
905905
);
906906

907907
if (currentLockHost == null) {
908-
clvmLockManager.setClvmLockHostId(volume.getId(), destHostId);
908+
clvmPoolManager.setClvmLockHostId(volume.getId(), destHostId);
909909
} else if (!currentLockHost.equals(destHostId)) {
910910
logger.info("CLVM volume {} is locked on host {} but VM {} starting on host {}. Transferring lock.",
911911
volume.getUuid(), currentLockHost, vm.getInstanceName(), destHostId);
912912

913-
if (!clvmLockManager.transferClvmVolumeLock(volume.getUuid(), volume.getId(),
913+
if (!clvmPoolManager.transferClvmVolumeLock(volume.getUuid(), volume.getId(),
914914
volume.getPath(), pool, currentLockHost, destHostId)) {
915915
throw new CloudRuntimeException(
916916
String.format("Failed to transfer CLVM lock for volume %s from host %s to host %s",
@@ -1338,8 +1338,8 @@ public VolumeInfo createVolumeOnPrimaryStorage(VirtualMachine vm, VolumeInfo vol
13381338
Long clusterId = storagePool.getClusterId();
13391339
logger.trace("storage-pool {}/{} is associated with cluster {}",storagePool.getName(), storagePool.getUuid(), clusterId);
13401340
Long hostId = vm.getHostId();
1341-
if (hostId == null && (storagePool.isLocal() || ClvmLockManager.isClvmPoolType(storagePool.getPoolType()))) {
1342-
if (ClvmLockManager.isClvmPoolType(storagePool.getPoolType())) {
1341+
if (hostId == null && (storagePool.isLocal() || ClvmPoolManager.isClvmPoolType(storagePool.getPoolType()))) {
1342+
if (ClvmPoolManager.isClvmPoolType(storagePool.getPoolType())) {
13431343
hostId = getClvmLockHostFromVmVolumes(vm.getId());
13441344
if (hostId != null) {
13451345
logger.debug("Using CLVM lock host {} from VM {}'s existing volumes for new volume creation",
@@ -2012,11 +2012,11 @@ private Pair<VolumeVO, DataStore> recreateVolume(VolumeVO vol, VirtualMachinePro
20122012
// For CLVM pools, set the destination host hint so volume is created on the correct host
20132013
// This avoids the need for shared mode activation and improves performance
20142014
StoragePoolVO poolVO = _storagePoolDao.findById(destPool.getId());
2015-
if (poolVO != null && ClvmLockManager.isClvmPoolType(poolVO.getPoolType())) {
2015+
if (poolVO != null && ClvmPoolManager.isClvmPoolType(poolVO.getPoolType())) {
20162016
Long hostId = vm.getVirtualMachine().getHostId();
20172017
if (hostId != null) {
20182018
volume.setDestinationHostId(hostId);
2019-
clvmLockManager.setClvmLockHostId(volume.getId(), hostId);
2019+
clvmPoolManager.setClvmLockHostId(volume.getId(), hostId);
20202020
logger.info("CLVM pool detected during volume creation from template. Setting lock host {} for volume {} (persisted to DB) to route creation to correct host",
20212021
hostId, volume.getUuid());
20222022
}

engine/orchestration/src/main/resources/META-INF/cloudstack/core/spring-engine-orchestration-core-context.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
value="#{storagePoolAllocatorsRegistry.registered}" />
4545
</bean>
4646

47-
<bean id="clvmLockManager" class="com.cloud.storage.ClvmLockManager" />
47+
<bean id="clvmLockManager" class="com.cloud.storage.clvm.ClvmPoolManager" />
4848

4949
<bean id="storageOrchestrator"
5050
class="org.apache.cloudstack.engine.orchestration.StorageOrchestrator"/>

0 commit comments

Comments
 (0)