improve lock host retrieval logic and quicker retrival using db host as first check point and then fanning out

Author: Pearl Dsilva

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

@@ -27,28 +27,26 @@ public class ClvmLockTransferAnswer extends Answer {
 
     private String currentLockHostname;
     private boolean isActive;
-    private boolean isExclusive;
+    private boolean isOpen;
     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 currentLockHostname, boolean isActive, boolean isOpen,
                                   String lvAttributes) {
         super(cmd, result, details);
         this.currentLockHostname = currentLockHostname;
         this.isActive = isActive;
-        this.isExclusive = isExclusive;
+        this.isOpen = isOpen;
         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
+     * Get the hostname from lv_host. Retained for diagnostics only —
+     * do NOT use this to determine lock holder identity.
      */
     public String getCurrentLockHostname() {
         return currentLockHostname;
@@ -59,9 +57,8 @@ public void setCurrentLockHostname(String currentLockHostname) {
     }
 
     /**
-     * Whether the volume is currently active on any host.
-     *
-     * @return true if active, false otherwise
+     * Whether the LV is locally active on the queried host (lv_attr[4]=='a').
+     * This is the authoritative signal for lock holder discovery via fan-out.
      */
     public boolean isActive() {
         return isActive;
@@ -72,17 +69,15 @@ public void setActive(boolean active) {
     }
 
     /**
-     * Whether the lock is exclusive (as opposed to shared).
-     * Only meaningful if isActive() is true.
-     *
-     * @return true if exclusive lock, false if shared
+     * Whether a process has the device file open on the queried host (lv_attr[5]=='o').
+     * true means a VM is actively doing I/O on this host right now — do NOT deactivate.
      */
-    public boolean isExclusive() {
-        return isExclusive;
+    public boolean isOpen() {
+        return isOpen;
     }
 
-    public void setExclusive(boolean exclusive) {
-        isExclusive = exclusive;
+    public void setOpen(boolean open) {
+        isOpen = open;
     }
 
     public String getLvAttributes() {

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

@@ -96,13 +96,15 @@ public Answer execute(ClvmLockTransferCommand cmd, LibvirtComputingResource serv
     }
 
     /**
-     * Query which host currently holds the CLVM lock for a volume.
-     * Executes: lvs -o lv_attr,lv_host --noheadings <lvPath>
+     * Query whether this host currently has the CLVM LV activated locally.
+     * Executes: lvs -o lv_attr,lv_host,lv_active --noheadings <lvPath>
      *
-     * 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
+     * lv_attr[4]=='a' (isActive) is LOCAL and is the authoritative signal — true only on
+     * the host where the LV is currently activated. The management server fans out this
+     * query to all cluster hosts; the one returning isActive=true is the lock holder.
+     * lv_attr[5]=='o' (isOpen) means a VM has the device open on this host (doing I/O).
+     * lv_host is retained for diagnostic logging only — do NOT use it to identify the
+     * lock holder.
      */
     private Answer handleQueryLockState(ClvmLockTransferCommand cmd, String lvPath, String volumeUuid) {
         try {
@@ -121,14 +123,11 @@ private Answer handleQueryLockState(ClvmLockTransferCommand cmd, String lvPath,
                     String.format("lvs command failed: %s", result));
             }
 
-            // We need to find the line that contains the actual lv_attr (starts with '-' or other attr chars)
             String[] lines = parser.getLines().split("\n");
             String dataLine = null;
 
             for (String line : lines) {
                 String trimmed = line.trim();
-                // Skip empty lines and warning messages
-                // lv_attr always starts with '-', 'w', 'r', etc. and is at least 10 characters
                 if (!trimmed.isEmpty() &&
                     trimmed.length() >= 10 &&
                     (trimmed.charAt(0) == '-' || trimmed.charAt(0) == 'w' ||
@@ -157,23 +156,27 @@ private Answer handleQueryLockState(ClvmLockTransferCommand cmd, String lvPath,
             }
 
             String lvAttr = parts[0];
+            // lv_host: for diagnostics only, unreliable for lock-holder identification
             String hostname = parts.length > 1 ? parts[1] : null;
 
+            // lv_attr[4]=='a' → LV is active on THIS host (local activation state)
             boolean isActive = lvAttr.length() > 4 && lvAttr.charAt(4) == 'a';
-            boolean isExclusive = lvAttr.length() > 5 && lvAttr.charAt(5) == 'e';
+            // lv_attr[5]=='o' → a process has the device file open on this host (VM doing I/O)
+            boolean isOpen   = lvAttr.length() > 5 && lvAttr.charAt(5) == 'o';
 
-            logger.info("Queried lock state for volume {}: attr={}, hostname={}, active={}, exclusive={}",
-                    volumeUuid, lvAttr, hostname, isActive, isExclusive);
+            logger.info("Queried lock state for volume {}: attr={}, hostname={}, active={}, open={}",
+                    volumeUuid, lvAttr, hostname, isActive, isOpen);
 
             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);
+                    String.format("Lock state: active=%s, open=%s, host=%s",
+                            isActive, isOpen, hostname != null ? hostname : "none"),
+                    hostname, isActive, isOpen, 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());
         }
     }
+
 }