Skip to content

Commit 3af96c7

Browse files
Pearl1594Pearl1594
committed
revist lock tracking - use lvs command to get lock host than DB
1 parent a29f823 commit 3af96c7

File tree

15 files changed

+907
-85
lines changed

15 files changed

+907
-85
lines changed

‎core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferAnswer.java‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.cloudstack.storage.command;
19+
20+
import com.cloud.agent.api.Answer;
21+
22+
/**
23+
* Answer for ClvmLockTransferCommand, containing lock state information.
24+
* This answer includes the current lock holder information when querying lock state.
25+
*/
26+
public class ClvmLockTransferAnswer extends Answer {
27+
28+
private String currentLockHostname;
29+
private boolean isActive;
30+
private boolean isExclusive;
31+
private String lvAttributes;
32+
33+
public ClvmLockTransferAnswer(ClvmLockTransferCommand cmd, boolean result, String details) {
34+
super(cmd, result, details);
35+
}
36+
37+
public ClvmLockTransferAnswer(ClvmLockTransferCommand cmd, boolean result, String details,
38+
String currentLockHostname, boolean isActive, boolean isExclusive,
39+
String lvAttributes) {
40+
super(cmd, result, details);
41+
this.currentLockHostname = currentLockHostname;
42+
this.isActive = isActive;
43+
this.isExclusive = isExclusive;
44+
this.lvAttributes = lvAttributes;
45+
}
46+
47+
/**
48+
* Get the hostname of the host currently holding the lock (if any).
49+
* This is parsed from the LVM "lv_host" field.
50+
*
51+
* @return hostname or null if no lock is held
52+
*/
53+
public String getCurrentLockHostname() {
54+
return currentLockHostname;
55+
}
56+
57+
public void setCurrentLockHostname(String currentLockHostname) {
58+
this.currentLockHostname = currentLockHostname;
59+
}
60+
61+
/**
62+
* Whether the volume is currently active on any host.
63+
*
64+
* @return true if active, false otherwise
65+
*/
66+
public boolean isActive() {
67+
return isActive;
68+
}
69+
70+
public void setActive(boolean active) {
71+
isActive = active;
72+
}
73+
74+
/**
75+
* Whether the lock is exclusive (as opposed to shared).
76+
* Only meaningful if isActive() is true.
77+
*
78+
* @return true if exclusive lock, false if shared
79+
*/
80+
public boolean isExclusive() {
81+
return isExclusive;
82+
}
83+
84+
public void setExclusive(boolean exclusive) {
85+
isExclusive = exclusive;
86+
}
87+
88+
/**
89+
* Get the raw LV attributes string from lvs command.
90+
* Example: "-wi-a-e---" means active, exclusive
91+
*
92+
* @return LV attributes or null
93+
*/
94+
public String getLvAttributes() {
95+
return lvAttributes;
96+
}
97+
98+
public void setLvAttributes(String lvAttributes) {
99+
this.lvAttributes = lvAttributes;
100+
}
101+
}
102+

‎core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferCommand.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ public enum Operation {
4343
ACTIVATE_EXCLUSIVE("-aey", "activate exclusively"),
4444

4545
/** Activate the volume in shared mode on this host (-asy) */
46-
ACTIVATE_SHARED("-asy", "activate in shared mode");
46+
ACTIVATE_SHARED("-asy", "activate in shared mode"),
47+
48+
/** Query the current lock state (lvs -o lv_attr,lv_host) */
49+
QUERY_LOCK_STATE("query", "query lock state");
4750

4851
private final String lvchangeFlag;
4952
private final String description;

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -861,9 +861,15 @@ private Long getClvmLockHostFromVmVolumes(Long vmId) {
861861

862862
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
863863
if (pool != null && ClvmLockManager.isClvmPoolType(pool.getPoolType())) {
864-
Long lockHostId = clvmLockManager.getClvmLockHostId(volume.getId(), volume.getUuid());
864+
Long lockHostId = clvmLockManager.getClvmLockHostId(
865+
volume.getId(),
866+
volume.getUuid(),
867+
volume.getPath(),
868+
pool,
869+
true
870+
);
865871
if (lockHostId != null) {
866-
logger.debug("Found CLVM lock host {} from existing volume {} of VM {}",
872+
logger.debug("Found actual CLVM lock host {} from volume {} of VM {} via LVM query",
867873
lockHostId, volume.getUuid(), vmId);
868874
return lockHostId;
869875
}
@@ -888,7 +894,13 @@ private void transferClvmLocksForVmStart(List<VolumeVO> volumes, Long destHostId
888894
continue;
889895
}
890896

891-
Long currentLockHost = clvmLockManager.getClvmLockHostId(volume.getId(), volume.getUuid());
897+
Long currentLockHost = clvmLockManager.getClvmLockHostId(
898+
volume.getId(),
899+
volume.getUuid(),
900+
volume.getPath(),
901+
pool,
902+
true
903+
);
892904

893905
if (currentLockHost == null) {
894906
clvmLockManager.setClvmLockHostId(volume.getId(), destHostId);

‎engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -671,8 +671,10 @@ public void testTransferClvmLocksForVmStart_WithClvmVolumes() throws Exception {
671671
Mockito.when(vmInstance.getInstanceName()).thenReturn(MOCK_VM_NAME);
672672

673673
ClvmLockManager clvmLockManager = Mockito.mock(ClvmLockManager.class);
674-
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString())).thenReturn(currentHostId);
675-
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(102L), Mockito.anyString())).thenReturn(currentHostId);
674+
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(),
675+
Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId);
676+
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(102L), Mockito.anyString(),
677+
Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId);
676678
Mockito.when(clvmLockManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(),
677679
Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(true);
678680

@@ -741,7 +743,8 @@ public void testTransferClvmLocksForVmStart_NoLockTransferNeeded() throws Except
741743
VMInstanceVO vmInstance = Mockito.mock(VMInstanceVO.class);
742744

743745
ClvmLockManager clvmLockManager = Mockito.mock(ClvmLockManager.class);
744-
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class))).thenReturn(destHostId);
746+
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class),
747+
ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true))).thenReturn(destHostId);
745748

