From 22c4934b199e0fc5e71b96fc4bb2bc66f97a96aa Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 25 Mar 2026 17:44:28 -0400 Subject: [PATCH] Add support for incremental volume snapshots for clvm_ng --- ...LibvirtClvmLockTransferCommandWrapper.java | 10 ++--- .../kvm/storage/KVMStorageProcessor.java | 44 +++++++++++++++++-- .../cloud/storage/VolumeApiServiceImpl.java | 12 +---- .../storage/snapshot/SnapshotManagerImpl.java | 15 ++++--- .../java/com/cloud/vm/UserVmManagerImpl.java | 24 ++++++++-- 5 files changed, 77 insertions(+), 28 deletions(-) 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 index 1ef8ce59b59..907e39e59a9 100644 --- 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 @@ -39,8 +39,8 @@ public class LibvirtClvmLockTransferCommandWrapper 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)); + logger.info("Executing CLVM lock transfer: operation={}, lv={}, volume={}", + operation, lvPath, volumeUuid); try { String lvchangeOpt; @@ -69,20 +69,20 @@ public class LibvirtClvmLockTransferCommandWrapper String result = script.execute(); if (result != null) { - logger.error("CLVM lock transfer failed for volume {}: {}}", + 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 {}}", + 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 {}: {}}", + 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/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 2baafb3d4b2..a4d55dc6a7d 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 @@ -223,6 +223,26 @@ public class KVMStorageProcessor implements StorageProcessor { " \n" + ""; + private static final String DUMMY_VM_XML_BLOCK = "\n" + + " %s\n" + + " 256\n" + + " 256\n" + + " 1\n" + + " \n" + + " hvm\n" + + " \n" + + " \n" + + " \n" + + " %s\n" + + " \n" + + " \n"+ + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + public KVMStorageProcessor(final KVMStoragePoolManager storagePoolMgr, final LibvirtComputingResource resource) { this.storagePoolMgr = storagePoolMgr; @@ -2038,9 +2058,21 @@ public class KVMStorageProcessor implements StorageProcessor { newSnapshot.setPhysicalSize(snapshotSize); } } else if (primaryPool.getType() == StoragePoolType.CLVM || primaryPool.getType() == StoragePoolType.CLVM_NG) { - CreateObjectAnswer result = takeClvmVolumeSnapshotOfStoppedVm(disk, snapshotName); - if (result != null) return result; - newSnapshot.setPath(snapshotPath); + if (primaryPool.getType() == StoragePoolType.CLVM_NG && snapshotTO.isKvmIncrementalSnapshot()) { + if (secondaryPool == null) { + String errorMsg = String.format("Incremental snapshots for CLVM_NG require secondary storage. " + + "Please configure secondary storage or disable incremental snapshots for volume [%s].", volume.getName()); + logger.error(errorMsg); + return new CreateObjectAnswer(errorMsg); + } + logger.info("Taking incremental snapshot of CLVM_NG volume [{}] using QCOW2 backup to secondary storage.", volume.getName()); + newSnapshot = takeIncrementalVolumeSnapshotOfStoppedVm(snapshotTO, primaryPool, secondaryPool, + imageStoreTo.getUrl(), snapshotName, volume, conn, cmd.getWait()); + } else { + CreateObjectAnswer result = takeClvmVolumeSnapshotOfStoppedVm(disk, snapshotName); + if (result != null) return result; + newSnapshot.setPath(snapshotPath); + } } else { if (snapshotTO.isKvmIncrementalSnapshot()) { newSnapshot = takeIncrementalVolumeSnapshotOfStoppedVm(snapshotTO, primaryPool, secondaryPool, imageStoreTo != null ? imageStoreTo.getUrl() : null, snapshotName, volume, conn, cmd.getWait()); @@ -2113,7 +2145,11 @@ public class KVMStorageProcessor implements StorageProcessor { String machine = resource.isGuestAarch64() ? LibvirtComputingResource.VIRT : LibvirtComputingResource.PC; String cpuArch = resource.getGuestCpuArch() != null ? resource.getGuestCpuArch() : "x86_64"; - return String.format(DUMMY_VM_XML, vmName, cpuArch, machine, resource.getHypervisorPath(), primaryPool.getLocalPathFor(volumeObjectTo.getPath())); + String volumePath = primaryPool.getLocalPathFor(volumeObjectTo.getPath()); + boolean isClvmNg = StoragePoolType.CLVM_NG == primaryPool.getType(); + + String xmlTemplate = isClvmNg ? DUMMY_VM_XML_BLOCK : DUMMY_VM_XML; + return String.format(xmlTemplate, vmName, cpuArch, machine, resource.getHypervisorPath(), volumePath); } private SnapshotObjectTO takeIncrementalVolumeSnapshotOfRunningVm(SnapshotObjectTO snapshotObjectTO, KVMStoragePool primaryPool, KVMStoragePool secondaryPool, diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index f93e77143a7..89a94b19809 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2696,16 +2696,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic 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 @@ -2797,7 +2787,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic StoragePoolVO volumePool = pools.first(); StoragePoolVO vmPool = pools.second(); - if (!isClvmPool(volumePool)) { + if (volumePool != null && !ClvmLockManager.isClvmPoolType(volumePool.getPoolType())) { return false; } 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 2ebe2b9522a..b379a556b68 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -1641,7 +1641,8 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement boolean isKvmAndFileBasedStorage = isHypervisorKvmAndFileBasedStorage(volume, storagePool); boolean backupSnapToSecondary = isBackupSnapshotToSecondaryForZone(volume.getDataCenterId()); - if (isKvmAndFileBasedStorage && backupSnapToSecondary) { + StoragePoolType poolType = volume.getStoragePoolType(); + if ((isKvmAndFileBasedStorage || StoragePoolType.CLVM_NG == poolType) && backupSnapToSecondary) { DataStore imageStore = snapshotSrv.findSnapshotImageStore(snapshot); if (imageStore == null) { throw new CloudRuntimeException(String.format("Could not find any secondary storage to allocate snapshot [%s].", snapshot)); @@ -1649,7 +1650,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement snapshot.setImageStore(imageStore); } - updateSnapshotPayload(volume.getPoolId(), payload, isKvmAndFileBasedStorage, clusterId); + updateSnapshotPayload(volume.getPoolId(), payload, isKvmAndFileBasedStorage, poolType, clusterId); snapshot.addPayload(payload); try { @@ -1664,8 +1665,12 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement SnapshotInfo snapshotOnPrimary = snapshotStrategy.takeSnapshot(snapshot); + // For CLVM_NG with incremental snapshots, the snapshot is already created directly on secondary storage + boolean isClvmNgIncremental = storagePool.getPoolType() == StoragePoolType.CLVM_NG && + snapshot.isKvmIncrementalSnapshot(); + if (backupSnapToSecondary) { - if (!isKvmAndFileBasedStorage) { + if (!isKvmAndFileBasedStorage && !isClvmNgIncremental) { backupSnapshotToSecondary(payload.getAsyncBackup(), snapshotStrategy, snapshotOnPrimary, payload.getZoneIds(), payload.getStoragePoolIds()); if (storagePool.getPoolType() == StoragePoolType.CLVM || storagePool.getPoolType() == StoragePoolType.CLVM_NG) { _snapshotStoreDao.removeBySnapshotStore(snapshotId, snapshotOnPrimary.getDataStore().getId(), snapshotOnPrimary.getDataStore().getRole()); @@ -1852,7 +1857,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement } } - private void updateSnapshotPayload(long storagePoolId, CreateSnapshotPayload payload, boolean isKvmAndFileBasedStorage, Long clusterId) { + private void updateSnapshotPayload(long storagePoolId, CreateSnapshotPayload payload, boolean isKvmAndFileBasedStorage, StoragePoolType poolType, Long clusterId) { StoragePoolVO storagePoolVO = _storagePoolDao.findById(storagePoolId); if (storagePoolVO.isManaged()) { @@ -1865,7 +1870,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement payload.setLocationType(null); } - if (isKvmAndFileBasedStorage && kvmIncrementalSnapshot.valueIn(clusterId)) { + if ((isKvmAndFileBasedStorage || StoragePoolType.CLVM_NG == poolType) && kvmIncrementalSnapshot.valueIn(clusterId)) { payload.setKvmIncrementalSnapshot(true); } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 4597ef81965..77db1f1317c 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -5398,7 +5398,19 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir @Override public boolean setupVmForPvlan(boolean add, Long hostId, NicProfile nic) { - if (!nic.getBroadCastUri().getScheme().equals("pvlan")) { + if (nic == null) { + logger.warn("Skipping PVLAN setup on host {} because NIC profile is null", hostId); + return false; + } + + if (nic.getBroadCastUri() == null) { + logger.debug("Skipping PVLAN setup on host {} for NIC {} because broadcast URI is null", hostId, nic); + return false; + } + + String scheme = nic.getBroadCastUri().getScheme(); + if (!"pvlan".equalsIgnoreCase(scheme)) { + logger.debug("Skipping PVLAN setup on host {} for NIC {} because broadcast URI scheme is {}", hostId, nic, scheme); return false; } String op = "add"; @@ -5406,11 +5418,17 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // "delete" would remove all the rules(if using ovs) related to this vm op = "delete"; } - Network network = _networkDao.findById(nic.getNetworkId()); + Host host = _hostDao.findById(hostId); + if (host == null) { + logger.warn("Host with id {} does not exist", hostId); + return false; + } + + Network network = _networkDao.findById(nic.getNetworkId()); String networkTag = _networkModel.getNetworkTag(host.getHypervisorType(), network); PvlanSetupCommand cmd = PvlanSetupCommand.createVmSetup(op, nic.getBroadCastUri(), networkTag, nic.getMacAddress()); - Answer answer = null; + Answer answer; try { answer = _agentMgr.send(hostId, cmd); } catch (OperationTimedoutException e) {