CLVM/CLVM_NG: Host HA support, lock retry, lease-based lock holder lookup and template LV cleanup fix

api/src/main/java/com/cloud/agent/api/to/HostTO.java

@@ -25,6 +25,7 @@ public class HostTO {
     private NetworkTO storageNetwork1;
     private NetworkTO storageNetwork2;
     private String parent;
+    private Long sanlockHostId;
 
     protected HostTO() {
     }
@@ -46,6 +47,14 @@ public HostTO(Host vo) {
         }
     }
 
+    public Long getSanlockHostId() {
+        return sanlockHostId;
+    }
+
+    public void setSanlockHostId(Long sanlockHostId) {
+        this.sanlockHostId = sanlockHostId;
+    }
+
     public String getGuid() {
         return guid;
     }

api/src/main/java/com/cloud/host/Host.java

@@ -60,6 +60,7 @@ public static String[] toStrings(Host.Type... types) {
     String HOST_RBD_VOLUME_ENCRYPTION = "host.volume.encryption.rbd";
     String HOST_INSTANCE_CONVERSION = "host.instance.conversion";
     String HOST_VDDK_SUPPORT = "host.vddk.support";
+    String HOST_SANLOCK_ID = "host.sanlock.id";
     String HOST_VDDK_LIB_DIR = "vddk.lib.dir";
     String HOST_VDDK_VERSION = "host.vddk.version";
     String HOST_VDDK_BLOCKCOPY_SUPPORT = "host.vddk.blockcopy.support";

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

@@ -17,6 +17,8 @@
 
 package org.apache.cloudstack.storage.clvm.command;
 
+import java.util.List;
+
 import com.cloud.agent.api.Answer;
 
 /**
@@ -25,7 +27,17 @@
  */
 public class ClvmLockTransferAnswer extends Answer {
 
+    /** Mode in which the sanlock lease of a volume is held. */
+    public enum LeaseMode {
+        FREE, EXCLUSIVE, SHARED
+    }
+
     private String currentLockHostname;
+    private LeaseMode leaseMode;
+    private Long leaseOwnerId;
+    private boolean leaseOwnerAlive;
+    private boolean leaseOwnerRenewing;
+    private List<Long> sharedOwnerIds;
     private boolean isActive;
     private boolean isOpen;
     private String lvAttributes;
@@ -80,6 +92,46 @@ public void setOpen(boolean open) {
         isOpen = open;
     }
 
+    public static ClvmLockTransferAnswer leaseOwner(ClvmLockTransferCommand cmd, LeaseMode mode, Long ownerId,
+                                                    boolean ownerAlive, List<Long> sharedOwnerIds) {
+        return leaseOwner(cmd, mode, ownerId, ownerAlive, ownerAlive, sharedOwnerIds);
+    }
+
+    public static ClvmLockTransferAnswer leaseOwner(ClvmLockTransferCommand cmd, LeaseMode mode, Long ownerId,
+                                                    boolean ownerAlive, boolean ownerRenewing, List<Long> sharedOwnerIds) {
+        ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(cmd, true,
+                String.format("Lease %s, owner=%s, alive=%s, shared=%s", mode, ownerId, ownerAlive, sharedOwnerIds));
+        answer.leaseMode = mode;
+        answer.leaseOwnerId = ownerId;
+        answer.leaseOwnerAlive = ownerAlive;
+        answer.leaseOwnerRenewing = ownerRenewing;
+        answer.sharedOwnerIds = sharedOwnerIds;
+        return answer;
+    }
+
+    public LeaseMode getLeaseMode() {
+        return leaseMode;
+    }
+
+    /** sanlock host id of the exclusive holder, see {@link com.cloud.host.Host#HOST_SANLOCK_ID} */
+    public Long getLeaseOwnerId() {
+        return leaseOwnerId;
+    }
+
+    /** false when the exclusive holder stopped renewing its lockspace lease or was restarted since */
+    public boolean isLeaseOwnerAlive() {
+        return leaseOwnerAlive;
+    }
+
+    /** true while the exclusive holder still renews its lockspace lease, so its lease will not expire */
+    public boolean isLeaseOwnerRenewing() {
+        return leaseOwnerRenewing;
+    }
+
+    public List<Long> getSharedOwnerIds() {
+        return sharedOwnerIds;
+    }
+
     public String getLvAttributes() {
         return lvAttributes;
     }

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

@@ -46,7 +46,10 @@ public enum Operation {
         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");
+        QUERY_LOCK_STATE("query", "query lock state"),
+
+        /** Read the sanlock lease of the volume, which names the lock holder from any host in the lockspace */
+        QUERY_LEASE_OWNER("query", "query lease owner");
 
         private final String lvchangeFlag;
         private final String description;

engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java

@@ -33,7 +33,7 @@
  */
 public interface HighAvailabilityManager extends Manager {
 
-    List<StoragePoolType> LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint);
+    List<StoragePoolType> LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint, StoragePoolType.CLVM, StoragePoolType.CLVM_NG);
 
     ConfigKey<Boolean> ForceHA = new ConfigKey<>("Advanced", Boolean.class, "force.ha", "false",
         "Force High-Availability to happen even if the VM says no.", true, Cluster);

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

@@ -881,6 +881,7 @@ private Long getClvmLockHostFromVmVolumes(Long vmId) {
             return null;
         }
 
+        ClvmPoolManager.LockLookup lockLookup = clvmPoolManager.newLockLookup();
         for (VolumeVO volume : vmVolumes) {
             if (volume.getPoolId() == null) {
                 continue;
@@ -895,7 +896,8 @@ private Long getClvmLockHostFromVmVolumes(Long vmId) {
                     volume.getUuid(),
                     volume.getPath(),
                     pool,
-                    true
+                    true,
+                    lockLookup
             );
             if (lockHostId != null) {
                 logger.debug("Found actual CLVM lock host {} from volume {} of VM {} via LVM query",
@@ -912,6 +914,7 @@ private void transferClvmLocksForVmStart(List<VolumeVO> volumes, Long destHostId
             return;
         }
 
+        ClvmPoolManager.LockLookup lockLookup = clvmPoolManager.newLockLookup(destHostId);
         for (VolumeVO volume : volumes) {
             if (volume.getPoolId() == null) {
                 continue;
@@ -927,7 +930,8 @@ private void transferClvmLocksForVmStart(List<VolumeVO> volumes, Long destHostId
                     volume.getUuid(),
                     volume.getPath(),
                     pool,
-                    true
+                    true,
+                    lockLookup
             );
 
             if (currentLockHost == null) {

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

@@ -135,15 +135,15 @@ public void setUp() throws Exception {
                 accountPSResourceCount += increment;
             }
             return null;
-        }).when(resourceLimitMgr).incrementResourceCount(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyBoolean(), Mockito.anyLong());
+        }).when(resourceLimitMgr).incrementResourceCount(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyBoolean(), Mockito.any());
         Mockito.doAnswer((Answer<Void>) invocation -> {
             Resource.ResourceType type = (Resource.ResourceType)invocation.getArguments()[1];
             Long decrement = (Long)invocation.getArguments()[3];
             if (Resource.ResourceType.primary_storage.equals(type)) {
                 accountPSResourceCount -= decrement;
             }
             return null;
-        }).when(resourceLimitMgr).decrementResourceCount(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyBoolean(), Mockito.anyLong());
+        }).when(resourceLimitMgr).decrementResourceCount(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyBoolean(), Mockito.any());
     }
 
     private void runCheckAndUpdateVolumeAccountResourceCountTest(Long originalSize, Long newSize) {
@@ -696,9 +696,9 @@ public void testTransferClvmLocksForVmStart_WithClvmVolumes() throws Exception {
 
         ClvmPoolManager clvmPoolManager = Mockito.mock(ClvmPoolManager.class);
         Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(),
-                Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId);
+                Mockito.anyString(), Mockito.any(), Mockito.eq(true), Mockito.any())).thenReturn(currentHostId);
         Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(102L), Mockito.anyString(),
-                Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId);
+                Mockito.anyString(), Mockito.any(), Mockito.eq(true), Mockito.any())).thenReturn(currentHostId);
         Mockito.when(clvmPoolManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(),
             Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(true);
 
@@ -768,7 +768,7 @@ public void testTransferClvmLocksForVmStart_NoLockTransferNeeded() throws Except
 
         ClvmPoolManager clvmPoolManager = Mockito.mock(ClvmPoolManager.class);
         Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class),
