From 77f48ddd9edc483f27a0a9fc8b78ed15d3b5df37 Mon Sep 17 00:00:00 2001 From: Pearl1594 Date: Thu, 22 Oct 2026 15:39:22 -0400 Subject: [PATCH 1/2] Add support for clvm incremental volume snaps --- .../kvm/storage/KVMStorageProcessor.java | 18 ++++++++++-------- .../storage/snapshot/SnapshotManagerImpl.java | 7 ++++--- 2 files changed, 14 insertions(+), 11 deletions(-) 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 0cb32da318ea..c14b875a0f2b 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 @@ -2095,7 +2095,15 @@ public Answer createSnapshot(final CreateObjectCommand cmd) { String diskPath = disk.getPath(); String snapshotPath = diskPath + File.separator + snapshotName; SnapshotObjectTO newSnapshot = new SnapshotObjectTO(); - if (DomainInfo.DomainState.VIR_DOMAIN_RUNNING.equals(state) && !primaryPool.isExternalSnapshot()) { + boolean isClvmNgIncremental = primaryPool.getType() == StoragePoolType.CLVM_NG && snapshotTO.isKvmIncrementalSnapshot(); + if (isClvmNgIncremental && 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); + } + boolean isLiveClvmNgIncremental = isClvmNgIncremental && (DomainInfo.DomainState.VIR_DOMAIN_RUNNING.equals(state) || DomainInfo.DomainState.VIR_DOMAIN_PAUSED.equals(state)); + if ((DomainInfo.DomainState.VIR_DOMAIN_RUNNING.equals(state) && !primaryPool.isExternalSnapshot()) || isLiveClvmNgIncremental) { if (snapshotTO.isKvmIncrementalSnapshot()) { newSnapshot = takeIncrementalVolumeSnapshotOfRunningVm(snapshotTO, primaryPool, secondaryPool, imageStoreTo != null ? imageStoreTo.getUrl() : null, snapshotName, volume, vm, conn, cmd.getWait()); } else { @@ -2109,13 +2117,7 @@ public Answer createSnapshot(final CreateObjectCommand cmd) { newSnapshot.setPhysicalSize(snapshotSize); } } else if (primaryPool.getType() == StoragePoolType.CLVM || primaryPool.getType() == StoragePoolType.CLVM_NG) { - 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); - } + if (isClvmNgIncremental) { 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()); 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 2d23e22a0152..1ee1a1fde648 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -1638,8 +1638,9 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc boolean backupSnapToSecondary = isBackupSnapshotToSecondaryForZone(volume.getDataCenterId()); StoragePoolType poolType = volume.getStoragePoolType(); + boolean isClvmNgIncrementalCandidate = StoragePoolType.CLVM_NG == poolType && kvmIncrementalSnapshot.valueIn(clusterId); - if (isKvmAndFileBasedStorage && backupSnapToSecondary) { + if ((isKvmAndFileBasedStorage || isClvmNgIncrementalCandidate) && 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)); @@ -1663,7 +1664,7 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc SnapshotInfo snapshotOnPrimary = snapshotStrategy.takeSnapshot(snapshot); if (backupSnapToSecondary) { - if (!isKvmAndFileBasedStorage) { + if (!isKvmAndFileBasedStorage && !isClvmNgIncrementalCandidate) { backupSnapshotToSecondary(payload.getAsyncBackup(), snapshotStrategy, snapshotOnPrimary, payload.getZoneIds(), payload.getStoragePoolIds()); if (!payload.getAsyncBackup() && ClvmPoolManager.isClvmPoolType(storagePool.getPoolType())) { _snapshotStoreDao.removeBySnapshotStore(snapshotId, snapshotOnPrimary.getDataStore().getId(), snapshotOnPrimary.getDataStore().getRole()); @@ -1877,7 +1878,7 @@ private void updateSnapshotPayload(long storagePoolId, CreateSnapshotPayload pay payload.setLocationType(null); } - if (isKvmAndFileBasedStorage && kvmIncrementalSnapshot.valueIn(clusterId)) { + if ((isKvmAndFileBasedStorage || StoragePoolType.CLVM_NG == poolType) && kvmIncrementalSnapshot.valueIn(clusterId)) { payload.setKvmIncrementalSnapshot(true); } } From dde3d73fb46ce2c6f832ec1845c6335df4b18b42 Mon Sep 17 00:00:00 2001 From: Pearl1594 Date: Thu, 12 Nov 2026 17:30:54 -0500 Subject: [PATCH 2/2] add lock holder lookup for bitmap deletion path for clvm pooll type --- .../endpoint/DefaultEndPointSelector.java | 7 ++- .../endpoint/DefaultEndPointSelectorTest.java | 45 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) 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 0ce2cedb944c..96a5aef5140a 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 @@ -651,7 +651,12 @@ protected EndPoint getEndPointForBitmapRemoval(DataObject object, boolean encryp VirtualMachine attachedVM = volumeInfo.getAttachedVM(); if (attachedVM.getHostId() != null) { return getEndPointFromHostId(attachedVM.getHostId()); - } else if (attachedVM.getLastHostId() != null) { + } + EndPoint clvmEp = tryRouteToClvmLockHolder(volumeInfo, _storagePoolDao.findById(volumeInfo.getPoolId()), "bitmap removal"); + if (clvmEp != null) { + return clvmEp; + } + if (attachedVM.getLastHostId() != null) { return getEndPointFromHostId(attachedVM.getLastHostId()); } } diff --git a/engine/storage/src/test/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelectorTest.java b/engine/storage/src/test/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelectorTest.java index ca124ec30f04..e9f759b05436 100644 --- a/engine/storage/src/test/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelectorTest.java +++ b/engine/storage/src/test/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelectorTest.java @@ -173,6 +173,51 @@ public void getEndPointForBitmapRemovalTestVolumeIsAttachedNoHostIsSet() { Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).select(volumeInfoMock, false); } + private void mockClvmVolumeOfStoppedVm(Long lockHostId) { + Mockito.doReturn(STORE_ID).when(volumeInfoMock).getPoolId(); + Mockito.when(_storagePoolDao.findById(STORE_ID)).thenReturn(storagePoolVOMock); + Mockito.when(storagePoolVOMock.getPoolType()).thenReturn(StoragePoolType.CLVM_NG); + Mockito.doReturn(lockHostId).when(defaultEndPointSelectorSpy).getClvmLockHostId(volumeInfoMock); + Mockito.doReturn(true).when(volumeInfoMock).isAttachedVM(); + Mockito.doReturn(virtualMachineMock).when(volumeInfoMock).getAttachedVM(); + Mockito.doReturn(null).when(virtualMachineMock).getHostId(); + Mockito.doReturn(HOST_ID).when(virtualMachineMock).getLastHostId(); + } + + @Test + public void getEndPointForBitmapRemovalTestClvmVolumeOfStoppedVmRoutesToLockHolderInsteadOfLastHost() { + mockClvmVolumeOfStoppedVm(DEST_HOST_ID); + Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).getEndPointFromHostId(DEST_HOST_ID); + + EndPoint result = defaultEndPointSelectorSpy.getEndPointForBitmapRemoval(snapshotInfoMock, false); + + assertEquals(endPointMock, result); + Mockito.verify(defaultEndPointSelectorSpy, Mockito.never()).getEndPointFromHostId(HOST_ID); + } + + @Test + public void getEndPointForBitmapRemovalTestClvmVolumeOfStoppedVmWithoutLockHolderFallsBackToLastHost() { + mockClvmVolumeOfStoppedVm(null); + Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).getEndPointFromHostId(HOST_ID); + + EndPoint result = defaultEndPointSelectorSpy.getEndPointForBitmapRemoval(snapshotInfoMock, false); + + assertEquals(endPointMock, result); + } + + @Test + public void getEndPointForBitmapRemovalTestClvmVolumeOfRunningVmUsesVmHostWithoutLockLookup() { + Mockito.doReturn(true).when(volumeInfoMock).isAttachedVM(); + Mockito.doReturn(virtualMachineMock).when(volumeInfoMock).getAttachedVM(); + Mockito.doReturn(HOST_ID).when(virtualMachineMock).getHostId(); + Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).getEndPointFromHostId(HOST_ID); + + EndPoint result = defaultEndPointSelectorSpy.getEndPointForBitmapRemoval(snapshotInfoMock, false); + + assertEquals(endPointMock, result); + Mockito.verify(defaultEndPointSelectorSpy, Mockito.never()).getClvmLockHostId(volumeInfoMock); + } + @Test public void getEndPointForSnapshotOperationsInKvmTestVolumeIsNotAttachedToVMAndSnapshotOnPrimary() { Mockito.doReturn(null).when(volumeInfoMock).getAttachedVM();