From 70583a43f93575a86437aaccb3d7f8f36f7956cb Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 26 Mar 2026 15:43:50 -0400 Subject: [PATCH] prevent snapshot backup for incremental clvm_ng snaps, fix build failure, add unit tests --- .../subsystem/api/storage/VolumeService.java | 4 +- .../storage/volume/VolumeServiceImpl.java | 18 +- .../volume/VolumeServiceImplClvmTest.java | 311 +++++++++++++++ .../storage/snapshot/SnapshotManagerImpl.java | 2 +- .../storage/VolumeApiServiceImplTest.java | 354 ++++++++++++++++++ 5 files changed, 677 insertions(+), 12 deletions(-) create mode 100644 engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeServiceImplClvmTest.java diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java index f93e9607090..a7d82d0b962 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java @@ -177,7 +177,7 @@ public interface VolumeService { * @param vmHostId VM's current host ID (or last host ID if stopped) * @return true if CLVM lock transfer is needed */ - boolean isLockTransferRequired(VolumeInfo volumeToAttach, StoragePoolType volumePoolType, StoragePoolType vmPoolType, + boolean isLockTransferRequired(VolumeInfo volumeToAttach, StoragePoolType volumePoolType, StoragePoolType vmPoolType, Long volumePoolId, Long vmPoolId, Long vmHostId); /** @@ -189,6 +189,6 @@ public interface VolumeService { * @param vmPoolPath Storage pool path for the VM * @return true if lightweight migration should be used */ - boolean isLightweightMigrationNeeded(StoragePoolType volumePoolType, StoragePoolType vmPoolType, + boolean isLightweightMigrationNeeded(StoragePoolType volumePoolType, StoragePoolType vmPoolType, String volumePoolPath, String vmPoolPath); } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 77a7e4911bb..424f3f73afb 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -2976,7 +2976,7 @@ public class VolumeServiceImpl implements VolumeService { return false; } - logger.info("Transferring CLVM lock for volume {} (pool: {}) from host {} to host {}", + logger.info("Transferring CLVM lock for volume {} (pool: {}) from host {} to host {}", volume.getUuid(), pool.getName(), sourceHostId, destHostId); return clvmLockManager.transferClvmVolumeLock(volume.getUuid(), volume.getId(), volume.getPath(), @@ -3000,7 +3000,7 @@ public class VolumeServiceImpl implements VolumeService { if (instanceId != null) { VMInstanceVO vmInstance = vmDao.findById(instanceId); if (vmInstance != null && vmInstance.getHostId() != null) { - logger.debug("Volume {} is attached to VM {} on host {}", + logger.debug("Volume {} is attached to VM {} on host {}", volume.getUuid(), vmInstance.getUuid(), vmInstance.getHostId()); return vmInstance.getHostId(); } @@ -3012,7 +3012,7 @@ public class VolumeServiceImpl implements VolumeService { if (hosts != null && !hosts.isEmpty()) { for (HostVO host : hosts) { if (host.getStatus() == com.cloud.host.Status.Up) { - logger.debug("Using fallback: first UP host {} in cluster {} for volume {}", + logger.debug("Using fallback: first UP host {} in cluster {} for volume {}", host.getId(), pool.getClusterId(), volume.getUuid()); return host.getId(); } @@ -3031,33 +3031,33 @@ public class VolumeServiceImpl implements VolumeService { } String volumeUuid = volume.getUuid(); - logger.info("Starting CLVM lock migration for volume {} (id: {}) to host {}", + logger.info("Starting CLVM lock migration for volume {} (id: {}) to host {}", volumeUuid, volume.getUuid(), destHostId); Long sourceHostId = findVolumeLockHost(volume); if (sourceHostId == null) { - logger.warn("Could not determine source host for CLVM volume {} lock, assuming volume is not exclusively locked", + 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", + 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 {}", + logger.info("Migrating CLVM volume {} lock from host {} to host {}", volumeUuid, sourceHostId, destHostId); boolean success = transferVolumeLock(volume, sourceHostId, destHostId); if (!success) { throw new CloudRuntimeException( - String.format("Failed to transfer CLVM lock for volume %s from host %s to host %s", + 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 {}", + logger.info("Successfully migrated CLVM volume {} lock from host {} to host {}", volumeUuid, sourceHostId, destHostId); return volFactory.getVolume(volume.getId()); diff --git a/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeServiceImplClvmTest.java b/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeServiceImplClvmTest.java new file mode 100644 index 00000000000..5725bd91ffb --- /dev/null +++ b/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeServiceImplClvmTest.java @@ -0,0 +1,311 @@ +// 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.volume; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.when; + +import com.cloud.storage.ClvmLockManager; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; + +/** + * Tests for CLVM lock management methods in VolumeServiceImpl. + */ +@RunWith(MockitoJUnitRunner.class) +public class VolumeServiceImplClvmTest { + + @Spy + @InjectMocks + private VolumeServiceImpl volumeService; + + @Mock + private VolumeDao volumeDao; + + @Mock + private VolumeInfo volumeInfoMock; + + @Mock + private VolumeVO volumeVOMock; + + @Mock + ClvmLockManager clvmLockManager; + + private static final Long VOLUME_ID = 1L; + private static final Long POOL_ID_1 = 100L; + private static final Long POOL_ID_2 = 200L; + private static final Long HOST_ID_1 = 10L; + private static final Long HOST_ID_2 = 20L; + private static final String POOL_PATH_VG1 = "/vg1"; + private static final String POOL_PATH_VG2 = "/vg2"; + + @Before + public void setup() { + when(volumeInfoMock.getId()).thenReturn(VOLUME_ID); + when(volumeInfoMock.getUuid()).thenReturn("test-volume-uuid"); + } + + @Test + public void testAreBothPoolsClvmType_BothCLVM() { + assertTrue(volumeService.areBothPoolsClvmType(StoragePoolType.CLVM, StoragePoolType.CLVM)); + } + + @Test + public void testAreBothPoolsClvmType_BothCLVM_NG() { + assertTrue(volumeService.areBothPoolsClvmType(StoragePoolType.CLVM_NG, StoragePoolType.CLVM_NG)); + } + + @Test + public void testAreBothPoolsClvmType_MixedCLVMAndCLVM_NG() { + assertTrue(volumeService.areBothPoolsClvmType(StoragePoolType.CLVM, StoragePoolType.CLVM_NG)); + assertTrue(volumeService.areBothPoolsClvmType(StoragePoolType.CLVM_NG, StoragePoolType.CLVM)); + } + + @Test + public void testAreBothPoolsClvmType_OneCLVMOneNFS() { + assertFalse(volumeService.areBothPoolsClvmType(StoragePoolType.CLVM, StoragePoolType.NetworkFilesystem)); + assertFalse(volumeService.areBothPoolsClvmType(StoragePoolType.NetworkFilesystem, StoragePoolType.CLVM)); + } + + @Test + public void testAreBothPoolsClvmType_OneCLVM_NGOneNFS() { + assertFalse(volumeService.areBothPoolsClvmType(StoragePoolType.CLVM_NG, StoragePoolType.NetworkFilesystem)); + assertFalse(volumeService.areBothPoolsClvmType(StoragePoolType.NetworkFilesystem, StoragePoolType.CLVM_NG)); + } + + @Test + public void testAreBothPoolsClvmType_BothNFS() { + assertFalse(volumeService.areBothPoolsClvmType(StoragePoolType.NetworkFilesystem, StoragePoolType.NetworkFilesystem)); + } + + @Test + public void testAreBothPoolsClvmType_NullVolumePoolType() { + assertFalse(volumeService.areBothPoolsClvmType(null, StoragePoolType.CLVM)); + } + + @Test + public void testAreBothPoolsClvmType_NullVmPoolType() { + assertFalse(volumeService.areBothPoolsClvmType(StoragePoolType.CLVM, null)); + } + + @Test + public void testAreBothPoolsClvmType_BothNull() { + assertFalse(volumeService.areBothPoolsClvmType(null, null)); + } + + + @Test + public void testIsLockTransferRequired_NonCLVMPool() { + assertFalse(volumeService.isLockTransferRequired( + volumeInfoMock, StoragePoolType.NetworkFilesystem, StoragePoolType.CLVM, + POOL_ID_1, POOL_ID_1, HOST_ID_1)); + } + + @Test + public void testIsLockTransferRequired_DifferentPools() { + assertFalse(volumeService.isLockTransferRequired( + volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM, + POOL_ID_1, POOL_ID_2, HOST_ID_1)); + } + + @Test + public void testIsLockTransferRequired_NullPoolIds() { + assertFalse(volumeService.isLockTransferRequired( + volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM, + null, POOL_ID_1, HOST_ID_1)); + + assertFalse(volumeService.isLockTransferRequired( + volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM, + POOL_ID_1, null, HOST_ID_1)); + } + + @Test + public void testIsLockTransferRequired_DetachedVolumeReady() { + when(volumeDao.findById(VOLUME_ID)).thenReturn(volumeVOMock); + when(volumeVOMock.getState()).thenReturn(Volume.State.Ready); + when(volumeVOMock.getInstanceId()).thenReturn(null); // Detached + + when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(null); + + assertTrue(volumeService.isLockTransferRequired( + volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM, + POOL_ID_1, POOL_ID_1, HOST_ID_1)); + } + + @Test + public void testIsLockTransferRequired_DetachedVolumeNotReady() { + when(volumeDao.findById(VOLUME_ID)).thenReturn(volumeVOMock); + when(volumeVOMock.getState()).thenReturn(Volume.State.Allocated); + + when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(null); + + assertFalse(volumeService.isLockTransferRequired( + volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM, + POOL_ID_1, POOL_ID_1, HOST_ID_1)); + } + + @Test + public void testIsLockTransferRequired_DifferentHosts() { + when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(HOST_ID_1); + + assertTrue(volumeService.isLockTransferRequired( + volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM, + POOL_ID_1, POOL_ID_1, HOST_ID_2)); + } + + @Test + public void testIsLockTransferRequired_SameHost() { + when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(HOST_ID_1); + + assertFalse(volumeService.isLockTransferRequired( + volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM, + POOL_ID_1, POOL_ID_1, HOST_ID_1)); + } + + @Test + public void testIsLockTransferRequired_NullVmHostId() { + when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(HOST_ID_1); + + assertFalse(volumeService.isLockTransferRequired( + volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM, + POOL_ID_1, POOL_ID_1, null)); + } + + @Test + public void testIsLockTransferRequired_CLVM_NG_DifferentHosts() { + when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(HOST_ID_1); + + assertTrue(volumeService.isLockTransferRequired( + volumeInfoMock, StoragePoolType.CLVM_NG, StoragePoolType.CLVM_NG, + POOL_ID_1, POOL_ID_1, HOST_ID_2)); + } + + @Test + public void testIsLightweightMigrationNeeded_NonCLVMPools() { + assertFalse(volumeService.isLightweightMigrationNeeded( + StoragePoolType.NetworkFilesystem, StoragePoolType.NetworkFilesystem, + POOL_PATH_VG1, POOL_PATH_VG1)); + } + + @Test + public void testIsLightweightMigrationNeeded_OneCLVMOneNFS() { + assertFalse(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.NetworkFilesystem, + POOL_PATH_VG1, POOL_PATH_VG1)); + } + + @Test + public void testIsLightweightMigrationNeeded_SameVG() { + assertTrue(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM, + "/vg1", "/vg1")); + } + + @Test + public void testIsLightweightMigrationNeeded_SameVG_NoSlash() { + assertTrue(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM, + "vg1", "vg1")); + } + + @Test + public void testIsLightweightMigrationNeeded_SameVG_MixedSlash() { + assertTrue(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM, + "/vg1", "vg1")); + + assertTrue(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM, + "vg1", "/vg1")); + } + + @Test + public void testIsLightweightMigrationNeeded_DifferentVG() { + assertFalse(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM, + "/vg1", "/vg2")); + } + + @Test + public void testIsLightweightMigrationNeeded_CLVM_NG_SameVG() { + assertTrue(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM_NG, StoragePoolType.CLVM_NG, + "/vg1", "/vg1")); + } + + @Test + public void testIsLightweightMigrationNeeded_CLVM_NG_DifferentVG() { + assertFalse(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM_NG, StoragePoolType.CLVM_NG, + "/vg1", "/vg2")); + } + + @Test + public void testIsLightweightMigrationNeeded_MixedCLVM_CLVM_NG_SameVG() { + assertTrue(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM_NG, + "/vg1", "/vg1")); + + assertTrue(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM_NG, StoragePoolType.CLVM, + "/vg1", "/vg1")); + } + + @Test + public void testIsLightweightMigrationNeeded_NullVolumePath() { + assertFalse(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM, + null, "/vg1")); + } + + @Test + public void testIsLightweightMigrationNeeded_NullVmPath() { + assertFalse(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM, + "/vg1", null)); + } + + @Test + public void testIsLightweightMigrationNeeded_BothPathsNull() { + assertFalse(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM, + null, null)); + } + + @Test + public void testIsLightweightMigrationNeeded_ComplexVGNames() { + assertTrue(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM, + "/cloudstack-vg-01", "/cloudstack-vg-01")); + + assertFalse(volumeService.isLightweightMigrationNeeded( + StoragePoolType.CLVM, StoragePoolType.CLVM, + "/cloudstack-vg-01", "/cloudstack-vg-02")); + } +} 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 b379a556b68..046ddbe7e04 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -1667,7 +1667,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement // For CLVM_NG with incremental snapshots, the snapshot is already created directly on secondary storage boolean isClvmNgIncremental = storagePool.getPoolType() == StoragePoolType.CLVM_NG && - snapshot.isKvmIncrementalSnapshot(); + payload.isKvmIncrementalSnapshot(); if (backupSnapToSecondary) { if (!isKvmAndFileBasedStorage && !isClvmNgIncremental) { diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index ef3ef13a60f..c40cd746d4e 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -2290,4 +2290,358 @@ public class VolumeApiServiceImplTest { Mockito.doReturn(1L).when(mock2).getId(); return List.of(mock1, mock2); } + + @Test + public void testAreBothPoolsClvmType_BothCLVM() { + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Mockito.when(volumePool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(volumeServiceMock.areBothPoolsClvmType( + Storage.StoragePoolType.CLVM, Storage.StoragePoolType.CLVM)).thenReturn(true); + + boolean result = invokePrivateMethod("areBothPoolsClvmType", + new Class[]{StoragePoolVO.class, StoragePoolVO.class}, volumePool, vmPool); + + Assert.assertTrue(result); + Mockito.verify(volumeServiceMock).areBothPoolsClvmType( + Storage.StoragePoolType.CLVM, Storage.StoragePoolType.CLVM); + } + + @Test + public void testAreBothPoolsClvmType_BothCLVM_NG() { + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Mockito.when(volumePool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG); + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG); + Mockito.when(volumeServiceMock.areBothPoolsClvmType( + Storage.StoragePoolType.CLVM_NG, Storage.StoragePoolType.CLVM_NG)).thenReturn(true); + + boolean result = invokePrivateMethod("areBothPoolsClvmType", + new Class[]{StoragePoolVO.class, StoragePoolVO.class}, volumePool, vmPool); + + Assert.assertTrue(result); + } + + @Test + public void testAreBothPoolsClvmType_MixedCLVMAndCLVM_NG() { + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Mockito.when(volumePool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG); + Mockito.when(volumeServiceMock.areBothPoolsClvmType( + Storage.StoragePoolType.CLVM, Storage.StoragePoolType.CLVM_NG)).thenReturn(true); + + boolean result = invokePrivateMethod("areBothPoolsClvmType", + new Class[]{StoragePoolVO.class, StoragePoolVO.class}, volumePool, vmPool); + + Assert.assertTrue(result); + } + + @Test + public void testAreBothPoolsClvmType_OneCLVMOneNFS() { + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Mockito.when(volumePool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + Mockito.when(volumeServiceMock.areBothPoolsClvmType( + Storage.StoragePoolType.CLVM, Storage.StoragePoolType.NetworkFilesystem)).thenReturn(false); + + boolean result = invokePrivateMethod("areBothPoolsClvmType", + new Class[]{StoragePoolVO.class, StoragePoolVO.class}, volumePool, vmPool); + + Assert.assertFalse(result); + } + + @Test + public void testIsClvmLightweightMigrationNeeded_SameVG() { + VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class); + VolumeVO vmExistingVolume = Mockito.mock(VolumeVO.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Long volumePoolId = 100L; + Long vmPoolId = 200L; + + Mockito.when(volumeInfo.getPoolId()).thenReturn(volumePoolId); + Mockito.when(vmExistingVolume.getPoolId()).thenReturn(vmPoolId); + Mockito.when(primaryDataStoreDaoMock.findById(volumePoolId)).thenReturn(volumePool); + Mockito.when(primaryDataStoreDaoMock.findById(vmPoolId)).thenReturn(vmPool); + + Mockito.when(volumePool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(volumePool.getPath()).thenReturn("/vg1"); + Mockito.when(vmPool.getPath()).thenReturn("/vg1"); + + Mockito.when(volumeServiceMock.isLightweightMigrationNeeded( + Storage.StoragePoolType.CLVM, Storage.StoragePoolType.CLVM, + "/vg1", "/vg1")).thenReturn(true); + + boolean result = invokePrivateMethod("isClvmLightweightMigrationNeeded", + new Class[]{VolumeInfo.class, VolumeVO.class, UserVmVO.class}, + volumeInfo, vmExistingVolume, vm); + + Assert.assertTrue(result); + Mockito.verify(volumeServiceMock).isLightweightMigrationNeeded( + Storage.StoragePoolType.CLVM, Storage.StoragePoolType.CLVM, "/vg1", "/vg1"); + } + + @Test + public void testIsClvmLightweightMigrationNeeded_DifferentVG() { + VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class); + VolumeVO vmExistingVolume = Mockito.mock(VolumeVO.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Long volumePoolId = 100L; + Long vmPoolId = 200L; + + Mockito.when(volumeInfo.getPoolId()).thenReturn(volumePoolId); + Mockito.when(vmExistingVolume.getPoolId()).thenReturn(vmPoolId); + Mockito.when(primaryDataStoreDaoMock.findById(volumePoolId)).thenReturn(volumePool); + Mockito.when(primaryDataStoreDaoMock.findById(vmPoolId)).thenReturn(vmPool); + + Mockito.when(volumePool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(volumePool.getPath()).thenReturn("/vg1"); + Mockito.when(vmPool.getPath()).thenReturn("/vg2"); + + Mockito.when(volumeServiceMock.isLightweightMigrationNeeded( + Storage.StoragePoolType.CLVM, Storage.StoragePoolType.CLVM, + "/vg1", "/vg2")).thenReturn(false); + + boolean result = invokePrivateMethod("isClvmLightweightMigrationNeeded", + new Class[]{VolumeInfo.class, VolumeVO.class, UserVmVO.class}, + volumeInfo, vmExistingVolume, vm); + + Assert.assertFalse(result); + } + + @Test + public void testIsClvmLightweightMigrationNeeded_CLVM_NG_SameVG() { + VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class); + VolumeVO vmExistingVolume = Mockito.mock(VolumeVO.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Long volumePoolId = 100L; + Long vmPoolId = 200L; + + Mockito.when(volumeInfo.getPoolId()).thenReturn(volumePoolId); + Mockito.when(vmExistingVolume.getPoolId()).thenReturn(vmPoolId); + Mockito.when(primaryDataStoreDaoMock.findById(volumePoolId)).thenReturn(volumePool); + Mockito.when(primaryDataStoreDaoMock.findById(vmPoolId)).thenReturn(vmPool); + + Mockito.when(volumePool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG); + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG); + Mockito.when(volumePool.getPath()).thenReturn("/vg1"); + Mockito.when(vmPool.getPath()).thenReturn("/vg1"); + + Mockito.when(volumeServiceMock.isLightweightMigrationNeeded( + Storage.StoragePoolType.CLVM_NG, Storage.StoragePoolType.CLVM_NG, + "/vg1", "/vg1")).thenReturn(true); + + boolean result = invokePrivateMethod("isClvmLightweightMigrationNeeded", + new Class[]{VolumeInfo.class, VolumeVO.class, UserVmVO.class}, + volumeInfo, vmExistingVolume, vm); + + Assert.assertTrue(result); + } + + @Test + public void testIsClvmLockTransferRequired_DifferentHosts() { + VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class); + VolumeVO vmExistingVolume = Mockito.mock(VolumeVO.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Long volumePoolId = 100L; + Long vmPoolId = 100L; // Same pool + Long vmHostId = 10L; + + Mockito.when(volumeInfo.getPoolId()).thenReturn(volumePoolId); + Mockito.when(vmExistingVolume.getPoolId()).thenReturn(vmPoolId); + Mockito.when(vm.getHostId()).thenReturn(vmHostId); + Mockito.when(primaryDataStoreDaoMock.findById(volumePoolId)).thenReturn(volumePool); + Mockito.when(primaryDataStoreDaoMock.findById(vmPoolId)).thenReturn(vmPool); + + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(vmPool.getId()).thenReturn(vmPoolId); + + Mockito.when(volumeServiceMock.isLockTransferRequired( + eq(volumeInfo), eq(Storage.StoragePoolType.CLVM), eq(Storage.StoragePoolType.CLVM), + eq(volumePoolId), eq(vmPoolId), eq(vmHostId))).thenReturn(true); + + boolean result = invokePrivateMethod("isClvmLockTransferRequired", + new Class[]{VolumeInfo.class, VolumeVO.class, UserVmVO.class}, + volumeInfo, vmExistingVolume, vm); + + Assert.assertTrue(result); + Mockito.verify(volumeServiceMock).isLockTransferRequired( + eq(volumeInfo), eq(Storage.StoragePoolType.CLVM), eq(Storage.StoragePoolType.CLVM), + eq(volumePoolId), eq(vmPoolId), eq(vmHostId)); + } + + @Test + public void testIsClvmLockTransferRequired_SameHost() { + VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class); + VolumeVO vmExistingVolume = Mockito.mock(VolumeVO.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Long volumePoolId = 100L; + Long vmPoolId = 100L; + Long vmHostId = 10L; + + Mockito.when(volumeInfo.getPoolId()).thenReturn(volumePoolId); + Mockito.when(vmExistingVolume.getPoolId()).thenReturn(vmPoolId); + Mockito.when(vm.getHostId()).thenReturn(vmHostId); + Mockito.when(primaryDataStoreDaoMock.findById(volumePoolId)).thenReturn(volumePool); + Mockito.when(primaryDataStoreDaoMock.findById(vmPoolId)).thenReturn(vmPool); + + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(vmPool.getId()).thenReturn(vmPoolId); + + Mockito.when(volumeServiceMock.isLockTransferRequired( + eq(volumeInfo), eq(Storage.StoragePoolType.CLVM), eq(Storage.StoragePoolType.CLVM), + eq(volumePoolId), eq(vmPoolId), eq(vmHostId))).thenReturn(false); + + boolean result = invokePrivateMethod("isClvmLockTransferRequired", + new Class[]{VolumeInfo.class, VolumeVO.class, UserVmVO.class}, + volumeInfo, vmExistingVolume, vm); + + Assert.assertFalse(result); + } + + @Test + public void testIsClvmLockTransferRequired_DifferentPools() { + VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class); + VolumeVO vmExistingVolume = Mockito.mock(VolumeVO.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Long volumePoolId = 100L; + Long vmPoolId = 200L; // Different pool + Long vmHostId = 10L; + + Mockito.when(volumeInfo.getPoolId()).thenReturn(volumePoolId); + Mockito.when(vmExistingVolume.getPoolId()).thenReturn(vmPoolId); + Mockito.when(vm.getHostId()).thenReturn(vmHostId); + Mockito.when(primaryDataStoreDaoMock.findById(volumePoolId)).thenReturn(volumePool); + Mockito.when(primaryDataStoreDaoMock.findById(vmPoolId)).thenReturn(vmPool); + + Mockito.when(volumePool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(volumePool.getId()).thenReturn(volumePoolId); + Mockito.when(vmPool.getId()).thenReturn(vmPoolId); + + Mockito.when(volumeServiceMock.isLockTransferRequired( + eq(volumeInfo), eq(Storage.StoragePoolType.CLVM), eq(Storage.StoragePoolType.CLVM), + eq(volumePoolId), eq(vmPoolId), eq(vmHostId))).thenReturn(false); + + boolean result = invokePrivateMethod("isClvmLockTransferRequired", + new Class[]{VolumeInfo.class, VolumeVO.class, UserVmVO.class}, + volumeInfo, vmExistingVolume, vm); + + Assert.assertFalse(result); + } + + @Test + public void testIsClvmLockTransferRequired_NonCLVMPool() { + VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class); + VolumeVO vmExistingVolume = Mockito.mock(VolumeVO.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Long volumePoolId = 100L; + Long vmPoolId = 100L; + Long vmHostId = 10L; + + Mockito.when(volumeInfo.getPoolId()).thenReturn(volumePoolId); + Mockito.when(vmExistingVolume.getPoolId()).thenReturn(vmPoolId); + Mockito.when(vm.getHostId()).thenReturn(vmHostId); + Mockito.when(primaryDataStoreDaoMock.findById(volumePoolId)).thenReturn(volumePool); + Mockito.when(primaryDataStoreDaoMock.findById(vmPoolId)).thenReturn(vmPool); + + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(vmPool.getId()).thenReturn(vmPoolId); + + boolean result = invokePrivateMethod("isClvmLockTransferRequired", + new Class[]{VolumeInfo.class, VolumeVO.class, UserVmVO.class}, + volumeInfo, vmExistingVolume, vm); + + Assert.assertFalse(result); + } + + @Test + public void testIsClvmLockTransferRequired_NullVM() { + VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class); + VolumeVO vmExistingVolume = Mockito.mock(VolumeVO.class); + + boolean result = invokePrivateMethod("isClvmLockTransferRequired", + new Class[]{VolumeInfo.class, VolumeVO.class, UserVmVO.class}, + volumeInfo, vmExistingVolume, null); + + Assert.assertFalse(result); + } + + @Test + public void testIsClvmLockTransferRequired_VMStoppedUsesLastHostId() { + VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class); + VolumeVO vmExistingVolume = Mockito.mock(VolumeVO.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePoolVO volumePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO vmPool = Mockito.mock(StoragePoolVO.class); + + Long volumePoolId = 100L; + Long vmPoolId = 100L; + Long lastHostId = 10L; + + Mockito.when(volumeInfo.getPoolId()).thenReturn(volumePoolId); + Mockito.when(vmExistingVolume.getPoolId()).thenReturn(vmPoolId); + Mockito.when(vm.getHostId()).thenReturn(null); // VM is stopped + Mockito.when(vm.getLastHostId()).thenReturn(lastHostId); + Mockito.when(primaryDataStoreDaoMock.findById(volumePoolId)).thenReturn(volumePool); + Mockito.when(primaryDataStoreDaoMock.findById(vmPoolId)).thenReturn(vmPool); + + Mockito.when(vmPool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); + Mockito.when(vmPool.getId()).thenReturn(vmPoolId); + + Mockito.when(volumeServiceMock.isLockTransferRequired( + eq(volumeInfo), eq(Storage.StoragePoolType.CLVM), eq(Storage.StoragePoolType.CLVM), + eq(volumePoolId), eq(vmPoolId), eq(lastHostId))).thenReturn(true); + + boolean result = invokePrivateMethod("isClvmLockTransferRequired", + new Class[]{VolumeInfo.class, VolumeVO.class, UserVmVO.class}, + volumeInfo, vmExistingVolume, vm); + + Assert.assertTrue(result); + Mockito.verify(volumeServiceMock).isLockTransferRequired( + eq(volumeInfo), eq(Storage.StoragePoolType.CLVM), eq(Storage.StoragePoolType.CLVM), + eq(volumePoolId), eq(vmPoolId), eq(lastHostId)); + } + + + private T invokePrivateMethod(String methodName, Class[] paramTypes, Object... params) { + try { + java.lang.reflect.Method method = VolumeApiServiceImpl.class.getDeclaredMethod(methodName, paramTypes); + method.setAccessible(true); + return (T) method.invoke(volumeApiServiceImpl, params); + } catch (Exception e) { + throw new RuntimeException("Failed to invoke method: " + methodName, e); + } + } } + +