diff --git a/core/src/main/java/com/cloud/agent/api/PostMigrationAnswer.java b/core/src/main/java/com/cloud/agent/api/PostMigrationAnswer.java new file mode 100644 index 000000000000..24fdf8402029 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/PostMigrationAnswer.java @@ -0,0 +1,42 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.agent.api; + +/** + * Answer for PostMigrationCommand. + * Indicates success or failure of post-migration operations on the destination host. + */ +public class PostMigrationAnswer extends Answer { + + protected PostMigrationAnswer() { + } + + public PostMigrationAnswer(PostMigrationCommand cmd, String detail) { + super(cmd, false, detail); + } + + public PostMigrationAnswer(PostMigrationCommand cmd, Exception ex) { + super(cmd, ex); + } + + public PostMigrationAnswer(PostMigrationCommand cmd) { + super(cmd, true, null); + } +} diff --git a/core/src/main/java/com/cloud/agent/api/PostMigrationCommand.java b/core/src/main/java/com/cloud/agent/api/PostMigrationCommand.java new file mode 100644 index 000000000000..e32e6eacb344 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/PostMigrationCommand.java @@ -0,0 +1,54 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.agent.api; + +import com.cloud.agent.api.to.VirtualMachineTO; + +/** + * PostMigrationCommand is sent to the destination host after a successful VM migration. + * It performs post-migration tasks such as: + * - Claiming exclusive locks on CLVM volumes (converting from shared to exclusive mode) + * - Other post-migration cleanup operations + */ +public class PostMigrationCommand extends Command { + private VirtualMachineTO vm; + private String vmName; + + protected PostMigrationCommand() { + } + + public PostMigrationCommand(VirtualMachineTO vm, String vmName) { + this.vm = vm; + this.vmName = vmName; + } + + public VirtualMachineTO getVirtualMachine() { + return vm; + } + + public String getVmName() { + return vmName; + } + + @Override + public boolean executeInSequence() { + return true; + } +} diff --git a/core/src/main/java/com/cloud/agent/api/PreMigrationCommand.java b/core/src/main/java/com/cloud/agent/api/PreMigrationCommand.java new file mode 100644 index 000000000000..951ee46002fb --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/PreMigrationCommand.java @@ -0,0 +1,56 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.agent.api; + +import com.cloud.agent.api.to.VirtualMachineTO; + +/** + * PreMigrationCommand is sent to the source host before VM migration starts. + * It performs pre-migration tasks such as: + * - Converting CLVM volume exclusive locks to shared mode so destination host can access them + * - Other pre-migration preparation operations on the source host + * + * This command runs on the SOURCE host before PrepareForMigrationCommand runs on the DESTINATION host. + */ +public class PreMigrationCommand extends Command { + private VirtualMachineTO vm; + private String vmName; + + protected PreMigrationCommand() { + } + + public PreMigrationCommand(VirtualMachineTO vm, String vmName) { + this.vm = vm; + this.vmName = vmName; + } + + public VirtualMachineTO getVirtualMachine() { + return vm; + } + + public String getVmName() { + return vmName; + } + + @Override + public boolean executeInSequence() { + return true; + } +} diff --git a/core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferCommand.java b/core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferCommand.java new file mode 100644 index 000000000000..7d71ba78509b --- /dev/null +++ b/core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferCommand.java @@ -0,0 +1,97 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.storage.command; + +import com.cloud.agent.api.Command; + +/** + * Command to transfer CLVM (Clustered LVM) exclusive lock between hosts. + * This enables lightweight volume migration for CLVM storage pools where volumes + * reside in the same Volume Group (VG) but need to be accessed from different hosts. + * + *

Instead of copying volume data (traditional migration), this command simply + * deactivates the LV on the source host and activates it exclusively on the destination host. + * + *

This is significantly faster (10-100x) than traditional migration and uses no network bandwidth. + */ +public class ClvmLockTransferCommand extends Command { + + /** + * Operation to perform on the CLVM volume. + * Maps to lvchange flags for LVM operations. + */ + public enum Operation { + /** Deactivate the volume on this host (-an) */ + DEACTIVATE("-an", "deactivate"), + + /** Activate the volume exclusively on this host (-aey) */ + ACTIVATE_EXCLUSIVE("-aey", "activate exclusively"), + + /** Activate the volume in shared mode on this host (-asy) */ + ACTIVATE_SHARED("-asy", "activate in shared mode"); + + private final String lvchangeFlag; + private final String description; + + Operation(String lvchangeFlag, String description) { + this.lvchangeFlag = lvchangeFlag; + this.description = description; + } + + public String getLvchangeFlag() { + return lvchangeFlag; + } + + public String getDescription() { + return description; + } + } + + private String lvPath; + private Operation operation; + private String volumeUuid; + + public ClvmLockTransferCommand() { + // For serialization + } + + public ClvmLockTransferCommand(Operation operation, String lvPath, String volumeUuid) { + this.operation = operation; + this.lvPath = lvPath; + this.volumeUuid = volumeUuid; + // Execute in sequence to ensure lock safety + setWait(30); + } + + public String getLvPath() { + return lvPath; + } + + public Operation getOperation() { + return operation; + } + + public String getVolumeUuid() { + return volumeUuid; + } + + @Override + public boolean executeInSequence() { + return true; + } +} diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java index 8b0171870765..448c00ab240d 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java @@ -31,6 +31,12 @@ public interface VolumeInfo extends DownloadableDataInfo, Volume { + /** + * Constant for the volume detail key that stores the host ID currently holding the CLVM exclusive lock. + * This is used during lightweight lock migration to determine the source host for lock transfer. + */ + String CLVM_LOCK_HOST_ID = "clvmLockHostId"; + boolean isAttachedVM(); void addPayload(Object data); @@ -103,4 +109,21 @@ public interface VolumeInfo extends DownloadableDataInfo, Volume { List getCheckpointPaths(); Set getCheckpointImageStoreUrls(); + + /** + * Gets the destination host ID hint for CLVM volume creation. + * This is used to route volume creation commands to the specific host where the VM will be deployed. + * Only applicable for CLVM storage pools to avoid shared mode activation. + * + * @return The host ID where the volume should be created, or null if not set + */ + Long getDestinationHostId(); + + /** + * Sets the destination host ID hint for CLVM volume creation. + * This should be set before volume creation when the destination host is known. + * + * @param hostId The host ID where the volume should be created + */ + void setDestinationHostId(Long hostId); } diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index e8796fb02529..f30dd9f511a2 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -50,6 +50,7 @@ import javax.persistence.EntityExistsException; +import com.cloud.agent.api.PostMigrationCommand; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -135,6 +136,7 @@ import com.cloud.agent.api.PrepareExternalProvisioningCommand; import com.cloud.agent.api.PrepareForMigrationAnswer; import com.cloud.agent.api.PrepareForMigrationCommand; +import com.cloud.agent.api.PreMigrationCommand; import com.cloud.agent.api.RebootAnswer; import com.cloud.agent.api.RebootCommand; import com.cloud.agent.api.RecreateCheckpointsCommand; @@ -3107,6 +3109,24 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy updateOverCommitRatioForVmProfile(profile, dest.getHost().getClusterId()); final VirtualMachineTO to = toVmTO(profile); + + logger.info("Sending PreMigrationCommand to source host {} for VM {}", srcHostId, vm.getInstanceName()); + final PreMigrationCommand preMigCmd = new PreMigrationCommand(to, vm.getInstanceName()); + Answer preMigAnswer = null; + try { + preMigAnswer = _agentMgr.send(srcHostId, preMigCmd); + if (preMigAnswer == null || !preMigAnswer.getResult()) { + final String details = preMigAnswer != null ? preMigAnswer.getDetails() : "null answer returned"; + final String msg = "Failed to prepare source host for migration: " + details; + logger.error("Failed to prepare source host {} for migration of VM {}: {}", srcHostId, vm.getInstanceName(), details); + throw new CloudRuntimeException(msg); + } + logger.info("Successfully prepared source host {} for migration of VM {}", srcHostId, vm.getInstanceName()); + } catch (final AgentUnavailableException | OperationTimedoutException e) { + logger.error("Failed to send PreMigrationCommand to source host {}: {}", srcHostId, e.getMessage(), e); + throw new CloudRuntimeException("Failed to prepare source host for migration: " + e.getMessage(), e); + } + final PrepareForMigrationCommand pfmc = new PrepareForMigrationCommand(to); setVmNetworkDetails(vm, to); @@ -3238,6 +3258,22 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy logger.warn("Error while checking the vm {} on host {}", vm, dest.getHost(), e); } migrated = true; + try { + logger.info("Executing post-migration tasks for VM {} on destination host {}", vm.getInstanceName(), dstHostId); + final PostMigrationCommand postMigrationCommand = new PostMigrationCommand(to, vm.getInstanceName()); + final Answer postMigrationAnswer = _agentMgr.send(dstHostId, postMigrationCommand); + + if (postMigrationAnswer == null || !postMigrationAnswer.getResult()) { + final String details = postMigrationAnswer != null ? postMigrationAnswer.getDetails() : "null answer returned"; + logger.warn("Post-migration tasks failed for VM {} on destination host {}: {}. Migration completed but some cleanup may be needed.", + vm.getInstanceName(), dstHostId, details); + } else { + logger.info("Successfully completed post-migration tasks for VM {} on destination host {}", vm.getInstanceName(), dstHostId); + } + } catch (Exception e) { + logger.warn("Exception during post-migration tasks for VM {} on destination host {}: {}. Migration completed but some cleanup may be needed.", + vm.getInstanceName(), dstHostId, e.getMessage(), e); + } } finally { if (!migrated) { logger.info("Migration was unsuccessful. Cleaning up: {}", vm); @@ -4897,6 +4933,27 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI volumeMgr.prepareForMigration(profile, dest); final VirtualMachineTO to = toVmTO(profile); + + // Step 1: Send PreMigrationCommand to source host to convert CLVM volumes to shared mode + // This must happen BEFORE PrepareForMigrationCommand on destination to avoid lock conflicts + logger.info("Sending PreMigrationCommand to source host {} for VM {}", srcHostId, vm.getInstanceName()); + final PreMigrationCommand preMigCmd = new PreMigrationCommand(to, vm.getInstanceName()); + Answer preMigAnswer = null; + try { + preMigAnswer = _agentMgr.send(srcHostId, preMigCmd); + if (preMigAnswer == null || !preMigAnswer.getResult()) { + final String details = preMigAnswer != null ? preMigAnswer.getDetails() : "null answer returned"; + final String msg = "Failed to prepare source host for migration: " + details; + logger.error("Failed to prepare source host {} for migration of VM {}: {}", srcHostId, vm.getInstanceName(), details); + throw new CloudRuntimeException(msg); + } + logger.info("Successfully prepared source host {} for migration of VM {}", srcHostId, vm.getInstanceName()); + } catch (final AgentUnavailableException | OperationTimedoutException e) { + logger.error("Failed to send PreMigrationCommand to source host {}: {}", srcHostId, e.getMessage(), e); + throw new CloudRuntimeException("Failed to prepare source host for migration: " + e.getMessage(), e); + } + + // Step 2: Send PrepareForMigrationCommand to destination host final PrepareForMigrationCommand pfmc = new PrepareForMigrationCommand(to); ItWorkVO work = new ItWorkVO(UUID.randomUUID().toString(), _nodeId, State.Migrating, vm.getType(), vm.getId()); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index e8c75afa81c5..edf99215422e 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -745,6 +745,19 @@ public VolumeInfo createVolume(VolumeInfo volumeInfo, VirtualMachine vm, Virtual logger.debug("Trying to create volume [{}] on storage pool [{}].", volumeToString, poolToString); DataStore store = dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary); + + // For CLVM pools, set the lock host hint so volume is created on the correct host + // This avoids the need for shared mode activation and improves performance + if (pool.getPoolType() == Storage.StoragePoolType.CLVM && hostId != null) { + logger.info("CLVM pool detected. Setting lock host {} for volume {} to route creation to correct host", + hostId, volumeInfo.getUuid()); + volumeInfo.setDestinationHostId(hostId); + + // Persist CLVM lock host to database immediately so it survives across VolumeInfo object recreations + // and serves as both the creation routing hint and the lock host tracker + setClvmLockHostId(volumeInfo.getId(), hostId); + } + for (int i = 0; i < 2; i++) { // retry one more time in case of template reload is required for Vmware case AsyncCallFuture future = null; @@ -786,6 +799,26 @@ private String getVolumeIdentificationInfos(Volume volume) { return String.format("uuid: %s, name: %s", volume.getUuid(), volume.getName()); } + /** + * Sets or updates the CLVM_LOCK_HOST_ID detail for a volume. + * If the detail already exists, it will be updated. Otherwise, it will be created. + * + * @param volumeId The ID of the volume + * @param hostId The host ID that holds/should hold the CLVM exclusive lock + */ + private void setClvmLockHostId(long volumeId, long hostId) { + VolumeDetailVO existingDetail = _volDetailDao.findDetail(volumeId, VolumeInfo.CLVM_LOCK_HOST_ID); + + if (existingDetail != null) { + existingDetail.setValue(String.valueOf(hostId)); + _volDetailDao.update(existingDetail.getId(), existingDetail); + logger.debug("Updated CLVM_LOCK_HOST_ID for volume {} to host {}", volumeId, hostId); + } else { + _volDetailDao.addDetail(volumeId, VolumeInfo.CLVM_LOCK_HOST_ID, String.valueOf(hostId), false); + logger.debug("Created CLVM_LOCK_HOST_ID for volume {} with host {}", volumeId, hostId); + } + } + public String getRandomVolumeName() { return UUID.randomUUID().toString(); } @@ -1851,6 +1884,20 @@ private Pair recreateVolume(VolumeVO vol, VirtualMachinePro future = volService.createManagedStorageVolumeFromTemplateAsync(volume, destPool.getId(), templ, hostId); } else { + // For CLVM pools, set the destination host hint so volume is created on the correct host + // This avoids the need for shared mode activation and improves performance + StoragePoolVO poolVO = _storagePoolDao.findById(destPool.getId()); + if (poolVO != null && poolVO.getPoolType() == Storage.StoragePoolType.CLVM) { + Long hostId = vm.getVirtualMachine().getHostId(); + if (hostId != null) { + // Using CLVM_LOCK_HOST_ID which serves dual purpose: creation routing and lock tracking + volume.setDestinationHostId(hostId); + setClvmLockHostId(volume.getId(), hostId); + logger.info("CLVM pool detected during volume creation from template. Setting lock host {} for volume {} (persisted to DB) to route creation to correct host", + hostId, volume.getUuid()); + } + } + future = volService.createVolumeFromTemplateAsync(volume, destPool.getId(), templ); } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java index cdf903407c17..86f492e81d27 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java @@ -148,7 +148,7 @@ public boolean configure(String name, Map params) throws Configu idStateNeqSearch = createSearchBuilder(); idStateNeqSearch.and(SNAPSHOT_ID, idStateNeqSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ); - idStateNeqSearch.and(STATE, idStateNeqSearch.entity().getState(), SearchCriteria.Op.NEQ); + idStateNeqSearch.and(STATE, idStateNeqSearch.entity().getState(), SearchCriteria.Op.NIN); idStateNeqSearch.done(); snapshotVOSearch = snapshotDao.createSearchBuilder(); diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java index 7174336113b5..9c5cea442111 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java @@ -643,6 +643,10 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat return StrategyPriority.DEFAULT; } + if (isSnapshotStoredOnSecondaryForCLVMVolume(snapshot, volumeVO)) { + return StrategyPriority.DEFAULT; + } + return StrategyPriority.CANT_HANDLE; } if (zoneId != null && SnapshotOperation.DELETE.equals(op)) { @@ -691,4 +695,32 @@ protected boolean isSnapshotStoredOnSameZoneStoreForQCOW2Volume(Snapshot snapsho dataStoreMgr.getStoreZoneId(s.getDataStoreId(), s.getRole()), volumeVO.getDataCenterId())); } + /** + * Checks if a CLVM volume snapshot is stored on secondary storage in the same zone. + * CLVM snapshots are backed up to secondary storage and removed from primary storage. + */ + protected boolean isSnapshotStoredOnSecondaryForCLVMVolume(Snapshot snapshot, VolumeVO volumeVO) { + if (volumeVO == null) { + return false; + } + + Long poolId = volumeVO.getPoolId(); + if (poolId == null) { + return false; + } + + StoragePool pool = (StoragePool) dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary); + if (pool == null || pool.getPoolType() != StoragePoolType.CLVM) { + return false; + } + + List snapshotStores = snapshotStoreDao.listReadyBySnapshot(snapshot.getId(), DataStoreRole.Image); + if (CollectionUtils.isEmpty(snapshotStores)) { + return false; + } + + return snapshotStores.stream().anyMatch(s -> Objects.equals( + dataStoreMgr.getStoreZoneId(s.getDataStoreId(), s.getRole()), volumeVO.getDataCenterId())); + } + } diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java index b71d6cf3afac..665c3a4659ca 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java @@ -27,6 +27,7 @@ import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.Snapshot; +import com.cloud.storage.Storage; import com.cloud.storage.dao.SnapshotDao; import com.cloud.vm.snapshot.VMSnapshotDetailsVO; import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao; @@ -468,6 +469,13 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) { @Override public StrategyPriority canHandle(VMSnapshot vmSnapshot) { + UserVmVO vm = userVmDao.findById(vmSnapshot.getVmId()); + String cantHandleLog = String.format("Default VM snapshot cannot handle VM snapshot for [%s]", vm); + + if (isRunningVMVolumeOnCLVMStorage(vm, cantHandleLog)) { + return StrategyPriority.CANT_HANDLE; + } + return StrategyPriority.DEFAULT; } @@ -493,10 +501,31 @@ public boolean deleteVMSnapshotFromDB(VMSnapshot vmSnapshot, boolean unmanage) { return vmSnapshotDao.remove(vmSnapshot.getId()); } + protected boolean isRunningVMVolumeOnCLVMStorage(UserVmVO vm, String cantHandleLog) { + Long vmId = vm.getId(); + if (State.Running.equals(vm.getState())) { + List volumes = volumeDao.findByInstance(vmId); + for (VolumeVO volume : volumes) { + StoragePool pool = primaryDataStoreDao.findById(volume.getPoolId()); + if (pool != null && pool.getPoolType() == Storage.StoragePoolType.CLVM) { + logger.warn("Rejecting VM snapshot request: {} - VM is running on CLVM storage (pool: {}, poolType: CLVM)", + cantHandleLog, pool.getName()); + return true; + } + } + } + return false; + } + @Override public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMemory) { UserVmVO vm = userVmDao.findById(vmId); String cantHandleLog = String.format("Default VM snapshot cannot handle VM snapshot for [%s]", vm); + + if (isRunningVMVolumeOnCLVMStorage(vm, cantHandleLog)) { + return StrategyPriority.CANT_HANDLE; + } + if (State.Running.equals(vm.getState()) && !snapshotMemory) { logger.debug("{} as it is running and its memory will not be affected.", cantHandleLog, vm); return StrategyPriority.CANT_HANDLE; diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java index e3f28a7012c2..7a9cb4601543 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java @@ -345,6 +345,13 @@ public StrategyPriority canHandle(VMSnapshot vmSnapshot) { } } + Long vmId = vmSnapshot.getVmId(); + UserVmVO vm = userVmDao.findById(vmId); + String cantHandleLog = String.format("Storage VM snapshot strategy cannot handle VM snapshot for [%s]", vm); + if (vm != null && isRunningVMVolumeOnCLVMStorage(vm, cantHandleLog)) { + return StrategyPriority.CANT_HANDLE; + } + if ( SnapshotManager.VmStorageSnapshotKvm.value() && userVm.getHypervisorType() == Hypervisor.HypervisorType.KVM && vmSnapshot.getType() == VMSnapshot.Type.Disk) { return StrategyPriority.HYPERVISOR; diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java index 061d18dc3769..02d9ae2c1306 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java @@ -32,8 +32,12 @@ import com.cloud.dc.DedicatedResourceVO; import com.cloud.dc.dao.DedicatedResourceDao; +import com.cloud.storage.Storage; +import com.cloud.storage.VolumeDetailVO; +import com.cloud.storage.dao.VolumeDetailsDao; import com.cloud.user.Account; import com.cloud.utils.Pair; +import com.cloud.utils.db.QueryBuilder; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -46,6 +50,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.storage.LocalHostEndpoint; import org.apache.cloudstack.storage.RemoteHostEndPoint; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; import org.springframework.stereotype.Component; @@ -59,8 +64,8 @@ import com.cloud.storage.DataStoreRole; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage.TemplateType; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import com.cloud.utils.db.DB; -import com.cloud.utils.db.QueryBuilder; import com.cloud.utils.db.SearchCriteria.Op; import com.cloud.utils.db.TransactionLegacy; import com.cloud.utils.exception.CloudRuntimeException; @@ -75,6 +80,10 @@ public class DefaultEndPointSelector implements EndPointSelector { private HostDao hostDao; @Inject private DedicatedResourceDao dedicatedResourceDao; + @Inject + private PrimaryDataStoreDao _storagePoolDao; + @Inject + private VolumeDetailsDao _volDetailsDao; private static final String VOL_ENCRYPT_COLUMN_NAME = "volume_encryption_support"; private final String findOneHostOnPrimaryStorage = "select t.id from " @@ -264,6 +273,16 @@ public EndPoint select(DataObject srcData, DataObject destData) { @Override public EndPoint select(DataObject srcData, DataObject destData, boolean volumeEncryptionSupportRequired) { + // FOR CLVM: Check if destination is a volume with destinationHostId hint + // This ensures template-to-volume copy is routed to the correct host for optimal lock placement + if (destData instanceof VolumeInfo) { + EndPoint clvmEndpoint = selectClvmEndpointIfApplicable((VolumeInfo) destData, "template-to-volume copy"); + if (clvmEndpoint != null) { + return clvmEndpoint; + } + } + + // Default behavior for non-CLVM or when no destination host is set DataStore srcStore = srcData.getDataStore(); DataStore destStore = destData.getDataStore(); if (moveBetweenPrimaryImage(srcStore, destStore)) { @@ -388,9 +407,60 @@ private List listUpAndConnectingSecondaryStorageVmHost(Long dcId) { return sc.list(); } + /** + * Selects endpoint for CLVM volumes with destination host hint. + * This ensures volumes are created on the correct host with exclusive locks. + * + * @param volume The volume to check for CLVM routing + * @param operation Description of the operation (for logging) + * @return EndPoint for the destination host if CLVM routing applies, null otherwise + */ + private EndPoint selectClvmEndpointIfApplicable(VolumeInfo volume, String operation) { + DataStore store = volume.getDataStore(); + + if (store.getRole() != DataStoreRole.Primary) { + return null; + } + + // Check if this is a CLVM pool + StoragePoolVO pool = _storagePoolDao.findById(store.getId()); + if (pool == null || pool.getPoolType() != Storage.StoragePoolType.CLVM) { + return null; + } + + // Check if destination host hint is set + Long destHostId = volume.getDestinationHostId(); + if (destHostId == null) { + return null; + } + + logger.info("CLVM {}: routing volume {} to destination host {} for optimal exclusive lock placement", + operation, volume.getUuid(), destHostId); + + EndPoint ep = getEndPointFromHostId(destHostId); + if (ep != null) { + return ep; + } + + logger.warn("Could not get endpoint for destination host {}, falling back to default selection", destHostId); + return null; + } + @Override public EndPoint select(DataObject object, boolean encryptionSupportRequired) { DataStore store = object.getDataStore(); + + // For CLVM volumes with destination host hint, route to that specific host + // This ensures volumes are created on the correct host with exclusive locks + if (object instanceof VolumeInfo && store.getRole() == DataStoreRole.Primary) { + VolumeInfo volInfo = (VolumeInfo) object; + EndPoint clvmEndpoint = selectClvmEndpointIfApplicable(volInfo, "volume creation"); + if (clvmEndpoint != null) { + return clvmEndpoint; + } + } + + // Default behavior for non-CLVM or when no destination host is set if (store.getRole() == DataStoreRole.Primary) { return findEndPointInScope(store.getScope(), findOneHostOnPrimaryStorage, store.getId(), encryptionSupportRequired); } @@ -493,6 +563,31 @@ public EndPoint select(DataObject object, StorageAction action, boolean encrypti } case DELETEVOLUME: { VolumeInfo volume = (VolumeInfo) object; + + // For CLVM volumes, route to the host holding the exclusive lock + if (volume.getHypervisorType() == Hypervisor.HypervisorType.KVM) { + DataStore store = volume.getDataStore(); + if (store.getRole() == DataStoreRole.Primary) { + StoragePoolVO pool = _storagePoolDao.findById(store.getId()); + if (pool != null && pool.getPoolType() == Storage.StoragePoolType.CLVM) { + Long lockHostId = getClvmLockHostId(volume); + if (lockHostId != null) { + logger.info("Routing CLVM volume {} deletion to lock holder host {}", + volume.getUuid(), lockHostId); + EndPoint ep = getEndPointFromHostId(lockHostId); + if (ep != null) { + return ep; + } + logger.warn("Could not get endpoint for CLVM lock host {}, falling back to default selection", + lockHostId); + } else { + logger.debug("No CLVM lock host tracked for volume {}, using default endpoint selection", + volume.getUuid()); + } + } + } + } + if (volume.getHypervisorType() == Hypervisor.HypervisorType.VMware) { VirtualMachine vm = volume.getAttachedVM(); if (vm != null) { @@ -589,4 +684,24 @@ public List selectAll(DataStore store) { } return endPoints; } + + /** + * Retrieves the host ID that currently holds the exclusive lock on a CLVM volume. + * This is tracked in volume_details table for proper routing of delete operations. + * + * @param volume The CLVM volume + * @return Host ID holding the lock, or null if not tracked + */ + private Long getClvmLockHostId(VolumeInfo volume) { + VolumeDetailVO detail = _volDetailsDao.findDetail(volume.getId(), VolumeInfo.CLVM_LOCK_HOST_ID); + if (detail != null && detail.getValue() != null && !detail.getValue().isEmpty()) { + try { + return Long.parseLong(detail.getValue()); + } catch (NumberFormatException e) { + logger.warn("Invalid CLVM lock host ID in volume_details for volume {}: {}", + volume.getUuid(), detail.getValue()); + } + } + return null; + } } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java index 7de9000782ec..4485351ca389 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java @@ -43,6 +43,7 @@ import com.cloud.storage.StoragePool; import com.cloud.storage.StoragePoolHostVO; import com.cloud.storage.StorageService; +import com.cloud.storage.VolumeApiServiceImpl; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.utils.exception.CloudRuntimeException; @@ -139,6 +140,15 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep Map nfsMountOpts = storageManager.getStoragePoolNFSMountOpts(pool, null).first(); Optional.ofNullable(nfsMountOpts).ifPresent(detailsMap::putAll); + + if (pool.getPoolType() == Storage.StoragePoolType.CLVM) { + Boolean clvmSecureZeroFill = VolumeApiServiceImpl.CLVMSecureZeroFill.valueIn(poolId); + if (clvmSecureZeroFill != null) { + detailsMap.put("clvmsecurezerofill", String.valueOf(clvmSecureZeroFill)); + logger.debug("Added CLVM secure zero-fill setting: {} for storage pool: {}", clvmSecureZeroFill, pool); + } + } + ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, pool, detailsMap); cmd.setWait(modifyStoragePoolCommandWait); HostVO host = hostDao.findById(hostId); diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java index 43218b3f6a02..e8934d0ff712 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java @@ -126,6 +126,7 @@ public class VolumeObject implements VolumeInfo { private boolean directDownload; private String vSphereStoragePolicyId; private boolean followRedirects; + private Long destinationHostId; // For CLVM: hints where volume should be created private List checkpointPaths; private Set checkpointImageStoreUrls; @@ -361,6 +362,30 @@ public void setDirectDownload(boolean directDownload) { this.directDownload = directDownload; } + @Override + public Long getDestinationHostId() { + // If not in memory, try to load from database (volume_details table) + // For CLVM volumes, this uses the CLVM_LOCK_HOST_ID which serves dual purpose: + // 1. During creation: hints where to create the volume + // 2. After creation: tracks which host holds the exclusive lock + if (destinationHostId == null && volumeVO != null) { + VolumeDetailVO detail = volumeDetailsDao.findDetail(volumeVO.getId(), CLVM_LOCK_HOST_ID); + if (detail != null && detail.getValue() != null && !detail.getValue().isEmpty()) { + try { + destinationHostId = Long.parseLong(detail.getValue()); + } catch (NumberFormatException e) { + logger.warn("Invalid CLVM lock host ID value in volume_details for volume {}: {}", volumeVO.getUuid(), detail.getValue()); + } + } + } + return destinationHostId; + } + + @Override + public void setDestinationHostId(Long hostId) { + this.destinationHostId = hostId; + } + public void update() { volumeDao.update(volumeVO.getId(), volumeVO); volumeVO = volumeDao.findById(volumeVO.getId()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 0a9e0d2d98e6..92b1c7b4b6a5 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -6523,4 +6523,198 @@ public String getHypervisorPath() { public String getGuestCpuArch() { return guestCpuArch; } + + /** + * CLVM volume state for migration operations on source host + */ + public enum ClvmVolumeState { + /** Shared mode (-asy) - used before migration to allow both hosts to access volume */ + SHARED("-asy", "shared", "Before migration: activating in shared mode"), + + /** Deactivate (-an) - used after successful migration to release volume on source */ + DEACTIVATE("-an", "deactivated", "After successful migration: deactivating volume"), + + /** Exclusive mode (-aey) - used after failed migration to revert to original exclusive state */ + EXCLUSIVE("-aey", "exclusive", "After failed migration: reverting to exclusive mode"); + + private final String lvchangeFlag; + private final String description; + private final String logMessage; + + ClvmVolumeState(String lvchangeFlag, String description, String logMessage) { + this.lvchangeFlag = lvchangeFlag; + this.description = description; + this.logMessage = logMessage; + } + + public String getLvchangeFlag() { + return lvchangeFlag; + } + + public String getDescription() { + return description; + } + + public String getLogMessage() { + return logMessage; + } + } + + public static void modifyClvmVolumesStateForMigration(List disks, LibvirtComputingResource resource, + VirtualMachineTO vmSpec, ClvmVolumeState state) { + for (DiskDef disk : disks) { + if (isClvmVolume(disk, resource, vmSpec)) { + String volumePath = disk.getDiskPath(); + try { + LOGGER.info("[CLVM Migration] {} for volume [{}]", + state.getLogMessage(), volumePath); + + Script cmd = new Script("lvchange", Duration.standardSeconds(300), LOGGER); + cmd.add(state.getLvchangeFlag()); + cmd.add(volumePath); + + String result = cmd.execute(); + if (result != null) { + LOGGER.error("[CLVM Migration] Failed to set volume [{}] to {} state. Command result: {}", + volumePath, state.getDescription(), result); + } else { + LOGGER.info("[CLVM Migration] Successfully set volume [{}] to {} state.", + volumePath, state.getDescription()); + } + } catch (Exception e) { + LOGGER.error("[CLVM Migration] Exception while setting volume [{}] to {} state: {}", + volumePath, state.getDescription(), e.getMessage(), e); + } + } + } + } + + /** + * Determines if a disk is on a CLVM storage pool by checking the actual pool type from VirtualMachineTO. + * This is the most reliable method as it uses CloudStack's own storage pool information. + * + * @param disk The disk definition to check + * @param resource The LibvirtComputingResource instance (unused but kept for compatibility) + * @param vmSpec The VirtualMachineTO specification containing disk and pool information + * @return true if the disk is on a CLVM storage pool, false otherwise + */ + private static boolean isClvmVolume(DiskDef disk, LibvirtComputingResource resource, VirtualMachineTO vmSpec) { + String diskPath = disk.getDiskPath(); + if (diskPath == null || vmSpec == null) { + return false; + } + + try { + if (vmSpec.getDisks() != null) { + for (DiskTO diskTO : vmSpec.getDisks()) { + if (diskTO.getData() instanceof VolumeObjectTO) { + VolumeObjectTO volumeTO = (VolumeObjectTO) diskTO.getData(); + if (diskPath.equals(volumeTO.getPath()) || diskPath.equals(diskTO.getPath())) { + DataStoreTO dataStore = volumeTO.getDataStore(); + if (dataStore instanceof PrimaryDataStoreTO) { + PrimaryDataStoreTO primaryStore = (PrimaryDataStoreTO) dataStore; + boolean isClvm = StoragePoolType.CLVM == primaryStore.getPoolType(); + LOGGER.debug("Disk {} identified as CLVM={} via VirtualMachineTO pool type: {}", + diskPath, isClvm, primaryStore.getPoolType()); + return isClvm; + } + } + } + } + } + + // Fallback: Check VG attributes using vgs command (reliable) + // CLVM VGs have the 'c' (clustered) or 's' (shared) flag in their attributes + // Example: 'wz--ns' = shared, 'wz--n-' = not clustered + if (diskPath.startsWith("/dev/") && !diskPath.contains("/dev/mapper/")) { + String vgName = extractVolumeGroupFromPath(diskPath); + if (vgName != null) { + boolean isClustered = checkIfVolumeGroupIsClustered(vgName); + LOGGER.debug("Disk {} VG {} identified as clustered={} via vgs attribute check", + diskPath, vgName, isClustered); + return isClustered; + } + } + + } catch (Exception e) { + LOGGER.error("Error determining if volume {} is CLVM: {}", diskPath, e.getMessage(), e); + } + + return false; + } + + /** + * Extracts the volume group name from a device path. + * + * @param devicePath The device path (e.g., /dev/vgname/lvname) + * @return The volume group name, or null if cannot be determined + */ + static String extractVolumeGroupFromPath(String devicePath) { + if (devicePath == null || !devicePath.startsWith("/dev/")) { + return null; + } + + // Format: /dev// + String[] parts = devicePath.split("/"); + if (parts.length >= 3) { + return parts[2]; // ["", "dev", "vgname", ...] + } + + return null; + } + + /** + * Checks if a volume group is clustered (CLVM) by examining its attributes. + * Uses 'vgs' command to check for the clustered/shared flag in VG attributes. + * + * VG Attr format (6 characters): wz--nc or wz--ns + * Position 6: Clustered flag - 'c' = CLVM (clustered), 's' = shared (lvmlockd), '-' = not clustered + * + * @param vgName The volume group name + * @return true if the VG is clustered or shared, false otherwise + */ + static boolean checkIfVolumeGroupIsClustered(String vgName) { + if (vgName == null) { + return false; + } + + try { + // Use vgs with --noheadings and -o attr to get VG attributes + OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser(); + Script vgsCmd = new Script("vgs", 5000, LOGGER); + vgsCmd.add("--noheadings"); + vgsCmd.add("--unbuffered"); + vgsCmd.add("-o"); + vgsCmd.add("vg_attr"); + vgsCmd.add(vgName); + + String result = vgsCmd.execute(parser); + + if (result == null && parser.getLines() != null) { + String output = parser.getLines(); + if (output != null && !output.isEmpty()) { + // Parse VG attributes (format: wz--nc or wz--ns or wz--n-) + // Position 6 (0-indexed 5) indicates clustering/sharing: + // 'c' = clustered (CLVM) or 's' = shared (lvmlockd) or '-' = not clustered/shared + String vgAttr = output.trim(); + if (vgAttr.length() >= 6) { + char clusterFlag = vgAttr.charAt(5); // Position 6 (0-indexed 5) + boolean isClustered = (clusterFlag == 'c' || clusterFlag == 's'); + LOGGER.debug("VG {} has attributes '{}', cluster/shared flag '{}' = {}", + vgName, vgAttr, clusterFlag, isClustered); + return isClustered; + } else { + LOGGER.warn("VG {} attributes '{}' have unexpected format (expected 6+ chars)", vgName, vgAttr); + } + } + } else { + LOGGER.warn("Failed to get VG attributes for {}: {}", vgName, result); + } + + } catch (Exception e) { + LOGGER.debug("Error checking if VG {} is clustered: {}", vgName, e.getMessage()); + } + + return false; + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtClvmLockTransferCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtClvmLockTransferCommandWrapper.java new file mode 100644 index 000000000000..1ef8ce59b592 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtClvmLockTransferCommandWrapper.java @@ -0,0 +1,90 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package com.cloud.hypervisor.kvm.resource.wrapper; + +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LogManager; + +import com.cloud.agent.api.Answer; +import org.apache.cloudstack.storage.command.ClvmLockTransferCommand; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; +import com.cloud.utils.script.Script; + +@ResourceWrapper(handles = ClvmLockTransferCommand.class) +public class LibvirtClvmLockTransferCommandWrapper + extends CommandWrapper { + + protected Logger logger = LogManager.getLogger(getClass()); + + @Override + public Answer execute(ClvmLockTransferCommand cmd, LibvirtComputingResource serverResource) { + String lvPath = cmd.getLvPath(); + ClvmLockTransferCommand.Operation operation = cmd.getOperation(); + String volumeUuid = cmd.getVolumeUuid(); + + logger.info(String.format("Executing CLVM lock transfer: operation=%s, lv=%s, volume=%s", + operation, lvPath, volumeUuid)); + + try { + String lvchangeOpt; + String operationDesc; + switch (operation) { + case DEACTIVATE: + lvchangeOpt = "-an"; + operationDesc = "deactivated"; + break; + case ACTIVATE_EXCLUSIVE: + lvchangeOpt = "-aey"; + operationDesc = "activated exclusively"; + break; + case ACTIVATE_SHARED: + lvchangeOpt = "-asy"; + operationDesc = "activated in shared mode"; + break; + default: + return new Answer(cmd, false, "Unknown operation: " + operation); + } + + Script script = new Script("/usr/sbin/lvchange", 30000, logger); + script.add(lvchangeOpt); + script.add(lvPath); + + String result = script.execute(); + + if (result != null) { + logger.error("CLVM lock transfer failed for volume {}: {}}", + volumeUuid, result); + return new Answer(cmd, false, + String.format("lvchange %s %s failed: %s", lvchangeOpt, lvPath, result)); + } + + logger.info("Successfully executed CLVM lock transfer: {} {}} for volume {}}", + lvchangeOpt, lvPath, volumeUuid); + + return new Answer(cmd, true, + String.format("Successfully %s CLVM volume %s", operationDesc, volumeUuid)); + + } catch (Exception e) { + logger.error("Exception during CLVM lock transfer for volume {}: {}}", + volumeUuid, e.getMessage(), e); + return new Answer(cmd, false, "Exception: " + e.getMessage()); + } + } +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index 43607edc53a5..8fef202ddbc1 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -1,5 +1,3 @@ -// -// Licensed to the Apache Software Foundation (ASF) under one // or more contributor license agreements. See the NOTICE file // distributed with this work for additional information // regarding copyright ownership. The ASF licenses this file @@ -42,9 +40,15 @@ import javax.xml.transform.TransformerException; import com.cloud.agent.api.VgpuTypesInfo; +import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.GPUDeviceTO; import com.cloud.hypervisor.kvm.resource.LibvirtGpuDef; import com.cloud.hypervisor.kvm.resource.LibvirtXMLParser; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; +import com.cloud.utils.Ternary; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.collections4.CollectionUtils; @@ -69,7 +73,6 @@ import com.cloud.agent.api.MigrateAnswer; import com.cloud.agent.api.MigrateCommand; import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo; -import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.DiskTO; import com.cloud.agent.api.to.DpdkTO; import com.cloud.agent.api.to.VirtualMachineTO; @@ -82,11 +85,6 @@ import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.InterfaceDef; import com.cloud.hypervisor.kvm.resource.MigrateKVMAsync; import com.cloud.hypervisor.kvm.resource.VifDriver; -import com.cloud.resource.CommandWrapper; -import com.cloud.resource.ResourceWrapper; -import com.cloud.utils.Ternary; -import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.vm.VirtualMachine; @ResourceWrapper(handles = MigrateCommand.class) public final class LibvirtMigrateCommandWrapper extends CommandWrapper { @@ -117,7 +115,8 @@ public Answer execute(final MigrateCommand command, final LibvirtComputingResour Command.State commandState = null; List ifaces = null; - List disks; + List disks = new ArrayList<>(); + VirtualMachineTO to = null; Domain dm = null; Connect dconn = null; @@ -136,7 +135,7 @@ public Answer execute(final MigrateCommand command, final LibvirtComputingResour if (logger.isDebugEnabled()) { logger.debug(String.format("Found domain with name [%s]. Starting VM migration to host [%s].", vmName, destinationUri)); } - VirtualMachineTO to = command.getVirtualMachine(); + to = command.getVirtualMachine(); dm = conn.domainLookupByName(vmName); /* @@ -336,6 +335,12 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. logger.debug(String.format("Cleaning the disks of VM [%s] in the source pool after VM migration finished.", vmName)); } resumeDomainIfPaused(destDomain, vmName); + + // Deactivate CLVM volumes on source host after successful migration + if (to != null) { + LibvirtComputingResource.modifyClvmVolumesStateForMigration(disks, libvirtComputingResource, to, LibvirtComputingResource.ClvmVolumeState.DEACTIVATE); + } + deleteOrDisconnectDisksOnSourcePool(libvirtComputingResource, migrateDiskInfoList, disks); libvirtComputingResource.cleanOldSecretsByDiskDef(conn, disks); } @@ -382,6 +387,10 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. if (destDomain != null) { destDomain.free(); } + // Revert CLVM volumes to exclusive mode on failure + if (to != null) { + LibvirtComputingResource.modifyClvmVolumesStateForMigration(disks, libvirtComputingResource, to, LibvirtComputingResource.ClvmVolumeState.EXCLUSIVE); + } } catch (final LibvirtException e) { logger.trace("Ignoring libvirt error.", e); } @@ -681,7 +690,7 @@ protected String replaceDpdkInterfaces(String xmlDesc, Map dpdkP protected void deleteOrDisconnectDisksOnSourcePool(final LibvirtComputingResource libvirtComputingResource, final List migrateDiskInfoList, List disks) { for (DiskDef disk : disks) { - MigrateDiskInfo migrateDiskInfo = searchDiskDefOnMigrateDiskInfoList(migrateDiskInfoList, disk); + MigrateCommand.MigrateDiskInfo migrateDiskInfo = searchDiskDefOnMigrateDiskInfoList(migrateDiskInfoList, disk); if (migrateDiskInfo != null && migrateDiskInfo.isSourceDiskOnStorageFileSystem()) { deleteLocalVolume(disk.getDiskPath()); } else { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtModifyStoragePoolCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtModifyStoragePoolCommandWrapper.java index 990cefda8f33..bc22d7bfd70a 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtModifyStoragePoolCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtModifyStoragePoolCommandWrapper.java @@ -52,9 +52,19 @@ public Answer execute(final ModifyStoragePoolCommand command, final LibvirtCompu final KVMStoragePool storagepool; try { + Map poolDetails = command.getDetails(); + if (poolDetails == null) { + poolDetails = new HashMap<>(); + } + + // Ensure CLVM secure zero-fill setting has a default value if not provided by MS + if (!poolDetails.containsKey(KVMStoragePool.CLVM_SECURE_ZERO_FILL)) { + poolDetails.put(KVMStoragePool.CLVM_SECURE_ZERO_FILL, "false"); + } + storagepool = storagePoolMgr.createStoragePool(command.getPool().getUuid(), command.getPool().getHost(), command.getPool().getPort(), command.getPool().getPath(), command.getPool() - .getUserInfo(), command.getPool().getType(), command.getDetails()); + .getUserInfo(), command.getPool().getType(), poolDetails); if (storagepool == null) { return new Answer(command, false, " Failed to create storage pool"); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostMigrationCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostMigrationCommandWrapper.java new file mode 100644 index 000000000000..8e5cb83e89e9 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostMigrationCommandWrapper.java @@ -0,0 +1,83 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.hypervisor.kvm.resource.wrapper; + +import java.util.List; + +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LogManager; +import org.libvirt.Connect; +import org.libvirt.LibvirtException; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.PostMigrationAnswer; +import com.cloud.agent.api.PostMigrationCommand; +import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.resource.LibvirtConnection; +import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; + +/** + * Wrapper for PostMigrationCommand on KVM hypervisor. + * Handles post-migration tasks on the destination host after a VM has been successfully migrated. + * Primary responsibility: Convert CLVM volumes from shared mode to exclusive mode on destination. + */ +@ResourceWrapper(handles = PostMigrationCommand.class) +public final class LibvirtPostMigrationCommandWrapper extends CommandWrapper { + + protected Logger logger = LogManager.getLogger(getClass()); + + @Override + public Answer execute(final PostMigrationCommand command, final LibvirtComputingResource libvirtComputingResource) { + final VirtualMachineTO vm = command.getVirtualMachine(); + final String vmName = command.getVmName(); + + if (vm == null || vmName == null) { + return new PostMigrationAnswer(command, "VM or VM name is null"); + } + + logger.debug("Executing post-migration tasks for VM {} on destination host", vmName); + + try { + final Connect conn = LibvirtConnection.getConnectionByVmName(vmName); + + List disks = libvirtComputingResource.getDisks(conn, vmName); + logger.debug("[CLVM Post-Migration] Processing volumes for VM {} to claim exclusive locks on any CLVM volumes", vmName); + LibvirtComputingResource.modifyClvmVolumesStateForMigration( + disks, + libvirtComputingResource, + vm, + LibvirtComputingResource.ClvmVolumeState.EXCLUSIVE + ); + + logger.debug("Successfully completed post-migration tasks for VM {}", vmName); + return new PostMigrationAnswer(command); + + } catch (final LibvirtException e) { + logger.error("Libvirt error during post-migration for VM {}: {}", vmName, e.getMessage(), e); + return new PostMigrationAnswer(command, e); + } catch (final Exception e) { + logger.error("Error during post-migration for VM {}: {}", vmName, e.getMessage(), e); + return new PostMigrationAnswer(command, e); + } + } +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPreMigrationCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPreMigrationCommandWrapper.java new file mode 100644 index 000000000000..71fdb3df2316 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPreMigrationCommandWrapper.java @@ -0,0 +1,85 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.hypervisor.kvm.resource.wrapper; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.PreMigrationCommand; +import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LogManager; +import org.libvirt.Connect; +import org.libvirt.Domain; +import org.libvirt.LibvirtException; + +import java.util.List; + +/** + * Handles PreMigrationCommand on the source host before live migration. + * Converts CLVM volume locks from exclusive to shared mode so the destination host can access them. + */ +@ResourceWrapper(handles = PreMigrationCommand.class) +public class LibvirtPreMigrationCommandWrapper extends CommandWrapper { + protected Logger logger = LogManager.getLogger(getClass()); + + @Override + public Answer execute(PreMigrationCommand command, LibvirtComputingResource libvirtComputingResource) { + String vmName = command.getVmName(); + VirtualMachineTO vmSpec = command.getVirtualMachine(); + + logger.info("Preparing source host for migration of VM: {}", vmName); + + Connect conn = null; + Domain dm = null; + + try { + LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); + conn = libvirtUtilitiesHelper.getConnectionByVmName(vmName); + dm = conn.domainLookupByName(vmName); + + List disks = libvirtComputingResource.getDisks(conn, vmName); + logger.info("Converting CLVM volumes to shared mode for VM: {}", vmName); + LibvirtComputingResource.modifyClvmVolumesStateForMigration( + disks, + libvirtComputingResource, + vmSpec, + LibvirtComputingResource.ClvmVolumeState.SHARED + ); + + logger.info("Successfully prepared source host for migration of VM: {}", vmName); + return new Answer(command, true, "Source host prepared for migration"); + + } catch (LibvirtException e) { + logger.error("Failed to prepare source host for migration of VM: {}", vmName, e); + return new Answer(command, false, "Failed to prepare source host: " + e.getMessage()); + } finally { + if (dm != null) { + try { + dm.free(); + } catch (LibvirtException e) { + logger.warn("Failed to free domain {}: {}", vmName, e.getMessage()); + } + } + } + } +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java index d9323df4477d..03840cd7fdfa 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java @@ -21,6 +21,7 @@ import java.net.URISyntaxException; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.cloudstack.storage.configdrive.ConfigDrive; @@ -124,6 +125,20 @@ public Answer execute(final PrepareForMigrationCommand command, final LibvirtCom return new PrepareForMigrationAnswer(command, "failed to connect physical disks to host"); } + // Activate CLVM volumes in shared mode on destination host for live migration + try { + List disks = libvirtComputingResource.getDisks(conn, vm.getName()); + LibvirtComputingResource.modifyClvmVolumesStateForMigration( + disks, + libvirtComputingResource, + vm, + LibvirtComputingResource.ClvmVolumeState.SHARED + ); + } catch (Exception e) { + logger.warn("Failed to activate CLVM volumes in shared mode on destination for VM {}: {}", + vm.getName(), e.getMessage(), e); + } + logger.info("Successfully prepared destination host for migration of VM {}", vm.getName()); return createPrepareForMigrationAnswer(command, dpdkInterfaceMapping, libvirtComputingResource, vm); } catch (final LibvirtException | CloudRuntimeException | InternalErrorException | URISyntaxException e) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java index 8dd2116e1235..ea346a335850 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java @@ -38,6 +38,8 @@ public interface KVMStoragePool { public static final long HeartBeatUpdateMaxTries = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_HEARTBEAT_UPDATE_MAX_TRIES); public static final long HeartBeatUpdateRetrySleep = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_HEARTBEAT_UPDATE_RETRY_SLEEP); public static final long HeartBeatCheckerTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_HEARTBEAT_CHECKER_TIMEOUT); + public static final String CLVM_SECURE_ZERO_FILL = "clvmsecurezerofill"; + public default KVMPhysicalDisk createPhysicalDisk(String volumeUuid, PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, Long usableSize, byte[] passphrase) { return createPhysicalDisk(volumeUuid, format, provisioningType, size, passphrase); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 030d9747d6cd..2e30c972caff 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1115,7 +1115,14 @@ public Answer backupSnapshot(final CopyCommand cmd) { } } else { final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), logger); - command.add("-b", isCreatedFromVmSnapshot ? snapshotDisk.getPath() : snapshot.getPath()); + String backupPath; + if (primaryPool.getType() == StoragePoolType.CLVM) { + backupPath = snapshotDisk.getPath(); + logger.debug("Using snapshotDisk path for CLVM backup: " + backupPath); + } else { + backupPath = isCreatedFromVmSnapshot ? snapshotDisk.getPath() : snapshot.getPath(); + } + command.add("-b", backupPath); command.add(NAME_OPTION, snapshotName); command.add("-p", snapshotDestPath); @@ -1160,6 +1167,76 @@ public Answer backupSnapshot(final CopyCommand cmd) { } } + /** + * Delete a CLVM snapshot using comprehensive cleanup. + * For CLVM, the snapshot path stored in DB is: /dev/vgname/volumeuuid/snapshotuuid + * the actual snapshot LV created by CLVM is: /dev/vgname/md5(snapshotuuid) + * + * @param snapshotPath The snapshot path from database + * @param checkExistence If true, checks if snapshot exists before cleanup (for explicit deletion) + * If false, always performs cleanup (for post-backup cleanup) + * @return true if cleanup was performed, false if snapshot didn't exist (when checkExistence=true) + */ + private boolean deleteClvmSnapshot(String snapshotPath, boolean checkExistence) { + logger.info("Starting CLVM snapshot deletion for path: {}, checkExistence: {}", snapshotPath, checkExistence); + + try { + // Parse the snapshot path: /dev/acsvg/volume-uuid/snapshot-uuid + String[] pathParts = snapshotPath.split("/"); + if (pathParts.length < 5) { + logger.warn("Invalid CLVM snapshot path format: {}, expected format: /dev/vgname/volume-uuid/snapshot-uuid", snapshotPath); + return false; + } + + String vgName = pathParts[2]; + String volumeUuid = pathParts[3]; + String snapshotUuid = pathParts[4]; + + logger.info("Parsed snapshot path - VG: {}, Volume: {}, Snapshot: {}", vgName, volumeUuid, snapshotUuid); + + // Compute MD5 hash of snapshot UUID (same as managesnapshot.sh does) + String md5Hash = computeMd5Hash(snapshotUuid); + logger.debug("Computed MD5 hash for snapshot UUID {}: {}", snapshotUuid, md5Hash); + String snapshotLvPath = vgName + "/" + md5Hash; + String actualSnapshotPath = "/dev/" + snapshotLvPath; + + // Check if snapshot exists (if requested) + if (checkExistence) { + Script checkSnapshot = new Script("/usr/sbin/lvs", 5000, logger); + checkSnapshot.add("--noheadings"); + checkSnapshot.add(snapshotLvPath); + String checkResult = checkSnapshot.execute(); + + if (checkResult != null) { + // Snapshot doesn't exist - was already cleaned up + logger.info("CLVM snapshot {} was already deleted, no cleanup needed", md5Hash); + return false; + } + logger.info("CLVM snapshot still exists for {}, performing cleanup", md5Hash); + } + + // Use native LVM command to remove snapshot (handles all cleanup automatically) + Script removeSnapshot = new Script("lvremove", 10000, logger); + removeSnapshot.add("-f"); + removeSnapshot.add(snapshotLvPath); + + logger.info("Executing: lvremove -f {}", snapshotLvPath); + String removeResult = removeSnapshot.execute(); + + if (removeResult == null) { + logger.info("Successfully deleted CLVM snapshot: {} (actual path: {})", snapshotPath, actualSnapshotPath); + return true; + } else { + logger.warn("Failed to delete CLVM snapshot {}: {}", snapshotPath, removeResult); + return false; + } + + } catch (Exception ex) { + logger.error("Exception while deleting CLVM snapshot {}", snapshotPath, ex); + return false; + } + } + private void deleteSnapshotOnPrimary(final CopyCommand cmd, final SnapshotObjectTO snapshot, KVMStoragePool primaryPool) { String snapshotPath = snapshot.getPath(); @@ -1172,7 +1249,14 @@ private void deleteSnapshotOnPrimary(final CopyCommand cmd, final SnapshotObject if ((backupSnapshotAfterTakingSnapshot == null || BooleanUtils.toBoolean(backupSnapshotAfterTakingSnapshot)) && deleteSnapshotOnPrimary) { try { - Files.deleteIfExists(Paths.get(snapshotPath)); + if (primaryPool.getType() == StoragePoolType.CLVM) { + boolean cleanedUp = deleteClvmSnapshot(snapshotPath, false); + if (!cleanedUp) { + logger.info("No need to delete CLVM snapshot on primary as it doesn't exist: {}", snapshotPath); + } + } else { + Files.deleteIfExists(Paths.get(snapshotPath)); + } } catch (IOException ex) { logger.error("Failed to delete snapshot [{}] on primary storage [{}].", snapshot.getId(), snapshot.getName(), ex); } @@ -1181,6 +1265,26 @@ private void deleteSnapshotOnPrimary(final CopyCommand cmd, final SnapshotObject } } + + /** + * Compute MD5 hash of a string, matching what managesnapshot.sh does: + * echo "${snapshot}" | md5sum -t | awk '{ print $1 }' + */ + private String computeMd5Hash(String input) { + try { + java.security.MessageDigest md = java.security.MessageDigest.getInstance("MD5"); + byte[] array = md.digest((input + "\n").getBytes("UTF-8")); + StringBuilder sb = new StringBuilder(); + for (byte b : array) { + sb.append(String.format("%02x", b)); + } + return sb.toString(); + } catch (Exception e) { + logger.error("Failed to compute MD5 hash for: {}", input, e); + return input; + } + } + protected synchronized void attachOrDetachISO(final Connect conn, final String vmName, String isoPath, final boolean isAttach, Map params, DataStoreTO store) throws LibvirtException, InternalErrorException { DiskDef iso = new DiskDef(); @@ -1842,8 +1946,10 @@ public Answer createSnapshot(final CreateObjectCommand cmd) { } } - if (DomainInfo.DomainState.VIR_DOMAIN_RUNNING.equals(state) && volume.requiresEncryption()) { - throw new CloudRuntimeException("VM is running, encrypted volume snapshots aren't supported"); + if (DomainInfo.DomainState.VIR_DOMAIN_RUNNING.equals(state)) { + if (volume.requiresEncryption()) { + throw new CloudRuntimeException("VM is running, encrypted volume snapshots aren't supported"); + } } KVMStoragePool primaryPool = storagePoolMgr.getStoragePool(primaryStore.getPoolType(), primaryStore.getUuid()); @@ -2880,6 +2986,25 @@ public Answer deleteSnapshot(final DeleteCommand cmd) { if (snapshotTO.isKvmIncrementalSnapshot()) { deleteCheckpoint(snapshotTO); } + } else if (primaryPool.getType() == StoragePoolType.CLVM) { + // For CLVM, snapshots are typically already deleted from primary storage during backup + // via deleteSnapshotOnPrimary in the backupSnapshot finally block. + // This is called when the user explicitly deletes the snapshot via UI/API. + // We check if the snapshot still exists and clean it up if needed. + logger.info("Processing CLVM snapshot deletion (id={}, name={}, path={}) on primary storage", + snapshotTO.getId(), snapshotTO.getName(), snapshotTO.getPath()); + + String snapshotPath = snapshotTO.getPath(); + if (snapshotPath != null && !snapshotPath.isEmpty()) { + boolean wasDeleted = deleteClvmSnapshot(snapshotPath, true); + if (wasDeleted) { + logger.info("Successfully cleaned up CLVM snapshot {} from primary storage", snapshotName); + } else { + logger.info("CLVM snapshot {} was already deleted from primary storage during backup, no cleanup needed", snapshotName); + } + } else { + logger.debug("CLVM snapshot path is null or empty, assuming already cleaned up"); + } } else { logger.warn("Operation not implemented for storage pool type of " + primaryPool.getType().toString()); throw new InternalErrorException("Operation not implemented for storage pool type of " + primaryPool.getType().toString()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index a03daeb197bf..c51691ecb5ed 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -34,6 +34,7 @@ import com.cloud.agent.properties.AgentProperties; import com.cloud.agent.properties.AgentPropertiesFileHandler; +import com.cloud.utils.script.OutputInterpreter; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.utils.cryptsetup.KeyFile; import org.apache.cloudstack.utils.qemu.QemuImageOptions; @@ -254,9 +255,12 @@ public StorageVol getVolume(StoragePool pool, String volName) { try { vol = pool.storageVolLookupByName(volName); - logger.debug("Found volume " + volName + " in storage pool " + pool.getName() + " after refreshing the pool"); + if (vol != null) { + logger.debug("Found volume " + volName + " in storage pool " + pool.getName() + " after refreshing the pool"); + } } catch (LibvirtException e) { - throw new CloudRuntimeException("Could not find volume " + volName + ": " + e.getMessage()); + logger.debug("Volume " + volName + " still not found after pool refresh: " + e.getMessage()); + return null; } } @@ -663,6 +667,17 @@ public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) { try { StorageVol vol = getVolume(libvirtPool.getPool(), volumeUuid); + + // Check if volume was found - if null, treat as not found and trigger fallback for CLVM + if (vol == null) { + logger.debug("Volume " + volumeUuid + " not found in libvirt, will check for CLVM fallback"); + if (pool.getType() == StoragePoolType.CLVM) { + return getPhysicalDisk(volumeUuid, pool, libvirtPool); + } + + throw new CloudRuntimeException("Volume " + volumeUuid + " not found in libvirt pool"); + } + KVMPhysicalDisk disk; LibvirtStorageVolumeDef voldef = getStorageVolumeDef(libvirtPool.getPool().getConnect(), vol); disk = new KVMPhysicalDisk(vol.getPath(), vol.getName(), pool); @@ -693,11 +708,156 @@ public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) { } return disk; } catch (LibvirtException e) { - logger.debug("Failed to get physical disk:", e); + logger.debug("Failed to get volume from libvirt: " + e.getMessage()); + // For CLVM, try direct block device access as fallback + if (pool.getType() == StoragePoolType.CLVM) { + return getPhysicalDisk(volumeUuid, pool, libvirtPool); + } + throw new CloudRuntimeException(e.toString()); } } + private KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool, LibvirtStoragePool libvirtPool) { + logger.info("CLVM volume not visible to libvirt, attempting direct block device access for volume: {}", volumeUuid); + + try { + logger.debug("Refreshing libvirt storage pool: {}", pool.getUuid()); + libvirtPool.getPool().refresh(0); + + StorageVol vol = getVolume(libvirtPool.getPool(), volumeUuid); + if (vol != null) { + logger.info("Volume found after pool refresh: {}", volumeUuid); + KVMPhysicalDisk disk; + LibvirtStorageVolumeDef voldef = getStorageVolumeDef(libvirtPool.getPool().getConnect(), vol); + disk = new KVMPhysicalDisk(vol.getPath(), vol.getName(), pool); + disk.setSize(vol.getInfo().allocation); + disk.setVirtualSize(vol.getInfo().capacity); + disk.setFormat(voldef.getFormat() == LibvirtStorageVolumeDef.VolumeFormat.QCOW2 ? + PhysicalDiskFormat.QCOW2 : PhysicalDiskFormat.RAW); + return disk; + } + } catch (LibvirtException refreshEx) { + logger.debug("Pool refresh failed or volume still not found: {}", refreshEx.getMessage()); + } + + // Still not found after refresh, try direct block device access + return getPhysicalDiskViaDirectBlockDevice(volumeUuid, pool); + } + + private String getVgName(KVMStoragePool pool, String sourceDir) { + String vgName = sourceDir; + if (vgName.startsWith("/")) { + String[] parts = vgName.split("/"); + List tokens = Arrays.stream(parts) + .filter(s -> !s.isEmpty()).collect(Collectors.toList()); + + vgName = tokens.size() > 1 ? tokens.get(1) + : tokens.size() == 1 ? tokens.get(0) + : ""; + } + return vgName; + } + + /** + * For CLVM volumes that exist in LVM but are not visible to libvirt, + * access them directly via block device path. + */ + private KVMPhysicalDisk getPhysicalDiskViaDirectBlockDevice(String volumeUuid, KVMStoragePool pool) { + try { + // For CLVM, pool sourceDir contains the VG path (e.g., "/dev/acsvg") + // Extract the VG name + String sourceDir = pool.getLocalPath(); + if (sourceDir == null || sourceDir.isEmpty()) { + throw new CloudRuntimeException("CLVM pool sourceDir is not set, cannot determine VG name"); + } + String vgName = getVgName(pool, sourceDir); + logger.debug("Using VG name: {} (from sourceDir: {}) ", vgName, sourceDir); + + // Check if the LV exists in LVM using lvs command + logger.debug("Checking if volume {} exists in VG {}", volumeUuid, vgName); + Script checkLvCmd = new Script("/usr/sbin/lvs", 5000, logger); + checkLvCmd.add("--noheadings"); + checkLvCmd.add("--unbuffered"); + checkLvCmd.add(vgName + "/" + volumeUuid); + + String checkResult = checkLvCmd.execute(); + if (checkResult != null) { + logger.debug("Volume {} does not exist in VG {}: {}", volumeUuid, vgName, checkResult); + throw new CloudRuntimeException(String.format("Storage volume not found: no storage vol with matching name '%s'", volumeUuid)); + } + + logger.info("Volume {} exists in LVM but not visible to libvirt, accessing directly", volumeUuid); + + // Try standard device path first + String lvPath = "/dev/" + vgName + "/" + volumeUuid; + File lvDevice = new File(lvPath); + + if (!lvDevice.exists()) { + // Try device-mapper path with escaped hyphens + String vgNameEscaped = vgName.replace("-", "--"); + String volumeUuidEscaped = volumeUuid.replace("-", "--"); + lvPath = "/dev/mapper/" + vgNameEscaped + "-" + volumeUuidEscaped; + lvDevice = new File(lvPath); + + if (!lvDevice.exists()) { + logger.warn("Volume exists in LVM but device node not found: {}", volumeUuid); + throw new CloudRuntimeException(String.format("Could not find volume %s " + + "in VG %s - volume exists in LVM but device node not accessible", volumeUuid, vgName)); + } + } + + long size = 0; + try { + Script lvsCmd = new Script("/usr/sbin/lvs", 5000, logger); + lvsCmd.add("--noheadings"); + lvsCmd.add("--units"); + lvsCmd.add("b"); + lvsCmd.add("-o"); + lvsCmd.add("lv_size"); + lvsCmd.add(lvPath); + + OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser(); + String result = lvsCmd.execute(parser); + + String output = null; + if (result == null) { + output = parser.getLines(); + } else { + output = result; + } + + if (output != null && !output.isEmpty()) { + String sizeStr = output.trim().replaceAll("[^0-9]", ""); + if (!sizeStr.isEmpty()) { + size = Long.parseLong(sizeStr); + } + } + } catch (Exception sizeEx) { + logger.warn("Failed to get size for CLVM volume via lvs: {}", sizeEx.getMessage()); + if (lvDevice.isFile()) { + size = lvDevice.length(); + } + } + + KVMPhysicalDisk disk = new KVMPhysicalDisk(lvPath, volumeUuid, pool); + disk.setFormat(PhysicalDiskFormat.RAW); + disk.setSize(size); + disk.setVirtualSize(size); + + logger.info("Successfully accessed CLVM volume via direct block device: {} " + + "with size: {} bytes",lvPath, size); + + return disk; + + } catch (CloudRuntimeException ex) { + throw ex; + } catch (Exception ex) { + logger.error("Failed to access CLVM volume via direct block device: {}",volumeUuid, ex); + throw new CloudRuntimeException(String.format("Could not find volume %s: %s ",volumeUuid, ex.getMessage())); + } + } + /** * adjust refcount */ @@ -1046,6 +1206,12 @@ private KVMPhysicalDisk createPhysicalDiskByLibVirt(String name, KVMStoragePool volName = vol.getName(); volAllocation = vol.getInfo().allocation; volCapacity = vol.getInfo().capacity; + + // For CLVM volumes, activate in shared mode so all cluster hosts can access it + if (pool.getType() == StoragePoolType.CLVM) { + logger.info("Activating CLVM volume {} in shared mode for cluster-wide access", volPath); + activateClvmVolumeInSharedMode(volPath); + } } catch (LibvirtException e) { throw new CloudRuntimeException(e.toString()); } @@ -1057,6 +1223,30 @@ private KVMPhysicalDisk createPhysicalDiskByLibVirt(String name, KVMStoragePool return disk; } + /** + * Activates a CLVM volume in shared mode so all hosts in the cluster can access it. + * This is necessary after volume creation since libvirt creates LVs with exclusive activation by default. + * + * @param volumePath The full path to the LV (e.g., /dev/vgname/volume-path) + */ + private void activateClvmVolumeInSharedMode(String volumePath) { + try { + Script cmd = new Script("lvchange", 5000, logger); + cmd.add("-asy"); // Activate in shared mode + cmd.add(volumePath); + + String result = cmd.execute(); + if (result != null) { + logger.error("Failed to activate CLVM volume {} in shared mode. Result: {}", volumePath, result); + throw new CloudRuntimeException("Failed to activate CLVM volume in shared mode: " + result); + } + logger.info("Successfully activated CLVM volume {} in shared mode", volumePath); + } catch (Exception e) { + logger.error("Exception while activating CLVM volume {} in shared mode: {}", volumePath, e.getMessage(), e); + throw new CloudRuntimeException("Failed to activate CLVM volume in shared mode: " + e.getMessage(), e); + } + } + private KVMPhysicalDisk createPhysicalDiskByQemuImg(String name, KVMStoragePool pool, PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, byte[] passphrase) { @@ -1116,6 +1306,17 @@ private KVMPhysicalDisk createPhysicalDiskByQemuImg(String name, KVMStoragePool @Override public boolean connectPhysicalDisk(String name, KVMStoragePool pool, Map details, boolean isVMMigrate) { // this is for managed storage that needs to prep disks prior to use + if (pool.getType() == StoragePoolType.CLVM && isVMMigrate) { + logger.info("Activating CLVM volume {} at location: {} in shared mode for VM migration", name, pool.getLocalPath() + File.separator + name); + Script activateVolInSharedMode = new Script("lvchange", 5000, logger); + activateVolInSharedMode.add("-asy"); + activateVolInSharedMode.add(pool.getLocalPath() + File.separator + name); + String result = activateVolInSharedMode.execute(); + if (result != null) { + logger.error("Failed to activate CLVM volume {} in shared mode for VM migration. Command output: {}", name, result); + return false; + } + } return true; } @@ -1224,10 +1425,21 @@ public boolean deletePhysicalDisk(String uuid, KVMStoragePool pool, Storage.Imag } } + // For CLVM pools, always use direct LVM cleanup to ensure secure zero-fill + if (pool.getType() == StoragePoolType.CLVM) { + logger.info("CLVM pool detected - using direct LVM cleanup with secure zero-fill for volume {}", uuid); + return cleanupCLVMVolume(uuid, pool); + } + + // For non-CLVM pools, use libvirt deletion LibvirtStoragePool libvirtPool = (LibvirtStoragePool)pool; try { StorageVol vol = getVolume(libvirtPool.getPool(), uuid); - logger.debug("Instructing libvirt to remove volume " + uuid + " from pool " + pool.getUuid()); + if (vol == null) { + logger.warn("Volume {} not found in libvirt pool {}, it may have been already deleted", uuid, pool.getUuid()); + return true; + } + logger.debug("Instructing libvirt to remove volume {} from pool {}", uuid, pool.getUuid()); if(Storage.ImageFormat.DIR.equals(format)){ deleteDirVol(libvirtPool, vol); } else { @@ -1240,6 +1452,86 @@ public boolean deletePhysicalDisk(String uuid, KVMStoragePool pool, Storage.Imag } } + private boolean shouldSecureZeroFill(KVMStoragePool pool) { + Map details = pool.getDetails(); + String secureZeroFillStr = (details != null) ? details.get(KVMStoragePool.CLVM_SECURE_ZERO_FILL) : null; + return Boolean.parseBoolean(secureZeroFillStr); + } + + /** + * Clean up CLVM volume and its snapshots directly using LVM commands. + * This is used as a fallback when libvirt cannot find or delete the volume. + */ + private boolean cleanupCLVMVolume(String uuid, KVMStoragePool pool) { + logger.info("Starting direct LVM cleanup for CLVM volume: {} in pool: {}", uuid, pool.getUuid()); + + try { + String sourceDir = pool.getLocalPath(); + if (sourceDir == null || sourceDir.isEmpty()) { + logger.debug("Source directory is null or empty, cannot determine VG name for CLVM pool {}, skipping direct cleanup", pool.getUuid()); + return true; + } + String vgName = getVgName(pool, sourceDir); + logger.info("Determined VG name: {} for pool: {}", vgName, pool.getUuid()); + + if (vgName == null || vgName.isEmpty()) { + logger.warn("Cannot determine VG name for CLVM pool {}, skipping direct cleanup", pool.getUuid()); + return true; + } + + String lvPath = "/dev/" + vgName + "/" + uuid; + logger.debug("Volume path: {}", lvPath); + + // Check if the LV exists + Script checkLvs = new Script("lvs", 5000, logger); + checkLvs.add("--noheadings"); + checkLvs.add("--unbuffered"); + checkLvs.add(lvPath); + + logger.info("Checking if volume exists: lvs --noheadings --unbuffered {}", lvPath); + String checkResult = checkLvs.execute(); + + if (checkResult != null) { + logger.info("CLVM volume {} does not exist in LVM (check returned: {}), considering it as already deleted", uuid, checkResult); + return true; + } + + logger.info("Volume {} exists, proceeding with cleanup", uuid); + + boolean secureZeroFillEnabled = shouldSecureZeroFill(pool); + + if (secureZeroFillEnabled) { + logger.info("Step 1: Zero-filling volume {} for security", uuid); + secureZeroFillVolume(lvPath, uuid); + } else { + logger.info("Secure zero-fill is disabled, skipping zero-filling for volume {}", uuid); + } + + logger.info("Step 2: Removing volume {}", uuid); + Script removeLv = new Script("lvremove", 10000, logger); + removeLv.add("-f"); + removeLv.add(lvPath); + + logger.info("Executing command: lvremove -f {}", lvPath); + String removeResult = removeLv.execute(); + + if (removeResult == null) { + logger.info("Successfully removed CLVM volume {} using direct LVM cleanup", uuid); + return true; + } else { + logger.warn("Command 'lvremove -f {}' returned error: {}", lvPath, removeResult); + if (removeResult.contains("not found") || removeResult.contains("Failed to find")) { + logger.info("CLVM volume {} not found during cleanup, considering it as already deleted", uuid); + return true; + } + return false; + } + } catch (Exception ex) { + logger.error("Exception during CLVM volume cleanup for {}: {}", uuid, ex.getMessage(), ex); + return true; + } + } + /** * This function copies a physical disk from Secondary Storage to Primary Storage * or from Primary to Primary Storage @@ -1737,6 +2029,75 @@ private void deleteVol(LibvirtStoragePool pool, StorageVol vol) throws LibvirtEx vol.delete(0); } + /** + * Securely zero-fill a volume before deletion to prevent data leakage. + * Uses blkdiscard (fast TRIM) as primary method, with dd zero-fill as fallback. + * + * @param lvPath The full path to the logical volume (e.g., /dev/vgname/lvname) + * @param volumeUuid The UUID of the volume for logging purposes + */ + private void secureZeroFillVolume(String lvPath, String volumeUuid) { + logger.info("Starting secure zero-fill for CLVM volume: {} at path: {}", volumeUuid, lvPath); + + boolean blkdiscardSuccess = false; + + // Try blkdiscard first (fast - sends TRIM commands) + try { + Script blkdiscard = new Script("blkdiscard", 300000, logger); // 5 minute timeout + blkdiscard.add("-f"); // Force flag to suppress confirmation prompts + blkdiscard.add(lvPath); + + String result = blkdiscard.execute(); + if (result == null) { + logger.info("Successfully zero-filled CLVM volume {} using blkdiscard (TRIM)", volumeUuid); + blkdiscardSuccess = true; + } else { + if (result.contains("Operation not supported") || result.contains("BLKDISCARD ioctl failed")) { + logger.info("blkdiscard not supported for volume {} (device doesn't support TRIM/DISCARD), using dd fallback", volumeUuid); + } else { + logger.warn("blkdiscard failed for volume {}: {}, will try dd fallback", volumeUuid, result); + } + } + } catch (Exception e) { + logger.warn("Exception during blkdiscard for volume {}: {}, will try dd fallback", volumeUuid, e.getMessage()); + } + + // Fallback to dd zero-fill (slow) + if (!blkdiscardSuccess) { + logger.info("Attempting zero-fill using dd for CLVM volume: {}", volumeUuid); + try { + // nice -n 19: lowest CPU priority + // ionice -c 2 -n 7: best-effort I/O scheduling with lowest priority + // oflag=direct: bypass cache for more predictable performance + String command = String.format( + "nice -n 19 ionice -c 2 -n 7 dd if=/dev/zero of=%s bs=1M oflag=direct 2>&1 || true", + lvPath + ); + + Script ddZeroFill = new Script("/bin/bash", 3600000, logger); // 60 minute timeout for large volumes + ddZeroFill.add("-c"); + ddZeroFill.add(command); + + OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser(); + String ddResult = ddZeroFill.execute(parser); + String output = parser.getLines(); + + if (output != null && (output.contains("copied") || output.contains("records in") || + output.contains("No space left on device"))) { + logger.info("Successfully zero-filled CLVM volume {} using dd", volumeUuid); + } else if (ddResult == null) { + logger.info("Zero-fill completed for CLVM volume {}", volumeUuid); + } else { + logger.warn("dd zero-fill for volume {} completed with output: {}", volumeUuid, + output != null ? output : ddResult); + } + } catch (Exception e) { + logger.warn("Failed to zero-fill CLVM volume {} before deletion: {}. Proceeding with deletion anyway.", + volumeUuid, e.getMessage()); + } + } + } + private void deleteDirVol(LibvirtStoragePool pool, StorageVol vol) throws LibvirtException { Script.runSimpleBashScript("rm -r --interactive=never " + vol.getPath()); } diff --git a/scripts/storage/qcow2/managesnapshot.sh b/scripts/storage/qcow2/managesnapshot.sh index 3650bdd9b6f6..cba330b6b89e 100755 --- a/scripts/storage/qcow2/managesnapshot.sh +++ b/scripts/storage/qcow2/managesnapshot.sh @@ -58,15 +58,11 @@ is_lv() { } get_vg() { - lvm lvs --noheadings --unbuffered --separator=/ "${1}" | cut -d '/' -f 2 + lvm lvs --noheadings --unbuffered --separator=/ "${1}" | cut -d '/' -f 2 | tr -d ' ' } get_lv() { - lvm lvs --noheadings --unbuffered --separator=/ "${1}" | cut -d '/' -f 1 -} - -double_hyphens() { - echo ${1} | sed -e "s/-/--/g" + lvm lvs --noheadings --unbuffered --separator=/ "${1}" | cut -d '/' -f 1 | tr -d ' ' } create_snapshot() { @@ -77,30 +73,39 @@ create_snapshot() { islv_ret=$? if [ ${dmsnapshot} = "yes" ] && [ "$islv_ret" == "1" ]; then + # Modern LVM snapshot approach local lv=`get_lv ${disk}` local vg=`get_vg ${disk}` - local lv_dm=`double_hyphens ${lv}` - local vg_dm=`double_hyphens ${vg}` - local lvdevice=/dev/mapper/${vg_dm}-${lv_dm} - local lv_bytes=`blockdev --getsize64 ${lvdevice}` - local lv_sectors=`blockdev --getsz ${lvdevice}` - - lvm lvcreate --size ${lv_bytes}b --name "${snapshotname}-cow" ${vg} >&2 || return 2 - dmsetup suspend ${vg_dm}-${lv_dm} >&2 - if dmsetup info -c --noheadings -o name ${vg_dm}-${lv_dm}-real > /dev/null 2>&1; then - echo "0 ${lv_sectors} snapshot ${lvdevice}-real /dev/mapper/${vg_dm}-${snapshotname}--cow p 64" | \ - dmsetup create "${vg_dm}-${snapshotname}" >&2 || ( destroy_snapshot ${disk} "${snapshotname}"; return 2 ) - dmsetup resume "${vg_dm}-${snapshotname}" >&2 || ( destroy_snapshot ${disk} "${snapshotname}"; return 2 ) - else - dmsetup table ${vg_dm}-${lv_dm} | dmsetup create ${vg_dm}-${lv_dm}-real >&2 || ( destroy_snapshot ${disk} "${snapshotname}"; return 2 ) - dmsetup resume ${vg_dm}-${lv_dm}-real >&2 || ( destroy_snapshot ${disk} "${snapshotname}"; return 2 ) - echo "0 ${lv_sectors} snapshot ${lvdevice}-real /dev/mapper/${vg_dm}-${snapshotname}--cow p 64" | \ - dmsetup create "${vg_dm}-${snapshotname}" >&2 || ( destroy_snapshot ${disk} "${snapshotname}"; return 2 ) - echo "0 ${lv_sectors} snapshot-origin ${lvdevice}-real" | \ - dmsetup load ${vg_dm}-${lv_dm} >&2 || ( destroy_snapshot ${disk} "${snapshotname}"; return 2 ) - dmsetup resume "${vg_dm}-${snapshotname}" >&2 || ( destroy_snapshot ${disk} "${snapshotname}"; return 2 ) + local lv_bytes=`blockdev --getsize64 ${disk}` + + # Calculate snapshot size (10% of origin size, minimum 100MB, maximum 10GB) + local snapshot_size=$((lv_bytes / 10)) + local min_size=$((100 * 1024 * 1024)) # 100MB + local max_size=$((10 * 1024 * 1024 * 1024)) # 10GB + + if [ ${snapshot_size} -lt ${min_size} ]; then + snapshot_size=${min_size} + elif [ ${snapshot_size} -gt ${max_size} ]; then + snapshot_size=${max_size} + fi + + # Round to nearest 512-byte multiple (LVM requirement) + snapshot_size=$(((snapshot_size + 511) / 512 * 512)) + + # Create LVM snapshot using native command + lvm lvcreate -L ${snapshot_size}b -s -n "${snapshotname}" "${vg}/${lv}" >&2 + if [ $? -gt 0 ]; then + printf "***Failed to create LVM snapshot ${snapshotname} for ${vg}/${lv}\n" >&2 + return 2 + fi + + # Activate the snapshot + lvm lvchange --yes -ay "${vg}/${snapshotname}" >&2 + if [ $? -gt 0 ]; then + printf "***Failed to activate LVM snapshot ${snapshotname}\n" >&2 + lvm lvremove -f "${vg}/${snapshotname}" >&2 + return 2 fi - dmsetup resume "${vg_dm}-${lv_dm}" >&2 elif [ -f "${disk}" ]; then $qemu_img snapshot -c "$snapshotname" $disk @@ -132,25 +137,22 @@ destroy_snapshot() { islv_ret=$? if [ "$islv_ret" == "1" ]; then + # Modern LVM snapshot deletion local lv=`get_lv ${disk}` local vg=`get_vg ${disk}` - local lv_dm=`double_hyphens ${lv}` - local vg_dm=`double_hyphens ${vg}` - if [ -e /dev/mapper/${vg_dm}-${lv_dm}-real ]; then - local dm_refcount=`dmsetup info -c --noheadings -o open ${vg_dm}-${lv_dm}-real` - if [ ${dm_refcount} -le 2 ]; then - dmsetup suspend ${vg_dm}-${lv_dm} >&2 - dmsetup table ${vg_dm}-${lv_dm}-real | dmsetup load ${vg_dm}-${lv_dm} >&2 - dmsetup resume ${vg_dm}-${lv_dm} - dmsetup remove "${vg_dm}-${snapshotname}" - dmsetup remove ${vg_dm}-${lv_dm}-real - else - dmsetup remove "${vg_dm}-${snapshotname}" - fi - else - dmsetup remove "${vg_dm}-${snapshotname}" + + # Check if snapshot exists + if ! lvm lvs "${vg}/${snapshotname}" > /dev/null 2>&1; then + printf "Snapshot ${vg}/${snapshotname} does not exist or was already deleted\n" >&2 + return 0 + fi + + # Remove the snapshot using native LVM command + lvm lvremove -f "${vg}/${snapshotname}" >&2 + if [ $? -gt 0 ]; then + printf "***Failed to remove LVM snapshot ${vg}/${snapshotname}\n" >&2 + return 2 fi - lvm lvremove -f "${vg}/${snapshotname}-cow" elif [ -f $disk ]; then #delete all the existing snapshots $qemu_img snapshot -l $disk |tail -n +3|awk '{print $1}'|xargs -I {} $qemu_img snapshot -d {} $disk >&2 @@ -170,12 +172,37 @@ rollback_snapshot() { local disk=$1 local snapshotname="$2" local failed=0 + is_lv ${disk} + islv_ret=$? - $qemu_img snapshot -a $snapshotname $disk + if [ ${dmrollback} = "yes" ] && [ "$islv_ret" == "1" ]; then + # Modern LVM snapshot merge (rollback) + local lv=`get_lv ${disk}` + local vg=`get_vg ${disk}` - if [ $? -gt 0 ] - then - printf "***Failed to apply snapshot $snapshotname for path $disk\n" >&2 + # Check if snapshot exists + if ! lvm lvs "${vg}/${snapshotname}" > /dev/null 2>&1; then + printf "***Snapshot ${vg}/${snapshotname} does not exist\n" >&2 + return 1 + fi + + # Use lvconvert --merge to rollback + lvm lvconvert --merge "${vg}/${snapshotname}" >&2 + if [ $? -gt 0 ]; then + printf "***Failed to merge/rollback snapshot ${snapshotname} for ${vg}/${lv}\n" >&2 + return 1 + fi + elif [ -f ${disk} ]; then + # File-based snapshot rollback using qemu-img + $qemu_img snapshot -a $snapshotname $disk + + if [ $? -gt 0 ] + then + printf "***Failed to apply snapshot $snapshotname for path $disk\n" >&2 + failed=1 + fi + else + printf "***Failed to rollback snapshot $snapshotname, undefined type $disk\n" >&2 failed=1 fi @@ -204,20 +231,21 @@ backup_snapshot() { if [ ${dmsnapshot} = "yes" ] && [ "$islv_ret" == "1" ] ; then local vg=`get_vg ${disk}` - local vg_dm=`double_hyphens ${vg}` local scriptdir=`dirname ${0}` - if ! dmsetup info -c --noheadings -o name ${vg_dm}-${snapshotname} > /dev/null 2>&1; then + # Check if snapshot exists using native LVM command + if ! lvm lvs "${vg}/${snapshotname}" > /dev/null 2>&1; then printf "Disk ${disk} has no snapshot called ${snapshotname}.\n" >&2 return 1 fi - qemuimg_ret=$($qemu_img $forceShareFlag -f raw -O qcow2 "/dev/mapper/${vg_dm}-${snapshotname}" "${destPath}/${destName}") + # Use native LVM path for backup + qemuimg_ret=$($qemu_img convert $forceShareFlag -f raw -O qcow2 "/dev/${vg}/${snapshotname}" "${destPath}/${destName}" 2>&1) ret_code=$? - if [ $ret_code -gt 0 ] && [[ $qemuimg_ret == *"snapshot: invalid option -- 'U'"* ]] + if [ $ret_code -gt 0 ] && ([[ $qemuimg_ret == *"invalid option"*"'U'"* ]] || [[ $qemuimg_ret == *"unrecognized option"*"'-U'"* ]]) then forceShareFlag="" - $qemu_img $forceShareFlag -f raw -O qcow2 "/dev/mapper/${vg_dm}-${snapshotname}" "${destPath}/${destName}" + $qemu_img convert $forceShareFlag -f raw -O qcow2 "/dev/${vg}/${snapshotname}" "${destPath}/${destName}" ret_code=$? fi if [ $ret_code -gt 0 ] @@ -240,9 +268,9 @@ backup_snapshot() { # Backup VM snapshot qemuimg_ret=$($qemu_img snapshot $forceShareFlag -l $disk 2>&1) ret_code=$? - if [ $ret_code -gt 0 ] && [[ $qemuimg_ret == *"snapshot: invalid option -- 'U'"* ]]; then + if [ $ret_code -gt 0 ] && ([[ $qemuimg_ret == *"invalid option"*"'U'"* ]] || [[ $qemuimg_ret == *"unrecognized option"*"'-U'"* ]]); then forceShareFlag="" - qemuimg_ret=$($qemu_img snapshot $forceShareFlag -l $disk) + qemuimg_ret=$($qemu_img snapshot $forceShareFlag -l $disk 2>&1) ret_code=$? fi @@ -251,11 +279,11 @@ backup_snapshot() { return 1 fi - qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -l snapshot.name=$snapshotname $disk $destPath/$destName 2>&1 > /dev/null) + qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -l snapshot.name=$snapshotname $disk $destPath/$destName 2>&1) ret_code=$? - if [ $ret_code -gt 0 ] && [[ $qemuimg_ret == *"convert: invalid option -- 'U'"* ]]; then + if [ $ret_code -gt 0 ] && ([[ $qemuimg_ret == *"invalid option"*"'U'"* ]] || [[ $qemuimg_ret == *"unrecognized option"*"'-U'"* ]]); then forceShareFlag="" - qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -l snapshot.name=$snapshotname $disk $destPath/$destName 2>&1 > /dev/null) + qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -l snapshot.name=$snapshotname $disk $destPath/$destName 2>&1) ret_code=$? fi diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 17961dbd955f..70428ffbb48c 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -130,6 +130,7 @@ import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; +import org.apache.cloudstack.storage.command.ClvmLockTransferCommand; import com.cloud.agent.api.ModifyTargetsCommand; import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.DiskTO; @@ -152,6 +153,7 @@ import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.StorageUnavailableException; @@ -410,6 +412,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic public static final ConfigKey AllowCheckAndRepairVolume = new ConfigKey<>("Advanced", Boolean.class, "volume.check.and.repair.leaks.before.use", "false", "To check and repair the volume if it has any leaks before performing volume attach or VM start operations", true, ConfigKey.Scope.StoragePool); + public static final ConfigKey CLVMSecureZeroFill = new ConfigKey<>("Advanced", Boolean.class, "clvm.secure.zero.fill", "false", + "When enabled, CLVM volumes to be zero-filled at the time of deletion to prevent data from being recovered by VMs reusing the space, as thick LVM volumes write data linearly", true, ConfigKey.Scope.StoragePool); + private final StateMachine2 _volStateMachine; private static final Set STATES_VOLUME_CANNOT_BE_DESTROYED = new HashSet<>(Arrays.asList(Volume.State.Destroy, Volume.State.Expunging, Volume.State.Expunged, Volume.State.Allocated)); @@ -1752,6 +1757,11 @@ private void expungeVolumesInPrimaryOrSecondary(VolumeVO volume, DataStoreRole r if (DataStoreRole.Image.equals(role)) { _resourceLimitMgr.decrementResourceCount(volOnStorage.getAccountId(), ResourceType.secondary_storage, volOnStorage.getSize()); } + + // Clean up CLVM lock host tracking detail after successful deletion from primary storage + if (DataStoreRole.Primary.equals(role)) { + cleanupClvmLockHostDetail(volume); + } } } @@ -2602,21 +2612,42 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device logger.trace(String.format("is it needed to move the volume: %b?", moveVolumeNeeded)); } - if (moveVolumeNeeded) { + // Check if CLVM lock transfer is needed (even if moveVolumeNeeded is false) + // This handles the case where the volume is already on the correct storage pool + // but the VM is running on a different host, requiring only a lock transfer + boolean isClvmLockTransferNeeded = !moveVolumeNeeded && + isClvmLockTransferRequired(newVolumeOnPrimaryStorage, existingVolumeOfVm, vm); + + if (isClvmLockTransferNeeded) { + // CLVM lock transfer - no data copy, no pool change needed + newVolumeOnPrimaryStorage = executeClvmLightweightMigration( + newVolumeOnPrimaryStorage, vm, existingVolumeOfVm, + "CLVM lock transfer", "same pool to different host"); + } else if (moveVolumeNeeded) { PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)newVolumeOnPrimaryStorage.getDataStore(); if (primaryStore.isLocal()) { throw new CloudRuntimeException( "Failed to attach local data volume " + volumeToAttach.getName() + " to VM " + vm.getDisplayName() + " as migration of local data volume is not allowed"); } - StoragePoolVO vmRootVolumePool = _storagePoolDao.findById(existingVolumeOfVm.getPoolId()); - try { - HypervisorType volumeToAttachHyperType = _volsDao.getHypervisorType(volumeToAttach.getId()); - newVolumeOnPrimaryStorage = _volumeMgr.moveVolume(newVolumeOnPrimaryStorage, vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(), vmRootVolumePool.getClusterId(), - volumeToAttachHyperType); - } catch (ConcurrentOperationException | StorageUnavailableException e) { - logger.debug("move volume failed", e); - throw new CloudRuntimeException("move volume failed", e); + boolean isClvmLightweightMigration = isClvmLightweightMigrationNeeded( + newVolumeOnPrimaryStorage, existingVolumeOfVm, vm); + + if (isClvmLightweightMigration) { + newVolumeOnPrimaryStorage = executeClvmLightweightMigration( + newVolumeOnPrimaryStorage, vm, existingVolumeOfVm, + "CLVM lightweight migration", "different pools, same VG"); + } else { + StoragePoolVO vmRootVolumePool = _storagePoolDao.findById(existingVolumeOfVm.getPoolId()); + + try { + HypervisorType volumeToAttachHyperType = _volsDao.getHypervisorType(volumeToAttach.getId()); + newVolumeOnPrimaryStorage = _volumeMgr.moveVolume(newVolumeOnPrimaryStorage, vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(), vmRootVolumePool.getClusterId(), + volumeToAttachHyperType); + } catch (ConcurrentOperationException | StorageUnavailableException e) { + logger.debug("move volume failed", e); + throw new CloudRuntimeException("move volume failed", e); + } } } VolumeVO newVol = _volsDao.findById(newVolumeOnPrimaryStorage.getId()); @@ -2631,6 +2662,458 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device return newVol; } + /** + * Helper method to get storage pools for volume and VM. + * + * @param volumeToAttach The volume being attached + * @param vmExistingVolume The VM's existing volume + * @return Pair of StoragePoolVO objects (volumePool, vmPool), or null if either pool is missing + */ + private Pair getStoragePoolsForVolumeAttachment(VolumeInfo volumeToAttach, VolumeVO vmExistingVolume) { + if (volumeToAttach == null || vmExistingVolume == null) { + return null; + } + + StoragePoolVO volumePool = _storagePoolDao.findById(volumeToAttach.getPoolId()); + StoragePoolVO vmPool = _storagePoolDao.findById(vmExistingVolume.getPoolId()); + + if (volumePool == null || vmPool == null) { + return null; + } + + return new Pair<>(volumePool, vmPool); + } + + /** + * Checks if both storage pools are CLVM type. + * + * @param volumePool Storage pool for the volume + * @param vmPool Storage pool for the VM + * @return true if both pools are CLVM type + */ + private boolean areBothPoolsClvmType(StoragePoolVO volumePool, StoragePoolVO vmPool) { + return volumePool.getPoolType() == StoragePoolType.CLVM && + vmPool.getPoolType() == StoragePoolType.CLVM; + } + + /** + * Checks if a storage pool is CLVM type. + * + * @param pool Storage pool to check + * @return true if pool is CLVM type + */ + private boolean isClvmPool(StoragePoolVO pool) { + return pool != null && pool.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); + } + + /** + * Determines if a CLVM volume needs lightweight lock migration instead of full data copy. + * + * Lightweight migration is needed when: + * 1. Volume is on CLVM storage + * 2. Source and destination are in the same Volume Group + * 3. Only the host/lock needs to change (not the storage pool) + * + * @param volumeToAttach The volume being attached + * @param vmExistingVolume The VM's existing volume (typically root volume) + * @param vm The VM to attach the volume to + * @return true if lightweight CLVM lock migration should be used + */ + private boolean isClvmLightweightMigrationNeeded(VolumeInfo volumeToAttach, VolumeVO vmExistingVolume, UserVmVO vm) { + Pair pools = getStoragePoolsForVolumeAttachment(volumeToAttach, vmExistingVolume); + if (pools == null) { + return false; + } + + 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; + } + + /** + * Determines if a CLVM volume requires lock transfer when already on the correct storage pool. + * + * Lock transfer is needed when: + * 1. Volume is already on the same CLVM storage pool as VM's volumes + * 2. But the volume lock is held by a different host than where the VM is running + * 3. Only the lock needs to change (no pool change, no data copy) + * + * @param volumeToAttach The volume being attached + * @param vmExistingVolume The VM's existing volume (typically root volume) + * @param vm The VM to attach the volume to + * @return true if CLVM lock transfer is needed (but not full migration) + */ + private boolean isClvmLockTransferRequired(VolumeInfo volumeToAttach, VolumeVO vmExistingVolume, UserVmVO vm) { + if (vm == null) { + return false; + } + + Pair pools = getStoragePoolsForVolumeAttachment(volumeToAttach, vmExistingVolume); + if (pools == null) { + return false; + } + + StoragePoolVO volumePool = pools.first(); + StoragePoolVO vmPool = pools.second(); + + if (!isClvmPool(volumePool)) { + 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; + } + + /** + * Determines the destination host for CLVM lock migration. + * + * If VM is running, uses the VM's current host. + * If VM is stopped, picks an available UP host from the storage pool's cluster. + * + * @param vm The VM + * @param vmExistingVolume The VM's existing volume (to determine cluster) + * @return Host ID, or null if cannot be determined + */ + private Long determineClvmLockDestinationHost(UserVmVO vm, VolumeVO vmExistingVolume) { + Long destHostId = vm.getHostId(); + if (destHostId != null) { + return destHostId; + } + + if (vmExistingVolume != null && vmExistingVolume.getPoolId() != null) { + StoragePoolVO pool = _storagePoolDao.findById(vmExistingVolume.getPoolId()); + if (pool != null && pool.getClusterId() != null) { + List hosts = _hostDao.findByClusterId(pool.getClusterId()); + if (hosts != null && !hosts.isEmpty()) { + // Pick first available UP host + for (HostVO host : hosts) { + if (host.getStatus() == Status.Up) { + destHostId = host.getId(); + logger.debug("VM {} is stopped, selected host {} from cluster {} for CLVM lock migration", + vm.getUuid(), destHostId, pool.getClusterId()); + return destHostId; + } + } + } + } + } + + return null; + } + + /** + * Executes CLVM lightweight migration with consistent logging and error handling. + * + * This helper method wraps the actual migration logic to eliminate code duplication + * between different CLVM migration scenarios (lock transfer vs. lightweight migration). + * + * @param volume The volume to migrate locks for + * @param vm The VM to attach the volume to + * @param vmExistingVolume The VM's existing volume (to determine target host) + * @param operationType Description of the operation type for logging (e.g., "CLVM lock transfer") + * @param scenarioDescription Description of the scenario for logging (e.g., "same pool to different host") + * @return Updated VolumeInfo after lock migration + * @throws CloudRuntimeException if migration fails + */ + private VolumeInfo executeClvmLightweightMigration(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); + } catch (Exception e) { + logger.error("{} failed for volume {}: {}", + operationType, volume.getUuid(), e.getMessage(), e); + throw new CloudRuntimeException(operationType + " failed", e); + } + } + + /** + * 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). + * + * @param volume The volume to migrate locks for + * @param vm The VM to attach the volume to + * @param vmExistingVolume The VM's existing volume (to determine target host) + * @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); + + Long destHostId = determineClvmLockDestinationHost(vm, vmExistingVolume); + + if (destHostId == null) { + throw new CloudRuntimeException( + "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; + } + + 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. + * + * @param volume The CLVM volume + * @return Host ID that has the exclusive lock, or null if cannot be determined + */ + private Long findClvmVolumeLockHost(VolumeInfo volume) { + // Strategy 1: Check volume_details for a host hint we may have stored + VolumeDetailVO detail = _volsDetailsDao.findDetail(volume.getId(), VolumeInfo.CLVM_LOCK_HOST_ID); + if (detail != null && detail.getValue() != null && !detail.getValue().isEmpty()) { + try { + return Long.parseLong(detail.getValue()); + } catch (NumberFormatException e) { + logger.warn("Invalid clvmLockHostId in volume_details for volume {}: {}", + volume.getUuid(), detail.getValue()); + } + } + + // Strategy 2: If volume was attached to a VM, use that VM's last host + Long instanceId = volume.getInstanceId(); + if (instanceId != null) { + VMInstanceVO vmInstance = _vmInstanceDao.findById(instanceId); + if (vmInstance != null && vmInstance.getHostId() != null) { + return vmInstance.getHostId(); + } + } + + // Strategy 3: Check any host in the pool's cluster + 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; + } + + /** + * Cleans up CLVM lock host tracking detail from volume_details table. + * Called after successful volume deletion to prevent orphaned records. + * + * @param volume The volume being deleted + */ + private void cleanupClvmLockHostDetail(VolumeVO volume) { + try { + VolumeDetailVO detail = _volsDetailsDao.findDetail(volume.getId(), VolumeInfo.CLVM_LOCK_HOST_ID); + if (detail != null) { + logger.debug("Removing CLVM lock host detail for deleted volume {}", volume.getUuid()); + _volsDetailsDao.remove(detail.getId()); + } + } catch (Exception e) { + logger.warn("Failed to clean up CLVM lock host detail for volume {}: {}", + volume.getUuid(), e.getMessage()); + } + } + + /** + * Safely sets or updates the CLVM_LOCK_HOST_ID detail for a volume. + * If the detail already exists, it will be updated. Otherwise, it will be created. + * + * @param volumeId The ID of the volume + * @param hostId The host ID that holds/should hold the CLVM exclusive lock + */ + private void setClvmLockHostId(long volumeId, long hostId) { + VolumeDetailVO existingDetail = _volsDetailsDao.findDetail(volumeId, VolumeInfo.CLVM_LOCK_HOST_ID); + + if (existingDetail != null) { + existingDetail.setValue(String.valueOf(hostId)); + _volsDetailsDao.update(existingDetail.getId(), existingDetail); + logger.debug("Updated CLVM_LOCK_HOST_ID for volume {} to host {}", volumeId, hostId); + } else { + _volsDetailsDao.addDetail(volumeId, VolumeInfo.CLVM_LOCK_HOST_ID, String.valueOf(hostId), false); + logger.debug("Created CLVM_LOCK_HOST_ID for volume {} with host {}", volumeId, hostId); + } + } + + /** + * 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) { + String volumeUuid = volume.getUuid(); + + // Get storage pool info + StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId()); + if (pool == null) { + logger.error("Cannot find storage pool for volume {}", volumeUuid); + return false; + } + + String vgName = pool.getPath(); + if (vgName.startsWith("/")) { + vgName = vgName.substring(1); + } + + // Full LV path: /dev/vgname/volume-path + String lvPath = String.format("/dev/%s/%s", vgName, volume.getPath()); + + try { + // Step 1: Deactivate on source host (if different from dest) + if (!sourceHostId.equals(destHostId)) { + logger.debug("Deactivating CLVM volume {} on source host {}", volumeUuid, sourceHostId); + + ClvmLockTransferCommand deactivateCmd = new ClvmLockTransferCommand( + ClvmLockTransferCommand.Operation.DEACTIVATE, + lvPath, + volumeUuid + ); + + Answer deactivateAnswer = _agentMgr.send(sourceHostId, deactivateCmd); + + if (deactivateAnswer == null || !deactivateAnswer.getResult()) { + String error = deactivateAnswer != null ? deactivateAnswer.getDetails() : "null answer"; + logger.warn("Failed to deactivate CLVM volume {} on source host {}: {}. " + + "Will attempt to activate on destination anyway.", + volumeUuid, sourceHostId, error); + } + } + + // Step 2: Activate exclusively on destination host + logger.debug("Activating CLVM volume {} exclusively on destination host {}", volumeUuid, destHostId); + + ClvmLockTransferCommand activateCmd = new ClvmLockTransferCommand( + ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE, + lvPath, + volumeUuid + ); + + Answer activateAnswer = _agentMgr.send(destHostId, activateCmd); + + if (activateAnswer == null || !activateAnswer.getResult()) { + String error = activateAnswer != null ? activateAnswer.getDetails() : "null answer"; + logger.error("Failed to activate CLVM volume {} exclusively on dest host {}: {}", + volumeUuid, destHostId, error); + return false; + } + + // Step 3: Store the new lock host in volume_details for future reference + setClvmLockHostId(volume.getId(), destHostId); + + logger.info("Successfully transferred CLVM lock for volume {} from host {} to host {}", + volumeUuid, sourceHostId, destHostId); + + return true; + + } catch (AgentUnavailableException | OperationTimedoutException e) { + logger.error("Exception during CLVM lock transfer for volume {}: {}", volumeUuid, e.getMessage(), e); + return false; + } + } + public Volume attachVolumeToVM(Long vmId, Long volumeId, Long deviceId, Boolean allowAttachForSharedFS) { Account caller = CallContext.current().getCallingAccount(); @@ -5381,7 +5864,8 @@ public ConfigKey[] getConfigKeys() { MatchStoragePoolTagsWithDiskOffering, UseHttpsToUpload, WaitDetachDevice, - AllowCheckAndRepairVolume + AllowCheckAndRepairVolume, + CLVMSecureZeroFill }; } } diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index b5a03d1836bb..8109442acab4 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -1667,6 +1667,9 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc if (backupSnapToSecondary) { if (!isKvmAndFileBasedStorage) { backupSnapshotToSecondary(payload.getAsyncBackup(), snapshotStrategy, snapshotOnPrimary, payload.getZoneIds(), payload.getStoragePoolIds()); + if (storagePool.getPoolType() == StoragePoolType.CLVM) { + _snapshotStoreDao.removeBySnapshotStore(snapshotId, snapshotOnPrimary.getDataStore().getId(), snapshotOnPrimary.getDataStore().getRole()); + } } else { postSnapshotDirectlyToSecondary(snapshot, snapshotOnPrimary, snapshotId); }