-                ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true))).thenReturn(destHostId);
+                ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true), Mockito.any())).thenReturn(destHostId);
 
         Mockito.when(storagePoolDao.findById(poolId)).thenReturn(clvmPool);
 
@@ -801,7 +801,7 @@ public void testTransferClvmLocksForVmStart_EmptyVolumeList() throws Exception {
         method.invoke(volumeOrchestrator, new ArrayList<VolumeVO>(), destHostId, vmInstance);
 
         Mockito.verify(clvmPoolManager, Mockito.never()).getClvmLockHostId(Mockito.anyLong(), Mockito.anyString(),
-                Mockito.anyString(), Mockito.any(), Mockito.anyBoolean());
+                Mockito.anyString(), Mockito.any(), Mockito.anyBoolean(), Mockito.any());
     }
 
     @Test
@@ -842,7 +842,7 @@ public void testTransferClvmLocksForVmStart_SetInitialLockHost() throws Exceptio
 
         ClvmPoolManager clvmPoolManager = Mockito.mock(ClvmPoolManager.class);
         Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class),
-                ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true))).thenReturn(null);
+                ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true), Mockito.any())).thenReturn(null);
 
         Mockito.when(storagePoolDao.findById(poolId)).thenReturn(clvmPool);
 
@@ -888,7 +888,7 @@ public void testTransferClvmLocksForVmStart_MixedVolumes() throws Exception {
 
         ClvmPoolManager clvmPoolManager = Mockito.mock(ClvmPoolManager.class);
         Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(),
-                Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId);
+                Mockito.anyString(), Mockito.any(), Mockito.eq(true), Mockito.any())).thenReturn(currentHostId);
         Mockito.when(clvmPoolManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(),
             Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(true);
 
@@ -929,7 +929,7 @@ public void testTransferClvmLocksForVmStart_TransferFails() throws Throwable {
 
         ClvmPoolManager clvmPoolManager = Mockito.mock(ClvmPoolManager.class);
         Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(),
-                Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId);
+                Mockito.anyString(), Mockito.any(), Mockito.eq(true), Mockito.any())).thenReturn(currentHostId);
         Mockito.when(clvmPoolManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(),
             Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(false);
 

engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java

@@ -30,6 +30,7 @@
 import com.cloud.configuration.ConfigurationManager;
 import com.cloud.dc.dao.DataCenterDao;
 import com.cloud.exception.StorageConflictException;
+import com.cloud.host.Host;
 import com.cloud.host.HostVO;
 import com.cloud.host.dao.HostDao;
 import com.cloud.network.NetworkModel;
@@ -100,6 +101,8 @@
     ConfigurationManager configManager;
     @Inject
     NetworkDao networkDao;
+    @Inject
+    ClvmPoolManager clvmPoolManager;
 
     @Override
     public boolean hostAdded(long hostId) {
@@ -193,11 +196,27 @@
 
         storageService.updateStorageCapabilities(poolId, false);
 
+        if (ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
+            alertOnSharedSanlockId(pool, host);
+        }
+
         logger.info("Connection established between storage pool {} and host {}", pool, host);
 
         return createPersistentNetworkResourcesOnHost(hostId);
     }
 
+    /** CLVM lock holder lookups map a sanlock host id to exactly one host of the pool. */
+    private void alertOnSharedSanlockId(StoragePool pool, HostVO host) {
+        List<Long> otherHostIds = clvmPoolManager.findHostsSharingSanlockId(pool, host.getId());
+        if (otherHostIds.isEmpty()) {
+            return;
+        }
+        String msg = String.format("Host %s reports the same %s as hosts %s of CLVM storage pool %s. Set a unique host_id in lvmlocal.conf on each host.",
+                host, Host.HOST_SANLOCK_ID, otherHostIds, pool);
+        logger.error(msg);
+        alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, pool.getDataCenterId(), pool.getPodId(), msg, msg);
+    }
+
     private void updateStoragePoolHostVOAndDetails(StoragePool pool, long hostId, ModifyStoragePoolAnswer mspAnswer) {
         StoragePoolHostVO poolHost = storagePoolHostDao.findByPoolHost(pool.getId(), hostId);
         if (poolHost == null) {

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAChecker.java

@@ -44,13 +44,7 @@ public Boolean hasHeartBeat() {
                 storagePools.stream().map(pool -> pool.getPoolUUID()).collect(Collectors.joining(", ")));
         logger.debug("Checking heart beat with KVMHAChecker for {}", hostAndPools);
 
-        boolean heartBeatCheckResult = false;
-        for (HAStoragePool pool : storagePools) {
-            heartBeatCheckResult = pool.getPool().hasHeartBeat(pool, host);
-            if (reportIfHeartBeatFailedForOneStoragePool && !heartBeatCheckResult) {
-                break;
-            }
-        }
+        boolean heartBeatCheckResult = doesAnyPoolHasHeartBeat();
 
         if (!heartBeatCheckResult) {
             logger.warn("All checks with KVMHAChecker for {} considered it as dead. It may cause a shutdown of the host.", hostAndPools);
@@ -59,6 +53,22 @@ public Boolean hasHeartBeat() {
         return heartBeatCheckResult;
     }
 
+    /*
+     * With reportIfHeartBeatFailedForOneStoragePool the host is dead as soon as one pool has no heart beat,
+     * otherwise it is dead only when none of the pools has one.
+     */
+    private boolean doesAnyPoolHasHeartBeat() {
+        boolean anyPoolHasHeartBeat = false;
+        for (HAStoragePool pool : storagePools) {
+            boolean poolHasHeartBeat = pool.getPool().hasHeartBeat(pool, host);
+            if (reportIfHeartBeatFailedForOneStoragePool && !poolHasHeartBeat) {
+                return false;
+            }
+            anyPoolHasHeartBeat = anyPoolHasHeartBeat || poolHasHeartBeat;
+        }
+        return anyPoolHasHeartBeat;
+    }
+
     @Override
     public Boolean call() throws Exception {
         // logger.addAppender(new org.apache.log4j.ConsoleAppender(new

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java

@@ -18,7 +18,7 @@
 
 import com.cloud.agent.properties.AgentProperties;
 import com.cloud.agent.properties.AgentPropertiesFileHandler;
-import com.cloud.ha.HighAvailabilityManager;
+import com.cloud.storage.Storage.StoragePoolType;
 import com.cloud.utils.script.Script;
 import org.libvirt.Connect;
 import org.libvirt.LibvirtException;
@@ -34,6 +34,8 @@
 
 public class KVMHAMonitor extends KVMHABase implements Runnable {
 
+    private static final List<StoragePoolType> MOUNT_BASED_POOL_TYPES = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint);
+
     private final Map<String, HAStoragePool> haStoragePools = new ConcurrentHashMap<>();
     private final boolean rebootHostAndAlertManagementOnHeartbeatTimeout;
 
@@ -60,7 +62,9 @@ public void removeStoragePool(String uuid) {
         synchronized (haStoragePools) {
             HAStoragePool pool = haStoragePools.get(uuid);
             if (pool != null) {
-                Script.runSimpleBashScript("umount " + pool.getMountDestPath());
+                if (isMountBased(pool)) {
+                    Script.runSimpleBashScript("umount " + pool.getMountDestPath());
+                }
                 haStoragePools.remove(uuid);
             }
         }
@@ -78,12 +82,16 @@ public HAStoragePool getStoragePool(String uuid) {
         }
     }
 
+    private boolean isMountBased(HAStoragePool pool) {
+        return MOUNT_BASED_POOL_TYPES.contains(pool.getPool().getType());
+    }
+
     protected void runHeartBeat() {
         synchronized (haStoragePools) {
             Set<String> removedPools = new HashSet<>();
             for (String uuid : haStoragePools.keySet()) {
                 HAStoragePool primaryStoragePool = haStoragePools.get(uuid);
-                if (HighAvailabilityManager.LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT.contains(primaryStoragePool.getPool().getType())) {
+                if (isMountBased(primaryStoragePool)) {
                     checkForNotExistingLibvirtStoragePools(removedPools, uuid);
                     if (removedPools.contains(uuid)) {
                         continue;