746749
Mockito.when(storagePoolDao.findById(poolId)).thenReturn(clvmPool);
747750

@@ -773,7 +776,8 @@ public void testTransferClvmLocksForVmStart_EmptyVolumeList() throws Exception {
773776

774777
method.invoke(volumeOrchestrator, new ArrayList<VolumeVO>(), destHostId, vmInstance);
775778

776-
Mockito.verify(clvmLockManager, Mockito.never()).getClvmLockHostId(Mockito.anyLong(), Mockito.anyString());
779+
Mockito.verify(clvmLockManager, Mockito.never()).getClvmLockHostId(Mockito.anyLong(), Mockito.anyString(),
780+
Mockito.anyString(), Mockito.any(), Mockito.anyBoolean());
777781
}
778782

779783
@Test
@@ -813,7 +817,8 @@ public void testTransferClvmLocksForVmStart_SetInitialLockHost() throws Exceptio
813817
VMInstanceVO vmInstance = Mockito.mock(VMInstanceVO.class);
814818

815819
ClvmLockManager clvmLockManager = Mockito.mock(ClvmLockManager.class);
816-
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class))).thenReturn(null);
820+
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class),
821+
ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true))).thenReturn(null);
817822

818823
Mockito.when(storagePoolDao.findById(poolId)).thenReturn(clvmPool);
819824

@@ -858,7 +863,8 @@ public void testTransferClvmLocksForVmStart_MixedVolumes() throws Exception {
858863
Mockito.when(vmInstance.getInstanceName()).thenReturn(MOCK_VM_NAME);
859864

860865
ClvmLockManager clvmLockManager = Mockito.mock(ClvmLockManager.class);
861-
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString())).thenReturn(currentHostId);
866+
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(),
867+
Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId);
862868
Mockito.when(clvmLockManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(),
863869
Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(true);
864870

@@ -898,7 +904,8 @@ public void testTransferClvmLocksForVmStart_TransferFails() throws Throwable {
898904
Mockito.when(vmInstance.getInstanceName()).thenReturn(MOCK_VM_NAME);
899905

900906
ClvmLockManager clvmLockManager = Mockito.mock(ClvmLockManager.class);
901-
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString())).thenReturn(currentHostId);
907+
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(),
908+
Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId);
902909
Mockito.when(clvmLockManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(),
903910
Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(false);
904911

‎engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,13 @@ private void updateLockHostForVolume(EndPoint ep, DataObject volObj) {
342342
StoragePool destPool = (StoragePool) volObj.getDataStore();
343343
if (destPool != null && ClvmLockManager.isClvmPoolType(destPool.getPoolType())) {
344344
Long hostId = ep.getId();
345-
Long existingHostId = clvmLockManager.getClvmLockHostId(volumeInfo.getId(), volumeInfo.getUuid());
345+
Long existingHostId = clvmLockManager.getClvmLockHostId(
346+
volumeInfo.getId(),
347+
volumeInfo.getUuid(),
348+
volumeInfo.getPath(),
349+
destPool,
350+
true
351+
);
346352
if (existingHostId == null) {
347353
clvmLockManager.setClvmLockHostId(volumeInfo.getId(), hostId);
348354
logger.debug("Set lock host ID {} for CLVM volume {} being created from snapshot", hostId, volumeInfo.getId());

‎engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -318,8 +318,10 @@ public void testUpdateLockHostForVolume_CLVMPool_SetsLockHost() throws Exception
318318
Mockito.when(volumeInfo.getDataStore()).thenReturn(dataStore);
319319
Mockito.when(volumeInfo.getId()).thenReturn(volumeId);
320320
Mockito.when(volumeInfo.getUuid()).thenReturn(volumeUuid);
321+
Mockito.when(volumeInfo.getPath()).thenReturn("test-volume-path");
321322
Mockito.when(((StoragePool) dataStore).getPoolType()).thenReturn(Storage.StoragePoolType.CLVM);
322-
Mockito.when(clvmLockManager.getClvmLockHostId(volumeId, volumeUuid)).thenReturn(null);
323+
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(volumeId), Mockito.eq(volumeUuid),
324+
Mockito.anyString(), Mockito.any(StoragePool.class), Mockito.eq(true))).thenReturn(null);
323325

324326
method.invoke(strategy, endPoint, volumeInfo);
325327

@@ -351,8 +353,10 @@ public void testUpdateLockHostForVolume_CLVM_NG_Pool_SetsLockHost() throws Excep
351353
Mockito.when(volumeInfo.getDataStore()).thenReturn(dataStore);
352354
Mockito.when(volumeInfo.getId()).thenReturn(volumeId);
353355
Mockito.when(volumeInfo.getUuid()).thenReturn(volumeUuid);
356+
Mockito.when(volumeInfo.getPath()).thenReturn("test-clvm-ng-volume-path");
354357
Mockito.when(((StoragePool) dataStore).getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG);
355-
Mockito.when(clvmLockManager.getClvmLockHostId(volumeId, volumeUuid)).thenReturn(null);
358+
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(volumeId), Mockito.eq(volumeUuid),
359+
Mockito.anyString(), Mockito.any(StoragePool.class), Mockito.eq(true))).thenReturn(null);
356360

