diff --git a/core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferAnswer.java b/core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferAnswer.java new file mode 100644 index 00000000000..520839a6a5e --- /dev/null +++ b/core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferAnswer.java @@ -0,0 +1,95 @@ +// 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.Answer; + +/** + * Answer for ClvmLockTransferCommand, containing lock state information. + * This answer includes the current lock holder information when querying lock state. + */ +public class ClvmLockTransferAnswer extends Answer { + + private String currentLockHostname; + private boolean isActive; + private boolean isExclusive; + private String lvAttributes; + + public ClvmLockTransferAnswer(ClvmLockTransferCommand cmd, boolean result, String details) { + super(cmd, result, details); + } + + public ClvmLockTransferAnswer(ClvmLockTransferCommand cmd, boolean result, String details, + String currentLockHostname, boolean isActive, boolean isExclusive, + String lvAttributes) { + super(cmd, result, details); + this.currentLockHostname = currentLockHostname; + this.isActive = isActive; + this.isExclusive = isExclusive; + this.lvAttributes = lvAttributes; + } + + /** + * Get the hostname of the host currently holding the lock (if any). + * This is parsed from the LVM "lv_host" field. + * + * @return hostname or null if no lock is held + */ + public String getCurrentLockHostname() { + return currentLockHostname; + } + + public void setCurrentLockHostname(String currentLockHostname) { + this.currentLockHostname = currentLockHostname; + } + + /** + * Whether the volume is currently active on any host. + * + * @return true if active, false otherwise + */ + public boolean isActive() { + return isActive; + } + + public void setActive(boolean active) { + isActive = active; + } + + /** + * Whether the lock is exclusive (as opposed to shared). + * Only meaningful if isActive() is true. + * + * @return true if exclusive lock, false if shared + */ + public boolean isExclusive() { + return isExclusive; + } + + public void setExclusive(boolean exclusive) { + isExclusive = exclusive; + } + + public String getLvAttributes() { + return lvAttributes; + } + + public void setLvAttributes(String lvAttributes) { + this.lvAttributes = lvAttributes; + } +} 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 index 7d71ba78509..0dc80ea790e 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferCommand.java +++ b/core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferCommand.java @@ -43,7 +43,10 @@ public class ClvmLockTransferCommand extends Command { ACTIVATE_EXCLUSIVE("-aey", "activate exclusively"), /** Activate the volume in shared mode on this host (-asy) */ - ACTIVATE_SHARED("-asy", "activate in shared mode"); + ACTIVATE_SHARED("-asy", "activate in shared mode"), + + /** Query the current lock state (lvs -o lv_attr,lv_host) */ + QUERY_LOCK_STATE("query", "query lock state"); private final String lvchangeFlag; private final String description; 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 7d7bcb410fa..eef22161d93 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 @@ -861,9 +861,15 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId()); if (pool != null && ClvmLockManager.isClvmPoolType(pool.getPoolType())) { - Long lockHostId = clvmLockManager.getClvmLockHostId(volume.getId(), volume.getUuid()); + Long lockHostId = clvmLockManager.getClvmLockHostId( + volume.getId(), + volume.getUuid(), + volume.getPath(), + pool, + true + ); if (lockHostId != null) { - logger.debug("Found CLVM lock host {} from existing volume {} of VM {}", + logger.debug("Found actual CLVM lock host {} from volume {} of VM {} via LVM query", lockHostId, volume.getUuid(), vmId); return lockHostId; } @@ -888,7 +894,13 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati continue; } - Long currentLockHost = clvmLockManager.getClvmLockHostId(volume.getId(), volume.getUuid()); + Long currentLockHost = clvmLockManager.getClvmLockHostId( + volume.getId(), + volume.getUuid(), + volume.getPath(), + pool, + true + ); if (currentLockHost == null) { clvmLockManager.setClvmLockHostId(volume.getId(), destHostId); diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java index cebce60bfdd..d7d6d8eb9d3 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java +++ b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java @@ -671,8 +671,10 @@ public class VolumeOrchestratorTest { Mockito.when(vmInstance.getInstanceName()).thenReturn(MOCK_VM_NAME); ClvmLockManager clvmLockManager = Mockito.mock(ClvmLockManager.class); - Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString())).thenReturn(currentHostId); - Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(102L), Mockito.anyString())).thenReturn(currentHostId); + Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(), + Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId); + Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(102L), Mockito.anyString(), + Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId); Mockito.when(clvmLockManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(true); @@ -741,7 +743,8 @@ public class VolumeOrchestratorTest { VMInstanceVO vmInstance = Mockito.mock(VMInstanceVO.class); ClvmLockManager clvmLockManager = Mockito.mock(ClvmLockManager.class); - Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class))).thenReturn(destHostId); + Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class), + ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true))).thenReturn(destHostId); Mockito.when(storagePoolDao.findById(poolId)).thenReturn(clvmPool); @@ -773,7 +776,8 @@ public class VolumeOrchestratorTest { method.invoke(volumeOrchestrator, new ArrayList(), destHostId, vmInstance); - Mockito.verify(clvmLockManager, Mockito.never()).getClvmLockHostId(Mockito.anyLong(), Mockito.anyString()); + Mockito.verify(clvmLockManager, Mockito.never()).getClvmLockHostId(Mockito.anyLong(), Mockito.anyString(), + Mockito.anyString(), Mockito.any(), Mockito.anyBoolean()); } @Test @@ -813,7 +817,8 @@ public class VolumeOrchestratorTest { VMInstanceVO vmInstance = Mockito.mock(VMInstanceVO.class); ClvmLockManager clvmLockManager = Mockito.mock(ClvmLockManager.class); - Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class))).thenReturn(null); + Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class), + ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true))).thenReturn(null); Mockito.when(storagePoolDao.findById(poolId)).thenReturn(clvmPool); @@ -858,7 +863,8 @@ public class VolumeOrchestratorTest { Mockito.when(vmInstance.getInstanceName()).thenReturn(MOCK_VM_NAME); ClvmLockManager clvmLockManager = Mockito.mock(ClvmLockManager.class); - Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString())).thenReturn(currentHostId); + Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(), + Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId); Mockito.when(clvmLockManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(true); @@ -898,7 +904,8 @@ public class VolumeOrchestratorTest { Mockito.when(vmInstance.getInstanceName()).thenReturn(MOCK_VM_NAME); ClvmLockManager clvmLockManager = Mockito.mock(ClvmLockManager.class); - Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString())).thenReturn(currentHostId); + Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(), + Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId); Mockito.when(clvmLockManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(false); diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index e3b60bc625a..07d1bc4b389 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -342,7 +342,13 @@ public class AncientDataMotionStrategy implements DataMotionStrategy { StoragePool destPool = (StoragePool) volObj.getDataStore(); if (destPool != null && ClvmLockManager.isClvmPoolType(destPool.getPoolType())) { Long hostId = ep.getId(); - Long existingHostId = clvmLockManager.getClvmLockHostId(volumeInfo.getId(), volumeInfo.getUuid()); + Long existingHostId = clvmLockManager.getClvmLockHostId( + volumeInfo.getId(), + volumeInfo.getUuid(), + volumeInfo.getPath(), + destPool, + true + ); if (existingHostId == null) { clvmLockManager.setClvmLockHostId(volumeInfo.getId(), hostId); logger.debug("Set lock host ID {} for CLVM volume {} being created from snapshot", hostId, volumeInfo.getId()); diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java index eefc7312bd9..7976db2f007 100755 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java @@ -318,8 +318,10 @@ public class AncientDataMotionStrategyTest { Mockito.when(volumeInfo.getDataStore()).thenReturn(dataStore); Mockito.when(volumeInfo.getId()).thenReturn(volumeId); Mockito.when(volumeInfo.getUuid()).thenReturn(volumeUuid); + Mockito.when(volumeInfo.getPath()).thenReturn("test-volume-path"); Mockito.when(((StoragePool) dataStore).getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); - Mockito.when(clvmLockManager.getClvmLockHostId(volumeId, volumeUuid)).thenReturn(null); + Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(volumeId), Mockito.eq(volumeUuid), + Mockito.anyString(), Mockito.any(StoragePool.class), Mockito.eq(true))).thenReturn(null); method.invoke(strategy, endPoint, volumeInfo); @@ -351,8 +353,10 @@ public class AncientDataMotionStrategyTest { Mockito.when(volumeInfo.getDataStore()).thenReturn(dataStore); Mockito.when(volumeInfo.getId()).thenReturn(volumeId); Mockito.when(volumeInfo.getUuid()).thenReturn(volumeUuid); + Mockito.when(volumeInfo.getPath()).thenReturn("test-clvm-ng-volume-path"); Mockito.when(((StoragePool) dataStore).getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG); - Mockito.when(clvmLockManager.getClvmLockHostId(volumeId, volumeUuid)).thenReturn(null); + Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(volumeId), Mockito.eq(volumeUuid), + Mockito.anyString(), Mockito.any(StoragePool.class), Mockito.eq(true))).thenReturn(null); try { method.invoke(strategy, endPoint, volumeInfo); @@ -388,7 +392,8 @@ public class AncientDataMotionStrategyTest { method.invoke(strategy, endPoint, volumeInfo); Mockito.verify(clvmLockManager, never()).setClvmLockHostId(any(Long.class), any(Long.class)); - Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class)); + Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class), + any(String.class), any(StoragePool.class), Mockito.anyBoolean()); } @Test @@ -417,13 +422,16 @@ public class AncientDataMotionStrategyTest { Mockito.when(volumeInfo.getDataStore()).thenReturn(dataStore); Mockito.when(volumeInfo.getId()).thenReturn(volumeId); Mockito.when(volumeInfo.getUuid()).thenReturn(volumeUuid); + Mockito.when(volumeInfo.getPath()).thenReturn("existing-lock-volume-path"); Mockito.when(((StoragePool) dataStore).getPoolType()).thenReturn(Storage.StoragePoolType.CLVM); - Mockito.when(clvmLockManager.getClvmLockHostId(volumeId, volumeUuid)).thenReturn(existingHostId); + Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(volumeId), Mockito.eq(volumeUuid), + Mockito.anyString(), Mockito.any(StoragePool.class), Mockito.eq(true))).thenReturn(existingHostId); method.invoke(strategy, endPoint, volumeInfo); Mockito.verify(clvmLockManager, never()).setClvmLockHostId(any(Long.class), any(Long.class)); - Mockito.verify(clvmLockManager).getClvmLockHostId(volumeId, volumeUuid); + Mockito.verify(clvmLockManager).getClvmLockHostId(Mockito.eq(volumeId), Mockito.eq(volumeUuid), + Mockito.anyString(), Mockito.any(StoragePool.class), Mockito.eq(true)); } @Test @@ -446,7 +454,8 @@ public class AncientDataMotionStrategyTest { method.invoke(strategy, null, volumeInfo); Mockito.verify(clvmLockManager, never()).setClvmLockHostId(any(Long.class), any(Long.class)); - Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class)); + Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class), + any(String.class), any(StoragePool.class), Mockito.anyBoolean()); } @Test @@ -471,7 +480,8 @@ public class AncientDataMotionStrategyTest { method.invoke(strategy, endPoint, snapshotInfo); Mockito.verify(clvmLockManager, never()).setClvmLockHostId(any(Long.class), any(Long.class)); - Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class)); + Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class), + any(String.class), any(StoragePool.class), Mockito.anyBoolean()); } @Test @@ -496,6 +506,7 @@ public class AncientDataMotionStrategyTest { method.invoke(strategy, endPoint, volumeInfo); Mockito.verify(clvmLockManager, never()).setClvmLockHostId(any(Long.class), any(Long.class)); - Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class)); + Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class), + any(String.class), any(StoragePool.class), Mockito.anyBoolean()); } } 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 de06d22de5f..d313cc39ebf 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 @@ -33,7 +33,6 @@ import javax.inject.Inject; import com.cloud.dc.DedicatedResourceVO; import com.cloud.dc.dao.DedicatedResourceDao; import com.cloud.storage.ClvmLockManager; -import com.cloud.storage.VolumeDetailVO; import com.cloud.storage.dao.VolumeDetailsDao; import com.cloud.user.Account; import com.cloud.utils.Pair; @@ -84,6 +83,8 @@ public class DefaultEndPointSelector implements EndPointSelector { private PrimaryDataStoreDao _storagePoolDao; @Inject private VolumeDetailsDao _volDetailsDao; + @Inject + private ClvmLockManager clvmLockManager; private static final String VOL_ENCRYPT_COLUMN_NAME = "volume_encryption_support"; private final String findOneHostOnPrimaryStorage = "select t.id from " @@ -280,6 +281,28 @@ public class DefaultEndPointSelector implements EndPointSelector { } } + // Check if SOURCE is a CLVM volume with active lock (for operations copying FROM CLVM to secondary storage) + if (srcData instanceof VolumeInfo) { + VolumeInfo srcVolume = (VolumeInfo) srcData; + DataStore srcStore = srcVolume.getDataStore(); + if (srcStore.getRole() == DataStoreRole.Primary) { + StoragePoolVO pool = _storagePoolDao.findById(srcStore.getId()); + if (pool != null && ClvmLockManager.isClvmPoolType(pool.getPoolType())) { + Long lockHostId = getClvmLockHostId(srcVolume); + if (lockHostId != null) { + logger.info("Routing CLVM volume {} copy operation to source lock holder host {}", + srcVolume.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); + } + } + } + } + // Default behavior for non-CLVM or when no destination host is set DataStore srcStore = srcData.getDataStore(); DataStore destStore = destData.getDataStore(); @@ -715,22 +738,27 @@ public class DefaultEndPointSelector implements EndPointSelector { } /** - * 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. + /** + * Gets the CLVM lock host ID for a volume by querying actual LVM state. * * @param volume The CLVM volume - * @return Host ID holding the lock, or null if not tracked + * @return Host ID holding the lock, or null if not found */ - 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()); - } + protected Long getClvmLockHostId(VolumeInfo volume) { + StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId()); + + Long lockHostId = clvmLockManager.getClvmLockHostId( + volume.getId(), + volume.getUuid(), + volume.getPath(), + pool, + true + ); + + if (lockHostId != null) { + logger.debug("Found actual lock host {} for volume {} via LVM query", lockHostId, volume.getUuid()); } - return null; + + return lockHostId; } } 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 8dc06b3ef10..aee944ad829 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 @@ -17,7 +17,10 @@ package org.apache.cloudstack.storage.endpoint; +import com.cloud.host.Host; +import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor; +import com.cloud.storage.ClvmLockManager; import com.cloud.storage.DataStoreRole; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.VolumeDetailVO; @@ -30,18 +33,23 @@ import org.apache.cloudstack.engine.subsystem.api.storage.Scope; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.StorageAction; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.storage.RemoteHostEndPoint; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.mockito.Mockito.mockStatic; @RunWith(MockitoJUnitRunner.class) public class DefaultEndPointSelectorTest { @@ -73,6 +81,14 @@ public class DefaultEndPointSelectorTest { @Mock private EndPoint endPointMock; + @Mock + ClvmLockManager clvmLockManager; + + @Mock + HostDao hostDao; + + static MockedStatic remoteHostEndPointMock; + @InjectMocks private DefaultEndPointSelector defaultEndPointSelectorSpy = Mockito.spy(new DefaultEndPointSelector()); @@ -82,6 +98,16 @@ public class DefaultEndPointSelectorTest { private static final Long STORE_ID = 100L; private static final String VOLUME_UUID = "test-volume-uuid"; + @BeforeClass + public static void init() { + remoteHostEndPointMock = mockStatic(RemoteHostEndPoint.class); + } + + @AfterClass + public static void close() { + remoteHostEndPointMock.close(); + } + @Before public void setup() { Mockito.doReturn(volumeInfoMock).when(snapshotInfoMock).getBaseVolume(); @@ -331,8 +357,7 @@ public class DefaultEndPointSelectorTest { Mockito.when(volumeInfoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); Mockito.when(_storagePoolDao.findById(STORE_ID)).thenReturn(storagePoolVOMock); Mockito.when(storagePoolVOMock.getPoolType()).thenReturn(StoragePoolType.CLVM); - Mockito.when(_volDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(volumeDetailVOMock); - Mockito.when(volumeDetailVOMock.getValue()).thenReturn(String.valueOf(HOST_ID)); + Mockito.doReturn(HOST_ID).when(defaultEndPointSelectorSpy).getClvmLockHostId(volumeInfoMock); Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).getEndPointFromHostId(HOST_ID); EndPoint result = defaultEndPointSelectorSpy.select(volumeInfoMock, StorageAction.DELETEVOLUME, false); @@ -350,8 +375,7 @@ public class DefaultEndPointSelectorTest { Mockito.when(volumeInfoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); Mockito.when(_storagePoolDao.findById(STORE_ID)).thenReturn(storagePoolVOMock); Mockito.when(storagePoolVOMock.getPoolType()).thenReturn(StoragePoolType.CLVM_NG); - Mockito.when(_volDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(volumeDetailVOMock); - Mockito.when(volumeDetailVOMock.getValue()).thenReturn(String.valueOf(HOST_ID)); + Mockito.doReturn(HOST_ID).when(defaultEndPointSelectorSpy).getClvmLockHostId(volumeInfoMock); Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).getEndPointFromHostId(HOST_ID); EndPoint result = defaultEndPointSelectorSpy.select(volumeInfoMock, StorageAction.DELETEVOLUME, false); @@ -369,7 +393,6 @@ public class DefaultEndPointSelectorTest { Mockito.when(volumeInfoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); Mockito.when(_storagePoolDao.findById(STORE_ID)).thenReturn(storagePoolVOMock); Mockito.when(storagePoolVOMock.getPoolType()).thenReturn(StoragePoolType.CLVM); - Mockito.when(_volDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(null); Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).select(volumeInfoMock, false); EndPoint result = defaultEndPointSelectorSpy.select(volumeInfoMock, StorageAction.DELETEVOLUME, false); @@ -403,8 +426,7 @@ public class DefaultEndPointSelectorTest { Mockito.when(volumeInfoMock.getDataStore()).thenReturn(datastoreMock); Mockito.when(_storagePoolDao.findById(STORE_ID)).thenReturn(storagePoolVOMock); Mockito.when(storagePoolVOMock.getPoolType()).thenReturn(StoragePoolType.CLVM); - Mockito.when(_volDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(volumeDetailVOMock); - Mockito.when(volumeDetailVOMock.getValue()).thenReturn(String.valueOf(HOST_ID)); + Mockito.doReturn(HOST_ID).when(defaultEndPointSelectorSpy).getClvmLockHostId(volumeInfoMock); Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).getEndPointFromHostId(HOST_ID); EndPoint result = defaultEndPointSelectorSpy.select(volumeInfoMock); @@ -421,8 +443,7 @@ public class DefaultEndPointSelectorTest { Mockito.when(volumeInfoMock.getDataStore()).thenReturn(datastoreMock); Mockito.when(_storagePoolDao.findById(STORE_ID)).thenReturn(storagePoolVOMock); Mockito.when(storagePoolVOMock.getPoolType()).thenReturn(StoragePoolType.CLVM_NG); - Mockito.when(_volDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(volumeDetailVOMock); - Mockito.when(volumeDetailVOMock.getValue()).thenReturn(String.valueOf(HOST_ID)); + Mockito.doReturn(HOST_ID).when(defaultEndPointSelectorSpy).getClvmLockHostId(volumeInfoMock); Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).getEndPointFromHostId(HOST_ID); EndPoint result = defaultEndPointSelectorSpy.select(volumeInfoMock); @@ -439,8 +460,10 @@ public class DefaultEndPointSelectorTest { Mockito.when(volumeInfoMock.getDataStore()).thenReturn(datastoreMock); Mockito.when(_storagePoolDao.findById(STORE_ID)).thenReturn(storagePoolVOMock); Mockito.when(storagePoolVOMock.getPoolType()).thenReturn(StoragePoolType.CLVM); - Mockito.when(_volDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(null); Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).select(datastoreMock); + RemoteHostEndPoint ep = Mockito.mock(RemoteHostEndPoint.class); + Host lockHost = Mockito.mock(Host.class); + remoteHostEndPointMock.when(() -> RemoteHostEndPoint.getHypervisorHostEndPoint(lockHost)).thenReturn(ep); EndPoint result = defaultEndPointSelectorSpy.select(volumeInfoMock); @@ -456,8 +479,6 @@ public class DefaultEndPointSelectorTest { Mockito.when(volumeInfoMock.getDataStore()).thenReturn(datastoreMock); Mockito.when(_storagePoolDao.findById(STORE_ID)).thenReturn(storagePoolVOMock); Mockito.when(storagePoolVOMock.getPoolType()).thenReturn(StoragePoolType.CLVM); - Mockito.when(_volDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(volumeDetailVOMock); - Mockito.when(volumeDetailVOMock.getValue()).thenReturn("invalid-host-id"); Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).select(datastoreMock); EndPoint result = defaultEndPointSelectorSpy.select(volumeInfoMock); @@ -474,8 +495,6 @@ public class DefaultEndPointSelectorTest { Mockito.when(volumeInfoMock.getDataStore()).thenReturn(datastoreMock); Mockito.when(_storagePoolDao.findById(STORE_ID)).thenReturn(storagePoolVOMock); Mockito.when(storagePoolVOMock.getPoolType()).thenReturn(StoragePoolType.CLVM); - Mockito.when(_volDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(volumeDetailVOMock); - Mockito.when(volumeDetailVOMock.getValue()).thenReturn(""); Mockito.doReturn(endPointMock).when(defaultEndPointSelectorSpy).select(datastoreMock); EndPoint result = defaultEndPointSelectorSpy.select(volumeInfoMock); 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 d5f0940b3ef..b3fdb04da2c 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 @@ -2990,9 +2990,18 @@ public class VolumeServiceImpl implements VolumeService { return null; } - Long lockHostId = clvmLockManager.getClvmLockHostId(volume.getId(), volume.getUuid()); + StoragePoolVO pool = storagePoolDao.findById(volume.getPoolId()); + + Long lockHostId = clvmLockManager.getClvmLockHostId( + volume.getId(), + volume.getUuid(), + volume.getPath(), + pool, + true + ); + if (lockHostId != null) { - logger.debug("Found explicit lock host {} for volume {}", lockHostId, volume.getUuid()); + logger.debug("Found actual lock host {} for volume {}", lockHostId, volume.getUuid()); return lockHostId; } @@ -3006,7 +3015,6 @@ public class VolumeServiceImpl implements VolumeService { } } - StoragePoolVO pool = storagePoolDao.findById(volume.getPoolId()); if (pool != null && pool.getClusterId() != null) { List hosts = _hostDao.findByClusterId(pool.getClusterId()); if (hosts != null && !hosts.isEmpty()) { 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 index 0684b66a62f..59ff7dc8305 100644 --- 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 @@ -22,6 +22,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.eq; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; @@ -99,6 +100,7 @@ public class VolumeServiceImplClvmTest { public void setup() { when(volumeInfoMock.getId()).thenReturn(VOLUME_ID); when(volumeInfoMock.getUuid()).thenReturn("test-volume-uuid"); + when(volumeInfoMock.getPath()).thenReturn("test-volume-path"); volumeService.storagePoolDao = storagePoolDao; volumeService._hostDao = _hostDao; @@ -390,7 +392,10 @@ public class VolumeServiceImplClvmTest { @Test public void testFindVolumeLockHost_ExplicitLockFound() { - when(clvmLockManager.getClvmLockHostId(VOLUME_ID, "test-volume-uuid")) + when(volumeInfoMock.getPoolId()).thenReturn(POOL_ID_1); + when(storagePoolDao.findById(POOL_ID_1)).thenReturn(storagePoolVOMock); + when(clvmLockManager.getClvmLockHostId( + eq(VOLUME_ID), eq("test-volume-uuid"), eq("test-volume-path"), eq(storagePoolVOMock), eq(true))) .thenReturn(HOST_ID_1); Long result = volumeService.findVolumeLockHost(volumeInfoMock); @@ -399,7 +404,10 @@ public class VolumeServiceImplClvmTest { @Test public void testFindVolumeLockHost_FromAttachedVM() { - when(clvmLockManager.getClvmLockHostId(VOLUME_ID, "test-volume-uuid")) + when(volumeInfoMock.getPoolId()).thenReturn(POOL_ID_1); + when(storagePoolDao.findById(POOL_ID_1)).thenReturn(storagePoolVOMock); + when(clvmLockManager.getClvmLockHostId( + eq(VOLUME_ID), eq("test-volume-uuid"), eq("test-volume-path"), eq(storagePoolVOMock), eq(true))) .thenReturn(null); when(volumeInfoMock.getInstanceId()).thenReturn(100L); when(vmDao.findById(100L)).thenReturn(vmInstanceVOMock); @@ -412,11 +420,12 @@ public class VolumeServiceImplClvmTest { @Test public void testFindVolumeLockHost_FallbackToClusterHost() { - when(clvmLockManager.getClvmLockHostId(VOLUME_ID, "test-volume-uuid")) - .thenReturn(null); - when(volumeInfoMock.getInstanceId()).thenReturn(null); when(volumeInfoMock.getPoolId()).thenReturn(POOL_ID_1); when(storagePoolDao.findById(POOL_ID_1)).thenReturn(storagePoolVOMock); + when(clvmLockManager.getClvmLockHostId( + eq(VOLUME_ID), eq("test-volume-uuid"), eq("test-volume-path"), eq(storagePoolVOMock), eq(true))) + .thenReturn(null); + when(volumeInfoMock.getInstanceId()).thenReturn(null); when(storagePoolVOMock.getClusterId()).thenReturn(10L); when(hostVOMock.getId()).thenReturn(HOST_ID_1); when(hostVOMock.getStatus()).thenReturn(com.cloud.host.Status.Up); @@ -428,11 +437,12 @@ public class VolumeServiceImplClvmTest { @Test public void testFindVolumeLockHost_NoHostFound() { - when(clvmLockManager.getClvmLockHostId(VOLUME_ID, "test-volume-uuid")) - .thenReturn(null); - when(volumeInfoMock.getInstanceId()).thenReturn(null); when(volumeInfoMock.getPoolId()).thenReturn(POOL_ID_1); when(storagePoolDao.findById(POOL_ID_1)).thenReturn(storagePoolVOMock); + when(clvmLockManager.getClvmLockHostId( + eq(VOLUME_ID), eq("test-volume-uuid"), eq("test-volume-path"), eq(storagePoolVOMock), eq(true))) + .thenReturn(null); + when(volumeInfoMock.getInstanceId()).thenReturn(null); when(storagePoolVOMock.getClusterId()).thenReturn(10L); when(_hostDao.findByClusterId(10L)).thenReturn(java.util.Collections.emptyList()); @@ -445,8 +455,10 @@ public class VolumeServiceImplClvmTest { when(volumeInfoMock.getPoolId()).thenReturn(POOL_ID_1); when(volumeInfoMock.getId()).thenReturn(VOLUME_ID); when(volumeInfoMock.getPath()).thenReturn("/dev/vg1/volume-1"); - when(clvmLockManager.getClvmLockHostId(VOLUME_ID, "test-volume-uuid")).thenReturn(HOST_ID_1); when(storagePoolDao.findById(POOL_ID_1)).thenReturn(storagePoolVOMock); + when(clvmLockManager.getClvmLockHostId( + eq(VOLUME_ID), eq("test-volume-uuid"), eq("/dev/vg1/volume-1"), eq(storagePoolVOMock), eq(true))) + .thenReturn(HOST_ID_1); when(storagePoolVOMock.getName()).thenReturn("test-pool"); when(clvmLockManager.transferClvmVolumeLock( "test-volume-uuid", VOLUME_ID, "/dev/vg1/volume-1", storagePoolVOMock, HOST_ID_1, HOST_ID_2)) @@ -459,7 +471,11 @@ public class VolumeServiceImplClvmTest { @Test public void testPerformLockMigration_SameHost() { - when(clvmLockManager.getClvmLockHostId(VOLUME_ID, "test-volume-uuid")).thenReturn(HOST_ID_1); + when(volumeInfoMock.getPoolId()).thenReturn(POOL_ID_1); + when(storagePoolDao.findById(POOL_ID_1)).thenReturn(storagePoolVOMock); + when(clvmLockManager.getClvmLockHostId( + eq(VOLUME_ID), eq("test-volume-uuid"), eq("test-volume-path"), eq(storagePoolVOMock), eq(true))) + .thenReturn(HOST_ID_1); VolumeInfo result = volumeService.performLockMigration(volumeInfoMock, HOST_ID_1); assertEquals(volumeInfoMock, result); @@ -469,10 +485,11 @@ public class VolumeServiceImplClvmTest { public void testPerformLockMigration_SourceHostNull() { when(volumeInfoMock.getPoolId()).thenReturn(POOL_ID_1); when(volumeInfoMock.getId()).thenReturn(VOLUME_ID); - when(clvmLockManager.getClvmLockHostId(VOLUME_ID, "test-volume-uuid")).thenReturn(null); - when(volumeInfoMock.getInstanceId()).thenReturn(null); - when(volumeInfoMock.getPoolId()).thenReturn(POOL_ID_1); when(storagePoolDao.findById(POOL_ID_1)).thenReturn(storagePoolVOMock); + when(clvmLockManager.getClvmLockHostId( + eq(VOLUME_ID), eq("test-volume-uuid"), eq("test-volume-path"), eq(storagePoolVOMock), eq(true))) + .thenReturn(null); + when(volumeInfoMock.getInstanceId()).thenReturn(null); when(storagePoolVOMock.getClusterId()).thenReturn(null); VolumeInfo result = volumeService.performLockMigration(volumeInfoMock, HOST_ID_2); @@ -489,8 +506,10 @@ public class VolumeServiceImplClvmTest { when(volumeInfoMock.getPoolId()).thenReturn(POOL_ID_1); when(volumeInfoMock.getId()).thenReturn(VOLUME_ID); when(volumeInfoMock.getPath()).thenReturn("/dev/vg1/volume-1"); - when(clvmLockManager.getClvmLockHostId(VOLUME_ID, "test-volume-uuid")).thenReturn(HOST_ID_1); when(storagePoolDao.findById(POOL_ID_1)).thenReturn(storagePoolVOMock); + when(clvmLockManager.getClvmLockHostId( + eq(VOLUME_ID), eq("test-volume-uuid"), eq("/dev/vg1/volume-1"), eq(storagePoolVOMock), eq(true))) + .thenReturn(HOST_ID_1); when(storagePoolVOMock.getName()).thenReturn("test-pool"); when(clvmLockManager.transferClvmVolumeLock( "test-volume-uuid", VOLUME_ID, "/dev/vg1/volume-1", storagePoolVOMock, HOST_ID_1, HOST_ID_2)) 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 907e39e59a9..3547838290f 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 @@ -22,10 +22,12 @@ import org.apache.logging.log4j.LogManager; import com.cloud.agent.api.Answer; import org.apache.cloudstack.storage.command.ClvmLockTransferCommand; +import org.apache.cloudstack.storage.command.ClvmLockTransferAnswer; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; import com.cloud.utils.script.Script; +import com.cloud.utils.script.OutputInterpreter; @ResourceWrapper(handles = ClvmLockTransferCommand.class) public class LibvirtClvmLockTransferCommandWrapper @@ -43,6 +45,11 @@ public class LibvirtClvmLockTransferCommandWrapper operation, lvPath, volumeUuid); try { + + if (operation == ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE) { + return handleQueryLockState(cmd, lvPath, volumeUuid); + } + String lvchangeOpt; String operationDesc; switch (operation) { @@ -59,7 +66,7 @@ public class LibvirtClvmLockTransferCommandWrapper operationDesc = "activated in shared mode"; break; default: - return new Answer(cmd, false, "Unknown operation: " + operation); + return new ClvmLockTransferAnswer(cmd, false, "Unknown operation: " + operation); } Script script = new Script("/usr/sbin/lvchange", 30000, logger); @@ -71,20 +78,78 @@ public class LibvirtClvmLockTransferCommandWrapper if (result != null) { logger.error("CLVM lock transfer failed for volume {}: {}", volumeUuid, result); - return new Answer(cmd, false, + return new ClvmLockTransferAnswer(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, + return new ClvmLockTransferAnswer(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()); + return new ClvmLockTransferAnswer(cmd, false, "Exception: " + e.getMessage()); + } + } + + /** + * Query which host currently holds the CLVM lock for a volume. + * Executes: lvs -o lv_attr,lv_host --noheadings + * + * This queries the actual CLVM lock state (source of truth). + * The lv_host attribute shows which host currently has the volume activated. + * + * @return ClvmLockTransferAnswer with lock holder hostname + */ + private Answer handleQueryLockState(ClvmLockTransferCommand cmd, String lvPath, String volumeUuid) { + try { + Script script = new Script("/usr/sbin/lvs", 10000, logger); + script.add("-o"); + script.add("lv_attr,lv_host"); + script.add("--noheadings"); + script.add(lvPath); + + OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser(); + String result = script.execute(parser); + + if (result != null) { + logger.error("Failed to query lock state for volume {}: {}", volumeUuid, result); + return new ClvmLockTransferAnswer(cmd, false, + String.format("lvs command failed: %s", result)); + } + + // Parse output: " -wi-a-e--- host5.example.com" + String output = parser.getLine(); + if (output == null || output.trim().isEmpty()) { + return new ClvmLockTransferAnswer(cmd, false, "No output from lvs command"); + } + + String[] parts = output.trim().split("\\s+"); + if (parts.length < 1) { + return new ClvmLockTransferAnswer(cmd, false, "Invalid lvs output format"); + } + + String lvAttr = parts[0]; + String hostname = parts.length > 1 ? parts[1] : null; + + boolean isActive = lvAttr.length() > 4 && lvAttr.charAt(4) == 'a'; + boolean isExclusive = lvAttr.length() > 5 && lvAttr.charAt(5) == 'e'; + + logger.info("Queried lock state for volume {}: attr={}, hostname={}, active={}, exclusive={}", + volumeUuid, lvAttr, hostname, isActive, isExclusive); + + return new ClvmLockTransferAnswer(cmd, true, + String.format("Lock state: active=%s, exclusive=%s, host=%s", + isActive, isExclusive, hostname != null ? hostname : "none"), + hostname, isActive, isExclusive, lvAttr); + + } catch (Exception e) { + logger.error("Exception during lock state query for volume {}: {}", + volumeUuid, e.getMessage(), e); + return new ClvmLockTransferAnswer(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 fc90c95f420..05591d6feef 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 @@ -138,6 +138,7 @@ import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef.DeviceType; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef.DiscardType; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef.DiskProtocol; +import com.cloud.storage.ClvmLockManager; import com.cloud.storage.JavaStorageLayer; import com.cloud.storage.MigrationOptions; import com.cloud.storage.ScopeType; @@ -619,7 +620,9 @@ public class KVMStorageProcessor implements StorageProcessor { String path = details != null ? details.get(DiskTO.IQN) : null; - storagePoolMgr.connectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), path, details); + if (!ClvmLockManager.isClvmPoolType(primaryStore.getPoolType())) { + storagePoolMgr.connectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), path, details); + } final String volumeName = UUID.randomUUID().toString(); @@ -648,7 +651,9 @@ public class KVMStorageProcessor implements StorageProcessor { final KVMPhysicalDisk newDisk = storagePoolMgr.copyPhysicalDisk(volume, path != null ? path : volumeName, primaryPool, cmd.getWaitInMillSeconds()); resource.createOrUpdateLogFileForCommand(cmd, Command.State.COMPLETED); - storagePoolMgr.disconnectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), path); + if (!ClvmLockManager.isClvmPoolType(primaryStore.getPoolType())) { + storagePoolMgr.disconnectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), path); + } final VolumeObjectTO newVol = new VolumeObjectTO(); 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 b6dfa70d760..e180cc1c074 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 @@ -2260,9 +2260,12 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { } else { destFile = new QemuImgFile(destPath, destFormat); try { - boolean isQCOW2 = PhysicalDiskFormat.QCOW2.equals(sourceFormat); + boolean keepBitmaps = PhysicalDiskFormat.QCOW2.equals(sourceFormat); + if (destPool.getType() == StoragePoolType.CLVM) { + keepBitmaps = false; + } qemu.convert(srcFile, destFile, null, null, new QemuImageOptions(srcFile.getFormat(), srcFile.getFileName(), null), - null, false, isQCOW2); + null, false, keepBitmaps); Map destInfo = qemu.info(destFile); Long virtualSize = Long.parseLong(destInfo.get(QemuImg.VIRTUAL_SIZE)); newDisk.setVirtualSize(virtualSize); diff --git a/server/src/main/java/com/cloud/storage/ClvmLockManager.java b/server/src/main/java/com/cloud/storage/ClvmLockManager.java index 90e5839ea62..adcb774c419 100644 --- a/server/src/main/java/com/cloud/storage/ClvmLockManager.java +++ b/server/src/main/java/com/cloud/storage/ClvmLockManager.java @@ -19,19 +19,22 @@ package com.cloud.storage; import java.util.Arrays; +import java.util.List; import javax.inject.Inject; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.OperationTimedoutException; +import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.Status; import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.dao.VolumeDetailsDao; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.storage.command.ClvmLockTransferCommand; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.command.ClvmLockTransferAnswer; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.stereotype.Component; @@ -51,6 +54,14 @@ public class ClvmLockManager { return Arrays.asList(Storage.StoragePoolType.CLVM, Storage.StoragePoolType.CLVM_NG).contains(poolType); } + /** + * Gets the CLVM lock host ID for a volume, optionally querying actual LVM state. + * + * @param volumeId The volume ID + * @param volumeUuid The volume UUID + * @return Host ID that holds the lock, or null if not found + * @deprecated Use getClvmLockHostId(volumeId, volumeUuid, volumePath, pool, queryActual) instead + */ public Long getClvmLockHostId(Long volumeId, String volumeUuid) { VolumeDetailVO detail = _volsDetailsDao.findDetail(volumeId, VolumeInfo.CLVM_LOCK_HOST_ID); if (detail != null && detail.getValue() != null && !detail.getValue().isEmpty()) { @@ -64,6 +75,31 @@ public class ClvmLockManager { return null; } + /** + * Gets the CLVM lock host ID for a volume, optionally querying actual LVM state. + * This method can query the actual lock state from LVM (source of truth) instead of + * relying solely on potentially stale database records. + * + * @param volumeId The volume ID + * @param volumeUuid The volume UUID + * @param volumePath The LV path (required if queryActual is true) + * @param pool The storage pool (required if queryActual is true) + * @param queryActual If true, queries actual LVM state instead of database + * @return Host ID that holds the lock, or null if not found + */ + public Long getClvmLockHostId(Long volumeId, String volumeUuid, String volumePath, + StoragePool pool, boolean queryActual) { + if (queryActual) { + if (volumePath == null || pool == null) { + logger.warn("Cannot query actual CLVM lock state for volume {} - missing volumePath or pool", volumeUuid); + return getClvmLockHostId(volumeId, volumeUuid); + } + return queryCurrentLockHolder(volumeId, volumeUuid, volumePath, pool, true); + } + + return getClvmLockHostId(volumeId, volumeUuid); + } + /** * 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. @@ -83,6 +119,125 @@ public class ClvmLockManager { logger.debug("Created CLVM_LOCK_HOST_ID for volume {} with host {}", volumeId, hostId); } + /** + * Query LVM to find the actual current lock holder for a volume. + * This is the SOURCE OF TRUTH - it queries the actual LVM state via sanlock/lvmlockd. + * + * @param volumeId The volume ID + * @param volumeUuid The volume UUID + * @param volumePath The LV path (e.g., "vm-123-disk-0") + * @param pool The storage pool + * @param updateDatabase If true, updates the database with the actual value (for debugging/audit) + * @return Host ID of current lock holder, or null if no lock is held or query failed + */ + public Long queryCurrentLockHolder(Long volumeId, String volumeUuid, String volumePath, + StoragePool pool, boolean updateDatabase) { + if (pool == null) { + logger.error("Cannot query CLVM lock for volume {} - pool is null", volumeUuid); + return null; + } + + List hosts = null; + + Long clusterId = pool.getClusterId(); + if (clusterId != null) { + hosts = _hostDao.findByClusterId(clusterId, Host.Type.Routing); + logger.debug("Found {} routing hosts in cluster {} for pool {}", + hosts != null ? hosts.size() : 0, clusterId, pool.getName()); + } + + if ((hosts == null || hosts.isEmpty()) && pool.getDataCenterId() > 0) { + logger.debug("Pool {} is zone-scoped or no hosts in cluster, checking zone {} for available routing hosts", + pool.getName(), pool.getDataCenterId()); + hosts = _hostDao.findByDataCenterId(pool.getDataCenterId()); + } + + if (hosts == null || hosts.isEmpty()) { + logger.warn("No KVM routing hosts found to query CLVM lock state for volume {} (pool: {}, cluster: {}, zone: {})", + volumeUuid, pool.getName(), clusterId, pool.getDataCenterId()); + return null; + } + + logger.debug("Querying lock state for volume {} from {} available hosts", volumeUuid, hosts.size()); + + for (HostVO host : hosts) { + if (host.getStatus() != Status.Up || + host.getType() != com.cloud.host.Host.Type.Routing || + host.getHypervisorType() != Hypervisor.HypervisorType.KVM) { + continue; + } + + String vgName = pool.getPath(); + if (vgName.startsWith("/")) { + vgName = vgName.substring(1); + } + String lvPath = String.format("/dev/%s/%s", vgName, volumePath); + + try { + ClvmLockTransferCommand queryCmd = new ClvmLockTransferCommand( + ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE, + lvPath, + volumeUuid + ); + + Answer answer = _agentMgr.send(host.getId(), queryCmd); + + if (answer == null || !answer.getResult()) { + logger.debug("Failed to query lock state from host {}: {}", + host.getId(), answer != null ? answer.getDetails() : "null answer"); + continue; + } + + if (!(answer instanceof ClvmLockTransferAnswer)) { + logger.warn("Unexpected answer type for query lock state: {}", answer.getClass()); + continue; + } + + ClvmLockTransferAnswer queryAnswer = (ClvmLockTransferAnswer) answer; + String hostname = queryAnswer.getCurrentLockHostname(); + + if (hostname == null || hostname.isEmpty()) { + logger.debug("Volume {} is not locked (no exclusive lock held)", volumeUuid); + if (updateDatabase) { + VolumeDetailVO detail = _volsDetailsDao.findDetail(volumeId, VolumeInfo.CLVM_LOCK_HOST_ID); + if (detail != null) { + _volsDetailsDao.remove(detail.getId()); + } + } + return null; + } + + HostVO lockHost = _hostDao.findByName(hostname); + if (lockHost == null) { + logger.warn("Could not resolve hostname {} to host ID for volume {}", + hostname, volumeUuid); + return null; + } + + Long lockHostId = lockHost.getId(); + logger.info("Queried CLVM lock state for volume {}: locked by host {} ({}), exclusive={}", + volumeUuid, lockHostId, hostname, queryAnswer.isExclusive()); + + if (updateDatabase) { + Long dbHostId = getClvmLockHostId(volumeId, volumeUuid); + if (dbHostId == null || !dbHostId.equals(lockHostId)) { + logger.info("Correcting database: volume {} lock host: {} -> {} (actual)", + volumeUuid, dbHostId, lockHostId); + setClvmLockHostId(volumeId, lockHostId); + } + } + + return lockHostId; + + } catch (AgentUnavailableException | OperationTimedoutException e) { + logger.debug("Could not query host {} for lock state: {}", host.getId(), e.getMessage()); + } + } + + logger.warn("Could not query CLVM lock state for volume {} from any host", volumeUuid); + return null; + } + /** * Cleans up CLVM lock host tracking detail from volume_details table. * Called after successful volume deletion to prevent orphaned records. @@ -103,7 +258,7 @@ public class ClvmLockManager { } public boolean transferClvmVolumeLock(String volumeUuid, Long volumeId, String volumePath, - StoragePoolVO pool, Long sourceHostId, Long destHostId) { + StoragePool pool, Long sourceHostId, Long destHostId) { if (pool == null) { logger.error("Cannot transfer CLVM lock for volume {} - pool is null", volumeUuid); return false; @@ -117,25 +272,36 @@ public class ClvmLockManager { String lvPath = String.format("/dev/%s/%s", vgName, volumePath); try { - if (!sourceHostId.equals(destHostId)) { - HostVO sourceHost = _hostDao.findById(sourceHostId); - if (sourceHost != null && sourceHost.getStatus() == Status.Up) { + Long actualLockHostId = queryCurrentLockHolder(volumeId, volumeUuid, volumePath, pool, false); + + Long hostToDeactivate = actualLockHostId != null ? actualLockHostId : sourceHostId; + + logger.info("Transferring CLVM lock for volume {}: actual holder={}, provided source={}, destination={}", + volumeUuid, actualLockHostId, sourceHostId, destHostId); + + if (hostToDeactivate != null && !hostToDeactivate.equals(destHostId)) { + HostVO deactivateHost = _hostDao.findById(hostToDeactivate); + if (deactivateHost != null && deactivateHost.getStatus() == Status.Up) { ClvmLockTransferCommand deactivateCmd = new ClvmLockTransferCommand( ClvmLockTransferCommand.Operation.DEACTIVATE, lvPath, volumeUuid ); - Answer deactivateAnswer = _agentMgr.send(sourceHostId, deactivateCmd); + Answer deactivateAnswer = _agentMgr.send(hostToDeactivate, deactivateCmd); if (deactivateAnswer == null || !deactivateAnswer.getResult()) { - logger.warn("Failed to deactivate CLVM volume {} on source host {}. Will attempt activation on destination.", - volumeUuid, sourceHostId); + logger.warn("Failed to deactivate CLVM volume {} on host {}. Will attempt activation on destination.", + volumeUuid, hostToDeactivate); + } else { + logger.debug("Successfully deactivated volume {} on host {}", volumeUuid, hostToDeactivate); } } else { - logger.warn("Source host {} is down. Will attempt force claim on destination host {}", - sourceHostId, destHostId); + logger.warn("Host {} (current lock holder) is down. Will attempt force claim on destination host {}", + hostToDeactivate, destHostId); } + } else if (actualLockHostId == null) { + logger.debug("Volume {} has no active lock, will directly activate on destination", volumeUuid); } ClvmLockTransferCommand activateCmd = new ClvmLockTransferCommand( @@ -156,7 +322,7 @@ public class ClvmLockManager { setClvmLockHostId(volumeId, destHostId); logger.info("Successfully transferred CLVM lock for volume {} from host {} to host {}", - volumeUuid, sourceHostId, destHostId); + volumeUuid, actualLockHostId != null ? actualLockHostId : "none", destHostId); return true; diff --git a/server/src/test/java/com/cloud/storage/ClvmLockManagerTest.java b/server/src/test/java/com/cloud/storage/ClvmLockManagerTest.java index 6d71d783ddc..bec64da0a1b 100644 --- a/server/src/test/java/com/cloud/storage/ClvmLockManagerTest.java +++ b/server/src/test/java/com/cloud/storage/ClvmLockManagerTest.java @@ -21,11 +21,14 @@ import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.OperationTimedoutException; +import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.Status; import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.dao.VolumeDetailsDao; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.storage.command.ClvmLockTransferAnswer; import org.apache.cloudstack.storage.command.ClvmLockTransferCommand; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.junit.Assert; @@ -37,6 +40,9 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import java.util.Arrays; +import java.util.Collections; + import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; @@ -69,7 +75,6 @@ public class ClvmLockManagerTest { @Before public void setUp() { - // Reset mocks before each test Mockito.reset(volsDetailsDao, agentMgr, hostDao); } @@ -226,4 +231,367 @@ public class ClvmLockManagerTest { Assert.assertFalse(result); } + + @Test + public void testQueryCurrentLockHolder_NullPool() { + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, null, false); + + Assert.assertNull(result); + verify(hostDao, never()).findByClusterId(anyLong(), any()); + } + + @Test + public void testQueryCurrentLockHolder_NoHostsInCluster() { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getDataCenterId()).thenReturn(1L); + when(pool.getName()).thenReturn("test-pool"); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.emptyList()); + when(hostDao.findByDataCenterId(1L)).thenReturn(Collections.emptyList()); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + verify(hostDao, times(1)).findByClusterId(10L, Host.Type.Routing); + verify(hostDao, times(1)).findByDataCenterId(1L); + } + + @Test + public void testQueryCurrentLockHolder_ZoneScopedPool() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(null); + when(pool.getDataCenterId()).thenReturn(1L); + when(pool.getName()).thenReturn("zone-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByDataCenterId(1L)).thenReturn(Collections.singletonList(host)); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, "host1", true, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + when(hostDao.findByName("host1")).thenReturn(host); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_1, result); + verify(hostDao, never()).findByClusterId(anyLong(), any()); + verify(hostDao, times(1)).findByDataCenterId(1L); + } + + @Test + public void testQueryCurrentLockHolder_SuccessfulQuery() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, "host1", true, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + when(hostDao.findByName("host1")).thenReturn(host); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_1, result); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class)); + } + + @Test + public void testQueryCurrentLockHolder_VolumeNotLocked() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, null, false, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + } + + @Test + public void testQueryCurrentLockHolder_EmptyHostname() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, "", false, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + } + + @Test + public void testQueryCurrentLockHolder_HostnameNotResolved() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, "unknown-host", true, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + when(hostDao.findByName("unknown-host")).thenReturn(null); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + } + + @Test + public void testQueryCurrentLockHolder_QueryFails() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + Answer failedAnswer = new Answer(null, false, "Query failed"); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(failedAnswer); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + } + + @Test + public void testQueryCurrentLockHolder_NullAnswer() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(null); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + } + + @Test + public void testQueryCurrentLockHolder_AgentUnavailableException() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))) + .thenThrow(new AgentUnavailableException("Host unavailable", HOST_ID_1)); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + } + + @Test + public void testQueryCurrentLockHolder_OperationTimedoutException() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))) + .thenThrow(new OperationTimedoutException(null, HOST_ID_1, 0, 0, false)); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + } + + @Test + public void testQueryCurrentLockHolder_UpdateDatabase_MatchingValue() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + VolumeDetailVO detail = new VolumeDetailVO(); + detail.setValue(String.valueOf(HOST_ID_1)); + when(volsDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(detail); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, "host1", true, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + when(hostDao.findByName("host1")).thenReturn(host); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true); + + Assert.assertEquals(HOST_ID_1, result); + verify(volsDetailsDao, never()).update(anyLong(), any()); + verify(volsDetailsDao, never()).addDetail(anyLong(), any(), any(), Mockito.anyBoolean()); + } + + @Test + public void testQueryCurrentLockHolder_UpdateDatabase_DifferentValue() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_2, "host2", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + VolumeDetailVO detail = Mockito.mock(VolumeDetailVO.class); + + detail.setValue(String.valueOf(HOST_ID_1)); + when(volsDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(detail); + when(detail.getId()).thenReturn(99L); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, "host2", true, false, null); + when(agentMgr.send(eq(HOST_ID_2), any(ClvmLockTransferCommand.class))).thenReturn(answer); + when(hostDao.findByName("host2")).thenReturn(host); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true); + + Assert.assertEquals(HOST_ID_2, result); + verify(detail, times(1)).setValue(String.valueOf(HOST_ID_2)); + verify(volsDetailsDao, times(1)).update(eq(99L), eq(detail)); + } + + @Test + public void testQueryCurrentLockHolder_UpdateDatabase_NoExistingDetail() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + when(volsDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(null); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, "host1", true, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + when(hostDao.findByName("host1")).thenReturn(host); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true); + + Assert.assertEquals(HOST_ID_1, result); + verify(volsDetailsDao, times(1)).addDetail(eq(VOLUME_ID), eq(VolumeInfo.CLVM_LOCK_HOST_ID), + eq(String.valueOf(HOST_ID_1)), eq(false)); + } + + @Test + public void testQueryCurrentLockHolder_UpdateDatabase_RemoveDetailWhenUnlocked() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + VolumeDetailVO detail = Mockito.mock(VolumeDetailVO.class); + when(detail.getId()).thenReturn(99L); + when(volsDetailsDao.findDetail(VOLUME_ID, VolumeInfo.CLVM_LOCK_HOST_ID)).thenReturn(detail); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, null, false, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true); + + Assert.assertNull(result); + verify(volsDetailsDao, times(1)).remove(99L); + } + + @Test + public void testQueryCurrentLockHolder_SkipsNonKVMHosts() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO xenHost = createMockHost(10L, "xen-host", Status.Up, Hypervisor.HypervisorType.XenServer); + HostVO kvmHost = createMockHost(HOST_ID_1, "kvm-host", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Arrays.asList(xenHost, kvmHost)); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, "kvm-host", true, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + when(hostDao.findByName("kvm-host")).thenReturn(kvmHost); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_1, result); + verify(agentMgr, never()).send(eq(10L), any(ClvmLockTransferCommand.class)); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class)); + } + + @Test + public void testQueryCurrentLockHolder_SkipsDownHosts() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + + HostVO downHost = createMockHost(10L, "down-host", Status.Down, Hypervisor.HypervisorType.KVM); + HostVO upHost = createMockHost(HOST_ID_1, "up-host", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Arrays.asList(downHost, upHost)); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, "up-host", true, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + when(hostDao.findByName("up-host")).thenReturn(upHost); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_1, result); + verify(agentMgr, never()).send(eq(10L), any(ClvmLockTransferCommand.class)); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class)); + } + + @Test + public void testQueryCurrentLockHolder_PathWithLeadingSlash() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn("/" + VG_NAME); + + HostVO host = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Collections.singletonList(host)); + + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(null, true, null, "host1", true, false, null); + when(agentMgr.send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class))).thenReturn(answer); + when(hostDao.findByName("host1")).thenReturn(host); + + Long result = clvmLockManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_1, result); + } + // Helper method to create mock hosts + private HostVO createMockHost(Long id, String name, Status status, Hypervisor.HypervisorType hypervisor) { + HostVO host = Mockito.mock(HostVO.class); + Mockito.lenient().when(host.getId()).thenReturn(id); + Mockito.lenient().when(host.getName()).thenReturn(name); + Mockito.lenient().when(host.getStatus()).thenReturn(status); + Mockito.lenient().when(host.getType()).thenReturn(Host.Type.Routing); + Mockito.lenient().when(host.getHypervisorType()).thenReturn(hypervisor); + return host; + } }