Skip to content

Commit d93e044

Browse files
committed
1. Create clvmlockmanager and move common code \n
2. handle attaching volumes to stopped VMs \n 3. Handle lock transfer when VM is started on another host
1 parent d8890ec commit d93e044

File tree

6 files changed

+264
-159
lines changed

6 files changed

+264
-159
lines changed

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

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151

5252

5353
import com.cloud.agent.api.PostMigrationCommand;
54+
import com.cloud.storage.ClvmLockManager;
5455
import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
5556
import org.apache.cloudstack.annotation.AnnotationService;
5657
import org.apache.cloudstack.annotation.dao.AnnotationDao;
@@ -258,7 +259,6 @@
258259
import com.cloud.storage.Volume.Type;
259260
import com.cloud.storage.VolumeApiService;
260261
import com.cloud.storage.VolumeApiServiceImpl;
261-
import com.cloud.storage.VolumeDetailVO;
262262
import com.cloud.storage.VolumeVO;
263263
import com.cloud.storage.dao.DiskOfferingDao;
264264
import com.cloud.storage.dao.GuestOSCategoryDao;
@@ -467,6 +467,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
467467
ExtensionsManager extensionsManager;
468468
@Inject
469469
ExtensionDetailsDao extensionDetailsDao;
470+
@Inject
471+
ClvmLockManager clvmLockManager;
470472

471473

472474
VmWorkJobHandlerProxy _jobHandlerProxy = new VmWorkJobHandlerProxy(this);
@@ -3381,17 +3383,7 @@ private void updateClvmLockHostForVmVolumes(long vmId, long destHostId) {
33813383
for (VolumeVO volume : volumes) {
33823384
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
33833385
if (pool != null && pool.getPoolType() == Storage.StoragePoolType.CLVM) {
3384-
VolumeDetailVO existingDetail = _volsDetailsDao.findDetail(volume.getId(), VolumeInfo.CLVM_LOCK_HOST_ID);
3385-
if (existingDetail != null) {
3386-
existingDetail.setValue(String.valueOf(destHostId));
3387-
_volsDetailsDao.update(existingDetail.getId(), existingDetail);
3388-
logger.debug("Updated CLVM_LOCK_HOST_ID for volume {} to host {} after VM {} migration",
3389-
volume.getUuid(), destHostId, vmId);
3390-
} else {
3391-
_volsDetailsDao.addDetail(volume.getId(), VolumeInfo.CLVM_LOCK_HOST_ID, String.valueOf(destHostId), false);
3392-
logger.debug("Created CLVM_LOCK_HOST_ID for volume {} with host {} after VM {} migration",
3393-
volume.getUuid(), destHostId, vmId);
3394-
}
3386+
clvmLockManager.setClvmLockHostId(volume.getId(), destHostId);
33953387
}
33963388
}
33973389
}

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

Lines changed: 101 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,10 @@
3838
import javax.inject.Inject;
3939
import javax.naming.ConfigurationException;
4040

41+
import com.cloud.agent.AgentManager;
4142
import com.cloud.deploy.DeploymentClusterPlanner;
4243
import com.cloud.exception.ResourceAllocationException;
44+
import com.cloud.storage.ClvmLockManager;
4345
import com.cloud.storage.DiskOfferingVO;
4446
import com.cloud.storage.VMTemplateVO;
4547
import com.cloud.storage.dao.VMTemplateDao;
@@ -273,6 +275,10 @@ public enum UserVmCloneType {
273275
ConfigurationDao configurationDao;
274276
@Inject
275277
VMInstanceDao vmInstanceDao;
278+
@Inject
279+
ClvmLockManager clvmLockManager;
280+
@Inject
281+
AgentManager _agentMgr;
276282

277283
@Inject
278284
protected SnapshotHelper snapshotHelper;
@@ -753,9 +759,7 @@ public VolumeInfo createVolume(VolumeInfo volumeInfo, VirtualMachine vm, Virtual
753759
hostId, volumeInfo.getUuid());
754760
volumeInfo.setDestinationHostId(hostId);
755761

756-
// Persist CLVM lock host to database immediately so it survives across VolumeInfo object recreations
757-
// and serves as both the creation routing hint and the lock host tracker
758-
setClvmLockHostId(volumeInfo.getId(), hostId);
762+
clvmLockManager.setClvmLockHostId(volumeInfo.getId(), hostId);
759763
}
760764