357361
try {
358362
method.invoke(strategy, endPoint, volumeInfo);
@@ -388,7 +392,8 @@ public void testUpdateLockHostForVolume_NonCLVMPool_DoesNotSetLockHost() throws
388392
method.invoke(strategy, endPoint, volumeInfo);
389393

390394
Mockito.verify(clvmLockManager, never()).setClvmLockHostId(any(Long.class), any(Long.class));
391-
Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class));
395+
Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class),
396+
any(String.class), any(StoragePool.class), Mockito.anyBoolean());
392397
}
393398

394399
@Test
@@ -417,13 +422,16 @@ public void testUpdateLockHostForVolume_ExistingLockHost_DoesNotOverwrite() thro
417422
Mockito.when(volumeInfo.getDataStore()).thenReturn(dataStore);
418423
Mockito.when(volumeInfo.getId()).thenReturn(volumeId);
419424
Mockito.when(volumeInfo.getUuid()).thenReturn(volumeUuid);
425+
Mockito.when(volumeInfo.getPath()).thenReturn("existing-lock-volume-path");
420426
Mockito.when(((StoragePool) dataStore).getPoolType()).thenReturn(Storage.StoragePoolType.CLVM);
421-
Mockito.when(clvmLockManager.getClvmLockHostId(volumeId, volumeUuid)).thenReturn(existingHostId);
427+
Mockito.when(clvmLockManager.getClvmLockHostId(Mockito.eq(volumeId), Mockito.eq(volumeUuid),
428+
Mockito.anyString(), Mockito.any(StoragePool.class), Mockito.eq(true))).thenReturn(existingHostId);
422429

423430
method.invoke(strategy, endPoint, volumeInfo);
424431

425432
Mockito.verify(clvmLockManager, never()).setClvmLockHostId(any(Long.class), any(Long.class));
426-
Mockito.verify(clvmLockManager).getClvmLockHostId(volumeId, volumeUuid);
433+
Mockito.verify(clvmLockManager).getClvmLockHostId(Mockito.eq(volumeId), Mockito.eq(volumeUuid),
434+
Mockito.anyString(), Mockito.any(StoragePool.class), Mockito.eq(true));
427435
}
428436

429437
@Test
@@ -446,7 +454,8 @@ public void testUpdateLockHostForVolume_NullEndPoint_DoesNotSetLockHost() throws
446454
method.invoke(strategy, null, volumeInfo);
447455

448456
Mockito.verify(clvmLockManager, never()).setClvmLockHostId(any(Long.class), any(Long.class));
449-
Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class));
457+
Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class),
458+
any(String.class), any(StoragePool.class), Mockito.anyBoolean());
450459
}
451460

452461
@Test
@@ -471,7 +480,8 @@ public void testUpdateLockHostForVolume_NonVolumeDataObject_DoesNotSetLockHost()
471480
method.invoke(strategy, endPoint, snapshotInfo);
472481

473482
Mockito.verify(clvmLockManager, never()).setClvmLockHostId(any(Long.class), any(Long.class));
474-
Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class));
483+
Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class),
484+
any(String.class), any(StoragePool.class), Mockito.anyBoolean());
475485
}
476486

