diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java index 682473ec94f..f93e9607090 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java @@ -30,6 +30,7 @@ import com.cloud.exception.StorageAccessException; import com.cloud.host.Host; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.offering.DiskOffering; +import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.Volume; import com.cloud.user.Account; import com.cloud.utils.Pair; @@ -123,4 +124,71 @@ public interface VolumeService { void checkAndRepairVolumeBasedOnConfig(DataObject dataObject, Host host); void validateChangeDiskOfferingEncryptionType(long existingDiskOfferingId, long newDiskOfferingId); + + /** + * Transfers exclusive lock for a volume on cluster-based storage (e.g., CLVM/CLVM_NG) from one host to another. + * This is used for storage that requires host-level lock management for volumes on shared storage pools. + * For non-CLVM pool types, this method returns false without taking action. + * + * @param volume The volume to transfer lock for + * @param sourceHostId Host currently holding the exclusive lock + * @param destHostId Host to receive the exclusive lock + * @return true if lock transfer succeeded or was not needed, false if it failed + */ + boolean transferVolumeLock(VolumeInfo volume, Long sourceHostId, Long destHostId); + + /** + * Finds which host currently has the exclusive lock on a CLVM volume. + * Checks in order: explicit lock tracking, attached VM's host, or first available cluster host. + * + * @param volume The CLVM volume + * @return Host ID that has the exclusive lock, or null if cannot be determined + */ + Long findVolumeLockHost(VolumeInfo volume); + + /** + * Performs lightweight CLVM lock migration for a volume to a target host. + * This transfers the LVM exclusive lock without copying data (CLVM volumes are on shared cluster storage). + * If the volume already has the lock on the destination host, no action is taken. + * + * @param volume The volume to migrate lock for + * @param destHostId Destination host ID + * @return Updated VolumeInfo after lock migration + */ + VolumeInfo performLockMigration(VolumeInfo volume, Long destHostId); + + /** + * Checks if both storage pools are CLVM type (CLVM or CLVM_NG). + * + * @param volumePoolType Storage pool type for the volume + * @param vmPoolType Storage pool type for the VM + * @return true if both pools are CLVM type (CLVM or CLVM_NG) + */ + boolean areBothPoolsClvmType(StoragePoolType volumePoolType, StoragePoolType vmPoolType); + + /** + * Determines if CLVM lock transfer is required when a volume is already on the correct storage pool. + * + * @param volumeToAttach The volume being attached + * @param volumePoolType Storage pool type for the volume + * @param vmPoolType Storage pool type for the VM's existing volume + * @param volumePoolId Storage pool ID for the volume + * @param vmPoolId Storage pool ID for the VM's existing volume + * @param vmHostId VM's current host ID (or last host ID if stopped) + * @return true if CLVM lock transfer is needed + */ + boolean isLockTransferRequired(VolumeInfo volumeToAttach, StoragePoolType volumePoolType, StoragePoolType vmPoolType, + Long volumePoolId, Long vmPoolId, Long vmHostId); + + /** + * Determines if lightweight CLVM migration is needed instead of full data copy. + * + * @param volumePoolType Storage pool type for the volume + * @param vmPoolType Storage pool type for the VM + * @param volumePoolPath Storage pool path for the volume + * @param vmPoolPath Storage pool path for the VM + * @return true if lightweight migration should be used + */ + boolean isLightweightMigrationNeeded(StoragePoolType volumePoolType, StoragePoolType vmPoolType, + String volumePoolPath, String vmPoolPath); } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 436f991afbd..77a7e4911bb 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -32,6 +32,8 @@ import java.util.concurrent.ExecutionException; import javax.inject.Inject; +import com.cloud.storage.ClvmLockManager; +import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -221,6 +223,8 @@ public class VolumeServiceImpl implements VolumeService { private PassphraseDao passphraseDao; @Inject protected DiskOfferingDao diskOfferingDao; + @Inject + ClvmLockManager clvmLockManager; public VolumeServiceImpl() { } @@ -2963,4 +2967,166 @@ public class VolumeServiceImpl implements VolumeService { protected String buildVolumePath(long accountId, long volumeId) { return String.format("%s/%s/%s", TemplateConstants.DEFAULT_VOLUME_ROOT_DIR, accountId, volumeId); } + + @Override + public boolean transferVolumeLock(VolumeInfo volume, Long sourceHostId, Long destHostId) { + StoragePoolVO pool = storagePoolDao.findById(volume.getPoolId()); + if (pool == null) { + logger.error("Cannot transfer volume lock for volume {}: storage pool not found", volume.getUuid()); + return false; + } + + logger.info("Transferring CLVM lock for volume {} (pool: {}) from host {} to host {}", + volume.getUuid(), pool.getName(), sourceHostId, destHostId); + + return clvmLockManager.transferClvmVolumeLock(volume.getUuid(), volume.getId(), volume.getPath(), + pool, sourceHostId, destHostId); + } + + @Override + public Long findVolumeLockHost(VolumeInfo volume) { + if (volume == null) { + logger.warn("Cannot find volume lock host: volume is null"); + return null; + } + + Long lockHostId = clvmLockManager.getClvmLockHostId(volume.getId(), volume.getUuid()); + if (lockHostId != null) { + logger.debug("Found explicit lock host {} for volume {}", lockHostId, volume.getUuid()); + return lockHostId; + } + + Long instanceId = volume.getInstanceId(); + if (instanceId != null) { + VMInstanceVO vmInstance = vmDao.findById(instanceId); + if (vmInstance != null && vmInstance.getHostId() != null) { + logger.debug("Volume {} is attached to VM {} on host {}", + volume.getUuid(), vmInstance.getUuid(), vmInstance.getHostId()); + return vmInstance.getHostId(); + } + } + + StoragePoolVO pool = storagePoolDao.findById(volume.getPoolId()); + if (pool != null && pool.getClusterId() != null) { + List hosts = _hostDao.findByClusterId(pool.getClusterId()); + if (hosts != null && !hosts.isEmpty()) { + for (HostVO host : hosts) { + if (host.getStatus() == com.cloud.host.Status.Up) { + logger.debug("Using fallback: first UP host {} in cluster {} for volume {}", + host.getId(), pool.getClusterId(), volume.getUuid()); + return host.getId(); + } + } + } + } + + logger.warn("Could not determine lock host for volume {}", volume.getUuid()); + return null; + } + + @Override + public VolumeInfo performLockMigration(VolumeInfo volume, Long destHostId) { + if (volume == null) { + throw new CloudRuntimeException("Cannot perform CLVM lock migration: volume is null"); + } + + String volumeUuid = volume.getUuid(); + logger.info("Starting CLVM lock migration for volume {} (id: {}) to host {}", + volumeUuid, volume.getUuid(), destHostId); + + Long sourceHostId = findVolumeLockHost(volume); + if (sourceHostId == null) { + logger.warn("Could not determine source host for CLVM volume {} lock, assuming volume is not exclusively locked", + volumeUuid); + sourceHostId = destHostId; + } + + if (sourceHostId.equals(destHostId)) { + logger.info("CLVM volume {} already has lock on destination host {}, no migration needed", + volumeUuid, destHostId); + return volume; + } + + logger.info("Migrating CLVM volume {} lock from host {} to host {}", + volumeUuid, sourceHostId, destHostId); + + boolean success = transferVolumeLock(volume, sourceHostId, destHostId); + if (!success) { + throw new CloudRuntimeException( + String.format("Failed to transfer CLVM lock for volume %s from host %s to host %s", + volumeUuid, sourceHostId, destHostId)); + } + + logger.info("Successfully migrated CLVM volume {} lock from host {} to host {}", + volumeUuid, sourceHostId, destHostId); + + return volFactory.getVolume(volume.getId()); + } + + @Override + public boolean areBothPoolsClvmType(StoragePoolType volumePoolType, StoragePoolType vmPoolType) { + if (volumePoolType == null || vmPoolType == null) { + logger.debug("Cannot check if both pools are CLVM type: one or both pool types are null"); + return false; + } + return ClvmLockManager.isClvmPoolType(volumePoolType) && + ClvmLockManager.isClvmPoolType(vmPoolType); + } + + @Override + public boolean isLockTransferRequired(VolumeInfo volumeToAttach, StoragePoolType volumePoolType, StoragePoolType vmPoolType, + Long volumePoolId, Long vmPoolId, Long vmHostId) { + if (volumePoolType != null && !ClvmLockManager.isClvmPoolType(volumePoolType)) { + return false; + } + + if (volumePoolId == null || !volumePoolId.equals(vmPoolId)) { + return false; + } + + Long volumeLockHostId = findVolumeLockHost(volumeToAttach); + + if (volumeLockHostId == null) { + VolumeVO volumeVO = _volumeDao.findById(volumeToAttach.getId()); + if (volumeVO != null && volumeVO.getState() == Volume.State.Ready && volumeVO.getInstanceId() == null) { + logger.debug("CLVM volume {} is detached on same pool, lock transfer may be needed", + volumeToAttach.getUuid()); + return true; + } + } + + if (volumeLockHostId != null && vmHostId != null && !volumeLockHostId.equals(vmHostId)) { + logger.info("CLVM lock transfer required: Volume {} lock is on host {} but VM is on host {}", + volumeToAttach.getUuid(), volumeLockHostId, vmHostId); + return true; + } + + return false; + } + + @Override + public boolean isLightweightMigrationNeeded(StoragePoolType volumePoolType, StoragePoolType vmPoolType, + String volumePoolPath, String vmPoolPath) { + if (!areBothPoolsClvmType(volumePoolType, vmPoolType)) { + return false; + } + + String volumeVgName = extractVgNameFromPath(volumePoolPath); + String vmVgName = extractVgNameFromPath(vmPoolPath); + + if (volumeVgName != null && volumeVgName.equals(vmVgName)) { + logger.info("CLVM lightweight migration detected: Volume is in same VG ({}), only lock transfer needed (no data copy)", volumeVgName); + return true; + } + + return false; + } + + private String extractVgNameFromPath(String poolPath) { + if (poolPath == null) { + return null; + } + return poolPath.startsWith("/") ? poolPath.substring(1) : poolPath; + } } + diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 89a94b19809..f5e9c681d9a 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2620,7 +2620,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic if (isClvmLockTransferNeeded) { // CLVM lock transfer - no data copy, no pool change needed - newVolumeOnPrimaryStorage = executeClvmLightweightMigration( + newVolumeOnPrimaryStorage = executeLightweightLockMigration( newVolumeOnPrimaryStorage, vm, existingVolumeOfVm, "CLVM lock transfer", "same pool to different host"); } else if (moveVolumeNeeded) { @@ -2634,7 +2634,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic newVolumeOnPrimaryStorage, existingVolumeOfVm, vm); if (isClvmLightweightMigration) { - newVolumeOnPrimaryStorage = executeClvmLightweightMigration( + newVolumeOnPrimaryStorage = executeLightweightLockMigration( newVolumeOnPrimaryStorage, vm, existingVolumeOfVm, "CLVM lightweight migration", "different pools, same VG"); } else { @@ -2685,43 +2685,15 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } /** - * Checks if both storage pools are CLVM type. + * Checks if both storage pools are CLVM type (CLVM or CLVM_NG). + * Delegates to VolumeService for the actual check. * * @param volumePool Storage pool for the volume * @param vmPool Storage pool for the VM - * @return true if both pools are CLVM type + * @return true if both pools are CLVM type (CLVM or CLVM_NG) */ private boolean areBothPoolsClvmType(StoragePoolVO volumePool, StoragePoolVO vmPool) { - return volumePool.getPoolType() == StoragePoolType.CLVM && - vmPool.getPoolType() == StoragePoolType.CLVM; - } - - /** - * Extracts the Volume Group (VG) name from a CLVM storage pool path. - * For CLVM, the path is typically: /vgname - * - * @param poolPath The storage pool path - * @return VG name, or null if path is null - */ - private String extractVgNameFromPath(String poolPath) { - if (poolPath == null) { - return null; - } - return poolPath.startsWith("/") ? poolPath.substring(1) : poolPath; - } - - /** - * Checks if two CLVM storage pools are in the same Volume Group. - * - * @param volumePool Storage pool for the volume - * @param vmPool Storage pool for the VM - * @return true if both pools are in the same VG - */ - private boolean arePoolsInSameVolumeGroup(StoragePoolVO volumePool, StoragePoolVO vmPool) { - String volumeVgName = extractVgNameFromPath(volumePool.getPath()); - String vmVgName = extractVgNameFromPath(vmPool.getPath()); - - return volumeVgName != null && volumeVgName.equals(vmVgName); + return volService.areBothPoolsClvmType(volumePool.getPoolType(), vmPool.getPoolType()); } /** @@ -2746,19 +2718,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic StoragePoolVO volumePool = pools.first(); StoragePoolVO vmPool = pools.second(); - if (!areBothPoolsClvmType(volumePool, vmPool)) { - return false; - } - - if (arePoolsInSameVolumeGroup(volumePool, vmPool)) { - String vgName = extractVgNameFromPath(volumePool.getPath()); - logger.info("CLVM lightweight migration detected: Volume {} is in same VG ({}) as VM {} volumes, " + - "only lock transfer needed (no data copy)", - volumeToAttach.getUuid(), vgName, vm.getUuid()); - return true; - } - - return false; + return volService.isLightweightMigrationNeeded(volumePool.getPoolType(), vmPool.getPoolType(), + volumePool.getPath(), vmPool.getPath()); } /** @@ -2787,37 +2748,13 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic StoragePoolVO volumePool = pools.first(); StoragePoolVO vmPool = pools.second(); - if (volumePool != null && !ClvmLockManager.isClvmPoolType(volumePool.getPoolType())) { - return false; - } - - if (volumePool.getId() != vmPool.getId()) { - return false; - } - - Long volumeLockHostId = findClvmVolumeLockHost(volumeToAttach); - Long vmHostId = vm.getHostId(); if (vmHostId == null) { vmHostId = vm.getLastHostId(); } - if (volumeLockHostId == null) { - VolumeVO volumeVO = _volsDao.findById(volumeToAttach.getId()); - if (volumeVO != null && volumeVO.getState() == Volume.State.Ready && volumeVO.getInstanceId() == null) { - logger.debug("CLVM volume {} is detached on same pool as VM {}, lock transfer may be needed", - volumeToAttach.getUuid(), vm.getUuid()); - return true; - } - } - - if (volumeLockHostId != null && vmHostId != null && !volumeLockHostId.equals(vmHostId)) { - logger.info("CLVM lock transfer required: Volume {} lock is on host {} but VM {} is on host {}", - volumeToAttach.getUuid(), volumeLockHostId, vm.getUuid(), vmHostId); - return true; - } - - return false; + return volService.isLockTransferRequired(volumeToAttach, volumePool.getPoolType(), vmPool.getPoolType(), + volumePool.getId(), vmPool.getId(), vmHostId); } /** @@ -2871,13 +2808,13 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic * @return Updated VolumeInfo after lock migration * @throws CloudRuntimeException if migration fails */ - private VolumeInfo executeClvmLightweightMigration(VolumeInfo volume, UserVmVO vm, VolumeVO vmExistingVolume, + private VolumeInfo executeLightweightLockMigration(VolumeInfo volume, UserVmVO vm, VolumeVO vmExistingVolume, String operationType, String scenarioDescription) { logger.info("Performing {} for volume {} to VM {} ({})", operationType, volume.getUuid(), vm.getUuid(), scenarioDescription); try { - return performClvmLightweightMigration(volume, vm, vmExistingVolume); + return performLightweightLockMigration(volume, vm, vmExistingVolume); } catch (Exception e) { logger.error("{} failed for volume {}: {}", operationType, volume.getUuid(), e.getMessage(), e); @@ -2887,9 +2824,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic /** * Performs lightweight CLVM lock migration for volume attachment. - * - * This transfers the LVM exclusive lock from the current host to the VM's host - * without copying data (since CLVM volumes are on cluster-wide shared storage). + * Delegates to VolumeService for the actual lock migration. * * @param volume The volume to migrate locks for * @param vm The VM to attach the volume to @@ -2897,13 +2832,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic * @return Updated VolumeInfo after lock migration * @throws Exception if lock migration fails */ - private VolumeInfo performClvmLightweightMigration(VolumeInfo volume, UserVmVO vm, VolumeVO vmExistingVolume) throws Exception { - String volumeUuid = volume.getUuid(); - Long vmId = vm.getId(); - - logger.info("Starting CLVM lightweight lock migration for volume {} (id: {}) to VM {} (id: {})", - volumeUuid, volume.getId(), vm.getUuid(), vmId); - + private VolumeInfo performLightweightLockMigration(VolumeInfo volume, UserVmVO vm, VolumeVO vmExistingVolume) throws CloudRuntimeException { Long destHostId = determineClvmLockDestinationHost(vm, vmExistingVolume); if (destHostId == null) { @@ -2911,91 +2840,25 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic "Cannot determine destination host for CLVM lock migration - VM has no host and no available cluster hosts"); } - Long sourceHostId = findClvmVolumeLockHost(volume); - - if (sourceHostId == null) { - logger.warn("Could not determine source host for CLVM volume {} lock, " + - "assuming volume is not exclusively locked", volumeUuid); - sourceHostId = destHostId; + try { + return volService.performLockMigration(volume, destHostId); + } catch (CloudRuntimeException e) { + logger.error("CLVM lock migration failed for volume {}: {}", volume.getUuid(), e.getMessage(), e); + throw e; } - - if (sourceHostId.equals(destHostId)) { - logger.info("CLVM volume {} already has lock on destination host {}, no migration needed", - volumeUuid, destHostId); - return volume; - } - - logger.info("Migrating CLVM volume {} lock from host {} to host {}", - volumeUuid, sourceHostId, destHostId); - - boolean success = transferClvmVolumeLock(volume, sourceHostId, destHostId); - - if (!success) { - throw new CloudRuntimeException( - String.format("Failed to transfer CLVM lock for volume %s from host %s to host %s", - volumeUuid, sourceHostId, destHostId)); - } - - logger.info("Successfully migrated CLVM volume {} lock from host {} to host {}", - volumeUuid, sourceHostId, destHostId); - - return volFactory.getVolume(volume.getId()); } /** * Finds which host currently has the exclusive lock on a CLVM volume. + * Delegates to VolumeService for the actual lookup. * * @param volume The CLVM volume * @return Host ID that has the exclusive lock, or null if cannot be determined */ - private Long findClvmVolumeLockHost(VolumeInfo volume) { - Long lockHostId = clvmLockManager.getClvmLockHostId(volume.getId(), volume.getUuid()); - if (lockHostId != null) { - return lockHostId; - } - - Long instanceId = volume.getInstanceId(); - if (instanceId != null) { - VMInstanceVO vmInstance = _vmInstanceDao.findById(instanceId); - if (vmInstance != null && vmInstance.getHostId() != null) { - return vmInstance.getHostId(); - } - } - - StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId()); - if (pool != null && pool.getClusterId() != null) { - List hosts = _hostDao.findByClusterId(pool.getClusterId()); - if (hosts != null && !hosts.isEmpty()) { - // Return first available UP host - for (HostVO host : hosts) { - if (host.getStatus() == Status.Up) { - return host.getId(); - } - } - } - } - - return null; + private Long findVolumeLockHost(VolumeInfo volume) { + return volService.findVolumeLockHost(volume); } - /** - * Transfers CLVM volume exclusive lock from source host to destination host. - * - * @param volume The volume to transfer lock for - * @param sourceHostId Host currently holding the lock - * @param destHostId Host to transfer lock to - * @return true if successful, false otherwise - */ - private boolean transferClvmVolumeLock(VolumeInfo volume, Long sourceHostId, Long destHostId) { - StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId()); - if (pool == null) { - logger.error("Cannot find storage pool for volume {}", volume.getUuid()); - return false; - } - - return clvmLockManager.transferClvmVolumeLock(volume.getUuid(), volume.getId(), - volume.getPath(), pool, sourceHostId, destHostId); - } public Volume attachVolumeToVM(Long vmId, Long volumeId, Long deviceId, Boolean allowAttachForSharedFS) { Account caller = CallContext.current().getCallingAccount();