761765
for (int i = 0; i < 2; i++) {
@@ -799,26 +803,6 @@ private String getVolumeIdentificationInfos(Volume volume) {
799803
return String.format("uuid: %s, name: %s", volume.getUuid(), volume.getName());
800804
}
801805

802-
/**
803-
* Sets or updates the CLVM_LOCK_HOST_ID detail for a volume.
804-
* If the detail already exists, it will be updated. Otherwise, it will be created.
805-
*
806-
* @param volumeId The ID of the volume
807-
* @param hostId The host ID that holds/should hold the CLVM exclusive lock
808-
*/
809-
private void setClvmLockHostId(long volumeId, long hostId) {
810-
VolumeDetailVO existingDetail = _volDetailDao.findDetail(volumeId, VolumeInfo.CLVM_LOCK_HOST_ID);
811-
812-
if (existingDetail != null) {
813-
existingDetail.setValue(String.valueOf(hostId));
814-
_volDetailDao.update(existingDetail.getId(), existingDetail);
815-
logger.debug("Updated CLVM_LOCK_HOST_ID for volume {} to host {}", volumeId, hostId);
816-
} else {
817-
_volDetailDao.addDetail(volumeId, VolumeInfo.CLVM_LOCK_HOST_ID, String.valueOf(hostId), false);
818-
logger.debug("Created CLVM_LOCK_HOST_ID for volume {} with host {}", volumeId, hostId);
819-
}
820-
}
821-
822806
/**
823807
* Updates the CLVM_LOCK_HOST_ID for a migrated volume if applicable.
824808
* For CLVM volumes that are attached to a VM, this updates the lock host tracking
@@ -847,11 +831,81 @@ private void updateClvmLockHostAfterMigration(Volume migratedVolume, StoragePool
847831
return;
848832
}
849833

850-
setClvmLockHostId(migratedVolume.getId(), vm.getHostId());
834+
clvmLockManager.setClvmLockHostId(migratedVolume.getId(), vm.getHostId());
851835
logger.debug("Updated CLVM_LOCK_HOST_ID for {} volume {} to host {} where VM {} is running",
852836
operationType, migratedVolume.getUuid(), vm.getHostId(), vm.getInstanceName());
853837
}
854838

839+
/**
840+
* Retrieves the CLVM lock host ID from any existing volume of the specified VM.
841+
* This is useful when attaching a new volume to a stopped VM - we want to maintain
842+
* consistency by using the same host that manages the VM's other CLVM volumes.
843+
*
844+
* @param vmId The ID of the VM
845+
* @return The host ID if found, null otherwise
846+
*/
847+
private Long getClvmLockHostFromVmVolumes(Long vmId) {
848+
if (vmId == null) {
849+
return null;
850+
}
851+
852+
List<VolumeVO> vmVolumes = _volsDao.findByInstance(vmId);
853+
if (vmVolumes == null || vmVolumes.isEmpty()) {
854+
return null;
855+
}
856+
857+
for (VolumeVO volume : vmVolumes) {
858+
if (volume.getPoolId() == null) {
859+
continue;
860+
}
861+
862+
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
863+
if (pool != null && pool.getPoolType() == Storage.StoragePoolType.CLVM) {
864+
Long lockHostId = clvmLockManager.getClvmLockHostId(volume.getId(), volume.getUuid());
865+
if (lockHostId != null) {
866+
logger.debug("Found CLVM lock host {} from existing volume {} of VM {}",
867+
lockHostId, volume.getUuid(), vmId);
868+
return lockHostId;
869+
}
870+
}
871+
}
872+
873+
return null;
874+
}
875+
876+
private void transferClvmLocksForVmStart(List<VolumeVO> volumes, Long destHostId, VMInstanceVO vm) {
877+
if (volumes == null || volumes.isEmpty() || destHostId == null) {
878+
return;
879+
}
880+
881+
for (VolumeVO volume : volumes) {
882+
if (volume.getPoolId() == null) {
883+
continue;
884+
}
885+
886+
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
887+
if (pool == null || pool.getPoolType() != Storage.StoragePoolType.CLVM) {
888+
continue;
889+
}
890+
891+
Long currentLockHost = clvmLockManager.getClvmLockHostId(volume.getId(), volume.getUuid());
892+
893+
if (currentLockHost == null) {
894+
clvmLockManager.setClvmLockHostId(volume.getId(), destHostId);
895+
} else if (!currentLockHost.equals(destHostId)) {
896+
logger.info("CLVM volume {} is locked on host {} but VM {} starting on host {}. Transferring lock.",
897+
volume.getUuid(), currentLockHost, vm.getInstanceName(), destHostId);
898+
899+
if (!clvmLockManager.transferClvmVolumeLock(volume.getUuid(), volume.getId(),
900+
volume.getPath(), pool, currentLockHost, destHostId)) {
901+
throw new CloudRuntimeException(
902+
String.format("Failed to transfer CLVM lock for volume %s from host %s to host %s",
903+
volume.getUuid(), currentLockHost, destHostId));
904+
}
905+
}
906+
}
907+
}
908+
855909
public String getRandomVolumeName() {
856910
return UUID.randomUUID().toString();
857911
}
@@ -1270,10 +1324,22 @@ public VolumeInfo createVolumeOnPrimaryStorage(VirtualMachine vm, VolumeInfo vol
12701324
Long clusterId = storagePool.getClusterId();
12711325
logger.trace("storage-pool {}/{} is associated with cluster {}",storagePool.getName(), storagePool.getUuid(), clusterId);
12721326
Long hostId = vm.getHostId();
1273-
if (hostId == null && storagePool.isLocal()) {
1274-
List<StoragePoolHostVO> poolHosts = storagePoolHostDao.listByPoolId(storagePool.getId());
1275-
if (poolHosts.size() > 0) {
1276-
hostId = poolHosts.get(0).getHostId();
1327+
if (hostId == null && (storagePool.isLocal() || storagePool.getPoolType() == Storage.StoragePoolType.CLVM)) {
1328+
if (storagePool.getPoolType() == Storage.StoragePoolType.CLVM) {
1329+
hostId = getClvmLockHostFromVmVolumes(vm.getId());
1330+
if (hostId != null) {
1331+
logger.debug("Using CLVM lock host {} from VM {}'s existing volumes for new volume creation",
1332+
hostId, vm.getUuid());
1333+
}
1334+
}
1335+
1336+
if (hostId == null) {
1337+
List<StoragePoolHostVO> poolHosts = storagePoolHostDao.listByPoolId(storagePool.getId());
1338+
if (!poolHosts.isEmpty()) {
1339+
hostId = poolHosts.get(0).getHostId();
1340+
logger.debug("Selected host {} from storage pool {} for stopped VM {} volume creation",
1341+
hostId, storagePool.getUuid(), vm.getUuid());
1342+
}
12771343
}
12781344
}
12791345

@@ -1935,9 +2001,8 @@ private Pair<VolumeVO, DataStore> recreateVolume(VolumeVO vol, VirtualMachinePro
19352001
if (poolVO != null && poolVO.getPoolType() == Storage.StoragePoolType.CLVM) {
19362002
Long hostId = vm.getVirtualMachine().getHostId();
19372003
if (hostId != null) {
1938-
// Using CLVM_LOCK_HOST_ID which serves dual purpose: creation routing and lock tracking
19392004
volume.setDestinationHostId(hostId);
1940-
setClvmLockHostId(volume.getId(), hostId);
2005+
clvmLockManager.setClvmLockHostId(volume.getId(), hostId);
19412006
logger.info("CLVM pool detected during volume creation from template. Setting lock host {} for volume {} (persisted to DB) to route creation to correct host",
19422007
hostId, volume.getUuid());
19432008
}
@@ -2058,13 +2123,18 @@ public void prepare(VirtualMachineProfile vm, DeployDestination dest) throws Sto
20582123
throw new CloudRuntimeException(msg);
20592124
}
20602125

2061-
// don't allow to start vm that doesn't have a root volume
20622126
if (_volsDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT).isEmpty()) {
20632127
throw new CloudRuntimeException(String.format("ROOT volume is missing, unable to prepare volumes for the VM [%s].", vm.getVirtualMachine()));
20642128
}
20652129

20662130
List<VolumeVO> vols = _volsDao.findUsableVolumesForInstance(vm.getId());
20672131

2132+
VirtualMachine vmInstance = vm.getVirtualMachine();
2133+
VMInstanceVO vmInstanceVO = vmInstanceDao.findById(vmInstance.getId());
2134+
if (vmInstance.getState() == State.Starting && dest.getHost() != null) {
2135+
transferClvmLocksForVmStart(vols, dest.getHost().getId(), vmInstanceVO);
2136+
}
2137+
20682138
List<VolumeTask> tasks = getTasks(vols, dest.getStorageForDisks(), vm);
20692139
Volume vol = null;
20702140
PrimaryDataStore store;

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@
4444
value="#{storagePoolAllocatorsRegistry.registered}" />
4545
</bean>
4646

47+
<bean id="clvmLockManager" class="com.cloud.storage.ClvmLockManager" />
48+
4749
<bean id="storageOrchestrator"
4850
class="org.apache.cloudstack.engine.orchestration.StorageOrchestrator"/>
4951
<bean id="dataMigrationHelper"

0 commit comments

Comments
 (0)