477487
@Test
@@ -496,6 +506,7 @@ public void testUpdateLockHostForVolume_NullPool_DoesNotSetLockHost() throws Exc
496506
method.invoke(strategy, endPoint, volumeInfo);
497507

498508
Mockito.verify(clvmLockManager, never()).setClvmLockHostId(any(Long.class), any(Long.class));
499-
Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class));
509+
Mockito.verify(clvmLockManager, never()).getClvmLockHostId(any(Long.class), any(String.class),
510+
any(String.class), any(StoragePool.class), Mockito.anyBoolean());
500511
}
501512
}

‎engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java‎

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import com.cloud.dc.DedicatedResourceVO;
3434
import com.cloud.dc.dao.DedicatedResourceDao;
3535
import com.cloud.storage.ClvmLockManager;
36-
import com.cloud.storage.VolumeDetailVO;
3736
import com.cloud.storage.dao.VolumeDetailsDao;
3837
import com.cloud.user.Account;
3938
import com.cloud.utils.Pair;
@@ -84,6 +83,8 @@ public class DefaultEndPointSelector implements EndPointSelector {
8483
private PrimaryDataStoreDao _storagePoolDao;
8584
@Inject
8685
private VolumeDetailsDao _volDetailsDao;
86+
@Inject
87+
private ClvmLockManager clvmLockManager;
8788

8889
private static final String VOL_ENCRYPT_COLUMN_NAME = "volume_encryption_support";
8990
private final String findOneHostOnPrimaryStorage = "select t.id from "
@@ -280,6 +281,28 @@ public EndPoint select(DataObject srcData, DataObject destData, boolean volumeEn
280281
}
281282
}
282283

284+
// Check if SOURCE is a CLVM volume with active lock (for operations copying FROM CLVM to secondary storage)
285+
if (srcData instanceof VolumeInfo) {
286+
VolumeInfo srcVolume = (VolumeInfo) srcData;
287+
DataStore srcStore = srcVolume.getDataStore();
288+
if (srcStore.getRole() == DataStoreRole.Primary) {
289+
StoragePoolVO pool = _storagePoolDao.findById(srcStore.getId());
290+
if (pool != null && ClvmLockManager.isClvmPoolType(pool.getPoolType())) {
291+
Long lockHostId = getClvmLockHostId(srcVolume);
292+
if (lockHostId != null) {
293+
logger.info("Routing CLVM volume {} copy operation to source lock holder host {}",
294+
srcVolume.getUuid(), lockHostId);
295+
EndPoint ep = getEndPointFromHostId(lockHostId);
296+
if (ep != null) {
297+
return ep;
298+
}
299+
logger.warn("Could not get endpoint for CLVM lock host {}, falling back to default selection",
300+
lockHostId);
301+
}
302+
}
303+
}
304+
}
305+
283306
// Default behavior for non-CLVM or when no destination host is set
284307
DataStore srcStore = srcData.getDataStore();
285308
DataStore destStore = destData.getDataStore();
@@ -715,22 +738,27 @@ public List<EndPoint> selectAll(DataStore store) {
715738
}
716739

717740
/**
718-
* Retrieves the host ID that currently holds the exclusive lock on a CLVM volume.
719-
* This is tracked in volume_details table for proper routing of delete operations.
741+
/**
742+
* Gets the CLVM lock host ID for a volume by querying actual LVM state.
720743
*
721744
* @param volume The CLVM volume
722-
* @return Host ID holding the lock, or null if not tracked
745+
* @return Host ID holding the lock, or null if not found
723746
*/
724-
private Long getClvmLockHostId(VolumeInfo volume) {
725-
VolumeDetailVO detail = _volDetailsDao.findDetail(volume.getId(), VolumeInfo.CLVM_LOCK_HOST_ID);
726-
if (detail != null && detail.getValue() != null && !detail.getValue().isEmpty()) {
727-
try {
728-
return Long.parseLong(detail.getValue());
729-
} catch (NumberFormatException e) {
730-
logger.warn("Invalid CLVM lock host ID in volume_details for volume {}: {}",
731-
volume.getUuid(), detail.getValue());
732-
}
747+
protected Long getClvmLockHostId(VolumeInfo volume) {
748+
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
749+
750+
Long lockHostId = clvmLockManager.getClvmLockHostId(
751+
volume.getId(),
752+
volume.getUuid(),
753+
volume.getPath(),
754+
pool,
755+
true
756+
);
757+
758+
if (lockHostId != null) {
759+
logger.debug("Found actual lock host {} for volume {} via LVM query", lockHostId, volume.getUuid());
733760
}
734-
return null;
761+
762+
return lockHostId;
735763
}
736764
}

0 commit comments

Comments
 (0)