GutoVeronezi commented on a change in pull request #5862:
URL: https://github.com/apache/cloudstack/pull/5862#discussion_r792744665



##########
File path: 
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
##########
@@ -243,6 +257,10 @@ public DataStore initialize(Map<String, Object> dsInfos) {
             if (port == -1) {
                 port = 0;
             }
+            if (multi) {
+                storageHost = storageHost + (hostPath.substring(0, 
hostPath.lastIndexOf("/")).replaceAll("/", ","));
+                hostPath = hostPath.substring(hostPath.lastIndexOf("/")+1);

Review comment:
       ```suggestion
                   hostPath = hostPath.substring(hostPath.lastIndexOf("/") + 1);
   ```

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFenceCommandWrapper.java
##########
@@ -47,20 +48,25 @@ public Answer execute(final FenceCommand command, final 
LibvirtComputingResource
         final ExecutorService executors = Executors.newSingleThreadExecutor();
         final KVMHAMonitor monitor = libvirtComputingResource.getMonitor();
 
-        final List<NfsStoragePool> pools = monitor.getStoragePools();
+        final List<NfsStoragePool> nfspools = monitor.getStoragePools();
+        final List<RbdStoragePool> rbdpools = monitor.getRbdStoragePools();
 
         /**
          * We can only safely fence off hosts when we use NFS
          * On NFS primary storage pools hosts continuesly write
          * a heartbeat. Disable Fencing Off for hosts without NFS
          */
-        if (pools.size() == 0) {
+        if (nfspools.size() == 0) {
             String logline = "No NFS storage pools found. No way to safely 
fence " + command.getVmName() + " on host " + command.getHostGuid();
             s_logger.warn(logline);
             return new FenceAnswer(command, false, logline);
+        }else if (rbdpools.size() == 0) {

Review comment:
       ```suggestion
           } else if (rbdpools.size() == 0) {
   ```

##########
File path: 
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
##########
@@ -162,10 +168,18 @@ public DataStore initialize(Map<String, Object> dsInfos) {
                     throw new InvalidParameterValueException("host or path is 
null, should be sharedmountpoint://localhost/path");
                 }
             } else if (uri.getScheme().equalsIgnoreCase("rbd")) {
+                String uriHost = uri.getHost();
                 String uriPath = uri.getPath();
                 if (uriPath == null) {
                     throw new InvalidParameterValueException("host or path is 
null, should be rbd://hostname/pool");
                 }
+                if (multi) {
+                    String multiHost = uriHost + (uriPath.substring(0, 
uriPath.lastIndexOf("/")).replaceAll("/", ","));
+                    String[] hostArr = multiHost.split(",");
+                    if (hostArr.length > 5) {
+                        throw new InvalidParameterValueException("RADOS 
monitor can support up to 5.");

Review comment:
       ```suggestion
                           throw new InvalidParameterValueException("RADOS 
monitor can support up to 5 hosts.");
   ```

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHABase.java
##########
@@ -181,6 +206,26 @@ protected String runScriptRetry(String cmdString, 
OutputInterpreter interpreter)
         return result;
     }
 
+    public static String getRbdMonIpAddress(String sourceHost) {
+        try {
+            String[] hostArr = sourceHost.split(",");
+            String sourceHostIP = "";
+            for (String host : hostArr) {
+                String ipRegex = 
"(\\d{1,2}|1\\d\\d|2[0-4]\\d|25[0-5])\\.(\\d{1,2}|1\\d\\d|2[0-4]\\d|25[0-5])\\.(\\d{1,2}|1\\d\\d|2[0-4]\\d|25[0-5])\\.(\\d{1,2}|1\\d\\d|2[0-4]\\d|25[0-5])";
+                if (host.matches(ipRegex)) {
+                    sourceHostIP += host + ",";
+                } else {
+                    InetAddress addr = InetAddress.getByName(host);
+                    sourceHostIP += addr.getHostAddress() + ",";
+                }
+            }
+            return sourceHostIP;
+        } catch (UnknownHostException e) {
+            s_logger.debug("Failed to get connection: " + e.getMessage());

Review comment:
       We could pass the exception as parameter.

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java
##########
@@ -35,121 +35,221 @@
 public class KVMHAMonitor extends KVMHABase implements Runnable {
 
     private static final Logger s_logger = 
Logger.getLogger(KVMHAMonitor.class);
-    private final Map<String, NfsStoragePool> storagePool = new 
ConcurrentHashMap<>();
+    private final Map<String, NfsStoragePool> nfsstoragePool = new 
ConcurrentHashMap<>();
+    private final Map<String, RbdStoragePool> rbdstoragePool = new 
ConcurrentHashMap<>();
     private final boolean rebootHostAndAlertManagementOnHeartbeatTimeout;
 
     private final String hostPrivateIp;
 
-    public KVMHAMonitor(NfsStoragePool pool, String host, String scriptPath) {
+    public KVMHAMonitor(NfsStoragePool pool, RbdStoragePool rbdpool, String 
host, String scriptPath, String scriptPathRbd) {
         if (pool != null) {
-            storagePool.put(pool._poolUUID, pool);
+            nfsstoragePool.put(pool._poolUUID, pool);
+        }else if (rbdpool != null) {
+            rbdstoragePool.put(rbdpool._poolUUID, rbdpool);
         }
+        configureHeartBeatPath(scriptPath, scriptPathRbd);
         hostPrivateIp = host;
-        configureHeartBeatPath(scriptPath);
-
         _heartBeatUpdateTimeout = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HEARTBEAT_UPDATE_TIMEOUT);
         rebootHostAndAlertManagementOnHeartbeatTimeout = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.REBOOT_HOST_AND_ALERT_MANAGEMENT_ON_HEARTBEAT_TIMEOUT);
     }
 
-    private static synchronized void configureHeartBeatPath(String scriptPath) 
{
+    private static synchronized void configureHeartBeatPath(String scriptPath, 
String scriptPathRbd) {
         KVMHABase.s_heartBeatPath = scriptPath;
+        KVMHABase.s_heartBeatPathRbd = scriptPathRbd;
     }
 
     public void addStoragePool(NfsStoragePool pool) {
-        synchronized (storagePool) {
-            storagePool.put(pool._poolUUID, pool);
+        synchronized (nfsstoragePool) {
+            nfsstoragePool.put(pool._poolUUID, pool);
+        }
+    }
+
+    public void addStoragePool(RbdStoragePool pool) {
+        synchronized (rbdstoragePool) {
+            rbdstoragePool.put(pool._poolUUID, pool);
         }
     }
 
     public void removeStoragePool(String uuid) {
-        synchronized (storagePool) {
-            NfsStoragePool pool = storagePool.get(uuid);
+        synchronized (nfsstoragePool) {
+            NfsStoragePool pool = nfsstoragePool.get(uuid);
             if (pool != null) {
                 Script.runSimpleBashScript("umount " + pool._mountDestPath);
-                storagePool.remove(uuid);
+                nfsstoragePool.remove(uuid);
+            }
+        }
+    }
+
+    public void removeRbdStoragePool(String uuid) {
+        synchronized (rbdstoragePool) {
+            RbdStoragePool pool = rbdstoragePool.get(uuid);
+            if (pool != null) {
+                Script.runSimpleBashScript("umount " + pool._mountDestPath);
+                rbdstoragePool.remove(uuid);
             }
         }
     }
 
     public List<NfsStoragePool> getStoragePools() {
-        synchronized (storagePool) {
-            return new ArrayList<>(storagePool.values());
+        synchronized (nfsstoragePool) {
+            return new ArrayList<>(nfsstoragePool.values());
+        }
+    }
+
+    public List<RbdStoragePool> getRbdStoragePools() {
+        synchronized (rbdstoragePool) {
+            return new ArrayList<>(rbdstoragePool.values());
         }
     }
 
     public NfsStoragePool getStoragePool(String uuid) {
-        synchronized (storagePool) {
-            return storagePool.get(uuid);
+        synchronized (nfsstoragePool) {
+            return nfsstoragePool.get(uuid);
+        }
+    }
+
+    public RbdStoragePool getRbdStoragePool(String uuid) {
+        synchronized (rbdstoragePool) {
+            return rbdstoragePool.get(uuid);
         }
     }
 
     protected void runHeartBeat() {
-        synchronized (storagePool) {
-            Set<String> removedPools = new HashSet<>();
-            for (String uuid : storagePool.keySet()) {
-                NfsStoragePool primaryStoragePool = storagePool.get(uuid);
-                StoragePool storage;
-                try {
-                    Connect conn = LibvirtConnection.getConnection();
-                    storage = conn.storagePoolLookupByUUIDString(uuid);
-                    if (storage == null || storage.getInfo().state != 
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
-                        if (storage == null) {
-                            s_logger.debug(String.format("Libvirt storage pool 
[%s] not found, removing from HA list.", uuid));
-                        } else {
-                            s_logger.debug(String.format("Libvirt storage pool 
[%s] found, but not running, removing from HA list.", uuid));
+        if(nfsstoragePool != null && !nfsstoragePool.isEmpty()) {
+            synchronized (nfsstoragePool) {
+                Set<String> removedPools = new HashSet<>();
+                for (String uuid : nfsstoragePool.keySet()) {
+                    NfsStoragePool primaryStoragePool = 
nfsstoragePool.get(uuid);
+                    StoragePool storage;
+                    try {
+                        Connect conn = LibvirtConnection.getConnection();
+                        storage = conn.storagePoolLookupByUUIDString(uuid);
+                        if (storage == null || storage.getInfo().state != 
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
+                            if (storage == null) {
+                                s_logger.debug(String.format("Libvirt storage 
pool [%s] not found, removing from HA list.", uuid));
+                            } else {
+                                s_logger.debug(String.format("Libvirt storage 
pool [%s] found, but not running, removing from HA list.", uuid));
+                            }
+
+                            removedPools.add(uuid);
+                            continue;
+                        }
+
+                        s_logger.debug(String.format("Found NFS storage pool 
[%s] in libvirt, continuing.", uuid));
+
+                    } catch (LibvirtException e) {
+                        s_logger.debug(String.format("Failed to lookup libvirt 
storage pool [%s].", uuid), e);
+
+                        if (e.toString().contains("pool not found")) {
+                            s_logger.debug(String.format("Removing pool [%s] 
from HA monitor since it was deleted.", uuid));
+                            removedPools.add(uuid);
+                            continue;
                         }
 
-                        removedPools.add(uuid);
-                        continue;
                     }
 
-                    s_logger.debug(String.format("Found NFS storage pool [%s] 
in libvirt, continuing.", uuid));
+                    String result = null;
+                    for (int i = 1; i <= _heartBeatUpdateMaxTries; i++) {
+                        Script cmd = 
createHeartBeatCommand(primaryStoragePool, hostPrivateIp, true);
+                        result = cmd.execute();
+
+                        s_logger.debug(String.format("The command (%s), to the 
pool [%s], has the result [%s].", cmd.toString(), uuid, result));

Review comment:
       ```suggestion
                           s_logger.debug(String.format("The command [%s], to 
the pool [%s], had the result [%s].", cmd.toString(), uuid, result));
   ```

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java
##########
@@ -35,121 +35,221 @@
 public class KVMHAMonitor extends KVMHABase implements Runnable {
 
     private static final Logger s_logger = 
Logger.getLogger(KVMHAMonitor.class);
-    private final Map<String, NfsStoragePool> storagePool = new 
ConcurrentHashMap<>();
+    private final Map<String, NfsStoragePool> nfsstoragePool = new 
ConcurrentHashMap<>();
+    private final Map<String, RbdStoragePool> rbdstoragePool = new 
ConcurrentHashMap<>();
     private final boolean rebootHostAndAlertManagementOnHeartbeatTimeout;
 
     private final String hostPrivateIp;
 
-    public KVMHAMonitor(NfsStoragePool pool, String host, String scriptPath) {
+    public KVMHAMonitor(NfsStoragePool pool, RbdStoragePool rbdpool, String 
host, String scriptPath, String scriptPathRbd) {
         if (pool != null) {
-            storagePool.put(pool._poolUUID, pool);
+            nfsstoragePool.put(pool._poolUUID, pool);
+        }else if (rbdpool != null) {
+            rbdstoragePool.put(rbdpool._poolUUID, rbdpool);
         }
+        configureHeartBeatPath(scriptPath, scriptPathRbd);
         hostPrivateIp = host;
-        configureHeartBeatPath(scriptPath);
-
         _heartBeatUpdateTimeout = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HEARTBEAT_UPDATE_TIMEOUT);
         rebootHostAndAlertManagementOnHeartbeatTimeout = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.REBOOT_HOST_AND_ALERT_MANAGEMENT_ON_HEARTBEAT_TIMEOUT);
     }
 
-    private static synchronized void configureHeartBeatPath(String scriptPath) 
{
+    private static synchronized void configureHeartBeatPath(String scriptPath, 
String scriptPathRbd) {
         KVMHABase.s_heartBeatPath = scriptPath;
+        KVMHABase.s_heartBeatPathRbd = scriptPathRbd;
     }
 
     public void addStoragePool(NfsStoragePool pool) {
-        synchronized (storagePool) {
-            storagePool.put(pool._poolUUID, pool);
+        synchronized (nfsstoragePool) {
+            nfsstoragePool.put(pool._poolUUID, pool);
+        }
+    }
+
+    public void addStoragePool(RbdStoragePool pool) {
+        synchronized (rbdstoragePool) {
+            rbdstoragePool.put(pool._poolUUID, pool);
         }
     }
 
     public void removeStoragePool(String uuid) {
-        synchronized (storagePool) {
-            NfsStoragePool pool = storagePool.get(uuid);
+        synchronized (nfsstoragePool) {
+            NfsStoragePool pool = nfsstoragePool.get(uuid);
             if (pool != null) {
                 Script.runSimpleBashScript("umount " + pool._mountDestPath);
-                storagePool.remove(uuid);
+                nfsstoragePool.remove(uuid);
+            }
+        }
+    }
+
+    public void removeRbdStoragePool(String uuid) {
+        synchronized (rbdstoragePool) {
+            RbdStoragePool pool = rbdstoragePool.get(uuid);
+            if (pool != null) {
+                Script.runSimpleBashScript("umount " + pool._mountDestPath);
+                rbdstoragePool.remove(uuid);
             }
         }
     }
 
     public List<NfsStoragePool> getStoragePools() {
-        synchronized (storagePool) {
-            return new ArrayList<>(storagePool.values());
+        synchronized (nfsstoragePool) {
+            return new ArrayList<>(nfsstoragePool.values());
+        }
+    }
+
+    public List<RbdStoragePool> getRbdStoragePools() {
+        synchronized (rbdstoragePool) {
+            return new ArrayList<>(rbdstoragePool.values());
         }
     }
 
     public NfsStoragePool getStoragePool(String uuid) {
-        synchronized (storagePool) {
-            return storagePool.get(uuid);
+        synchronized (nfsstoragePool) {
+            return nfsstoragePool.get(uuid);
+        }
+    }
+
+    public RbdStoragePool getRbdStoragePool(String uuid) {
+        synchronized (rbdstoragePool) {
+            return rbdstoragePool.get(uuid);
         }
     }
 
     protected void runHeartBeat() {
-        synchronized (storagePool) {
-            Set<String> removedPools = new HashSet<>();
-            for (String uuid : storagePool.keySet()) {
-                NfsStoragePool primaryStoragePool = storagePool.get(uuid);
-                StoragePool storage;
-                try {
-                    Connect conn = LibvirtConnection.getConnection();
-                    storage = conn.storagePoolLookupByUUIDString(uuid);
-                    if (storage == null || storage.getInfo().state != 
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
-                        if (storage == null) {
-                            s_logger.debug(String.format("Libvirt storage pool 
[%s] not found, removing from HA list.", uuid));
-                        } else {
-                            s_logger.debug(String.format("Libvirt storage pool 
[%s] found, but not running, removing from HA list.", uuid));
+        if(nfsstoragePool != null && !nfsstoragePool.isEmpty()) {

Review comment:
       ```suggestion
           if (nfsstoragePool != null && !nfsstoragePool.isEmpty()) {
   ```

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java
##########
@@ -35,121 +35,221 @@
 public class KVMHAMonitor extends KVMHABase implements Runnable {
 
     private static final Logger s_logger = 
Logger.getLogger(KVMHAMonitor.class);
-    private final Map<String, NfsStoragePool> storagePool = new 
ConcurrentHashMap<>();
+    private final Map<String, NfsStoragePool> nfsstoragePool = new 
ConcurrentHashMap<>();
+    private final Map<String, RbdStoragePool> rbdstoragePool = new 
ConcurrentHashMap<>();
     private final boolean rebootHostAndAlertManagementOnHeartbeatTimeout;
 
     private final String hostPrivateIp;
 
-    public KVMHAMonitor(NfsStoragePool pool, String host, String scriptPath) {
+    public KVMHAMonitor(NfsStoragePool pool, RbdStoragePool rbdpool, String 
host, String scriptPath, String scriptPathRbd) {
         if (pool != null) {
-            storagePool.put(pool._poolUUID, pool);
+            nfsstoragePool.put(pool._poolUUID, pool);
+        }else if (rbdpool != null) {
+            rbdstoragePool.put(rbdpool._poolUUID, rbdpool);
         }
+        configureHeartBeatPath(scriptPath, scriptPathRbd);
         hostPrivateIp = host;
-        configureHeartBeatPath(scriptPath);
-
         _heartBeatUpdateTimeout = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HEARTBEAT_UPDATE_TIMEOUT);
         rebootHostAndAlertManagementOnHeartbeatTimeout = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.REBOOT_HOST_AND_ALERT_MANAGEMENT_ON_HEARTBEAT_TIMEOUT);
     }
 
-    private static synchronized void configureHeartBeatPath(String scriptPath) 
{
+    private static synchronized void configureHeartBeatPath(String scriptPath, 
String scriptPathRbd) {
         KVMHABase.s_heartBeatPath = scriptPath;
+        KVMHABase.s_heartBeatPathRbd = scriptPathRbd;
     }
 
     public void addStoragePool(NfsStoragePool pool) {
-        synchronized (storagePool) {
-            storagePool.put(pool._poolUUID, pool);
+        synchronized (nfsstoragePool) {
+            nfsstoragePool.put(pool._poolUUID, pool);
+        }
+    }
+
+    public void addStoragePool(RbdStoragePool pool) {
+        synchronized (rbdstoragePool) {
+            rbdstoragePool.put(pool._poolUUID, pool);
         }
     }
 
     public void removeStoragePool(String uuid) {
-        synchronized (storagePool) {
-            NfsStoragePool pool = storagePool.get(uuid);
+        synchronized (nfsstoragePool) {
+            NfsStoragePool pool = nfsstoragePool.get(uuid);
             if (pool != null) {
                 Script.runSimpleBashScript("umount " + pool._mountDestPath);
-                storagePool.remove(uuid);
+                nfsstoragePool.remove(uuid);
+            }
+        }
+    }
+
+    public void removeRbdStoragePool(String uuid) {
+        synchronized (rbdstoragePool) {
+            RbdStoragePool pool = rbdstoragePool.get(uuid);
+            if (pool != null) {
+                Script.runSimpleBashScript("umount " + pool._mountDestPath);
+                rbdstoragePool.remove(uuid);
             }
         }
     }
 
     public List<NfsStoragePool> getStoragePools() {
-        synchronized (storagePool) {
-            return new ArrayList<>(storagePool.values());
+        synchronized (nfsstoragePool) {
+            return new ArrayList<>(nfsstoragePool.values());
+        }
+    }
+
+    public List<RbdStoragePool> getRbdStoragePools() {
+        synchronized (rbdstoragePool) {
+            return new ArrayList<>(rbdstoragePool.values());
         }
     }
 
     public NfsStoragePool getStoragePool(String uuid) {
-        synchronized (storagePool) {
-            return storagePool.get(uuid);
+        synchronized (nfsstoragePool) {
+            return nfsstoragePool.get(uuid);
+        }
+    }
+
+    public RbdStoragePool getRbdStoragePool(String uuid) {
+        synchronized (rbdstoragePool) {
+            return rbdstoragePool.get(uuid);
         }
     }
 
     protected void runHeartBeat() {
-        synchronized (storagePool) {
-            Set<String> removedPools = new HashSet<>();
-            for (String uuid : storagePool.keySet()) {
-                NfsStoragePool primaryStoragePool = storagePool.get(uuid);
-                StoragePool storage;
-                try {
-                    Connect conn = LibvirtConnection.getConnection();
-                    storage = conn.storagePoolLookupByUUIDString(uuid);
-                    if (storage == null || storage.getInfo().state != 
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
-                        if (storage == null) {
-                            s_logger.debug(String.format("Libvirt storage pool 
[%s] not found, removing from HA list.", uuid));
-                        } else {
-                            s_logger.debug(String.format("Libvirt storage pool 
[%s] found, but not running, removing from HA list.", uuid));
+        if(nfsstoragePool != null && !nfsstoragePool.isEmpty()) {
+            synchronized (nfsstoragePool) {
+                Set<String> removedPools = new HashSet<>();
+                for (String uuid : nfsstoragePool.keySet()) {
+                    NfsStoragePool primaryStoragePool = 
nfsstoragePool.get(uuid);
+                    StoragePool storage;
+                    try {
+                        Connect conn = LibvirtConnection.getConnection();
+                        storage = conn.storagePoolLookupByUUIDString(uuid);
+                        if (storage == null || storage.getInfo().state != 
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
+                            if (storage == null) {
+                                s_logger.debug(String.format("Libvirt storage 
pool [%s] not found, removing from HA list.", uuid));

Review comment:
       ```suggestion
                                   s_logger.debug(String.format("Libvirt 
storage pool [%s] was not found, removing it from HA list.", uuid));
   ```

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFenceCommandWrapper.java
##########
@@ -47,20 +48,25 @@ public Answer execute(final FenceCommand command, final 
LibvirtComputingResource
         final ExecutorService executors = Executors.newSingleThreadExecutor();
         final KVMHAMonitor monitor = libvirtComputingResource.getMonitor();
 
-        final List<NfsStoragePool> pools = monitor.getStoragePools();
+        final List<NfsStoragePool> nfspools = monitor.getStoragePools();
+        final List<RbdStoragePool> rbdpools = monitor.getRbdStoragePools();
 
         /**
          * We can only safely fence off hosts when we use NFS
          * On NFS primary storage pools hosts continuesly write
          * a heartbeat. Disable Fencing Off for hosts without NFS
          */
-        if (pools.size() == 0) {
+        if (nfspools.size() == 0) {
             String logline = "No NFS storage pools found. No way to safely 
fence " + command.getVmName() + " on host " + command.getHostGuid();
             s_logger.warn(logline);
             return new FenceAnswer(command, false, logline);
+        }else if (rbdpools.size() == 0) {
+            String logline = "No RBD storage pools found. No way to safely 
fence " + command.getVmName() + " on host " + command.getHostGuid();

Review comment:
       We could use `String.format` here.

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java
##########
@@ -35,121 +35,221 @@
 public class KVMHAMonitor extends KVMHABase implements Runnable {
 
     private static final Logger s_logger = 
Logger.getLogger(KVMHAMonitor.class);
-    private final Map<String, NfsStoragePool> storagePool = new 
ConcurrentHashMap<>();
+    private final Map<String, NfsStoragePool> nfsstoragePool = new 
ConcurrentHashMap<>();
+    private final Map<String, RbdStoragePool> rbdstoragePool = new 
ConcurrentHashMap<>();
     private final boolean rebootHostAndAlertManagementOnHeartbeatTimeout;
 
     private final String hostPrivateIp;
 
-    public KVMHAMonitor(NfsStoragePool pool, String host, String scriptPath) {
+    public KVMHAMonitor(NfsStoragePool pool, RbdStoragePool rbdpool, String 
host, String scriptPath, String scriptPathRbd) {
         if (pool != null) {
-            storagePool.put(pool._poolUUID, pool);
+            nfsstoragePool.put(pool._poolUUID, pool);
+        }else if (rbdpool != null) {
+            rbdstoragePool.put(rbdpool._poolUUID, rbdpool);
         }
+        configureHeartBeatPath(scriptPath, scriptPathRbd);
         hostPrivateIp = host;
-        configureHeartBeatPath(scriptPath);
-
         _heartBeatUpdateTimeout = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HEARTBEAT_UPDATE_TIMEOUT);
         rebootHostAndAlertManagementOnHeartbeatTimeout = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.REBOOT_HOST_AND_ALERT_MANAGEMENT_ON_HEARTBEAT_TIMEOUT);
     }
 
-    private static synchronized void configureHeartBeatPath(String scriptPath) 
{
+    private static synchronized void configureHeartBeatPath(String scriptPath, 
String scriptPathRbd) {
         KVMHABase.s_heartBeatPath = scriptPath;
+        KVMHABase.s_heartBeatPathRbd = scriptPathRbd;
     }
 
     public void addStoragePool(NfsStoragePool pool) {
-        synchronized (storagePool) {
-            storagePool.put(pool._poolUUID, pool);
+        synchronized (nfsstoragePool) {
+            nfsstoragePool.put(pool._poolUUID, pool);
+        }
+    }
+
+    public void addStoragePool(RbdStoragePool pool) {
+        synchronized (rbdstoragePool) {
+            rbdstoragePool.put(pool._poolUUID, pool);
         }
     }
 
     public void removeStoragePool(String uuid) {
-        synchronized (storagePool) {
-            NfsStoragePool pool = storagePool.get(uuid);
+        synchronized (nfsstoragePool) {
+            NfsStoragePool pool = nfsstoragePool.get(uuid);
             if (pool != null) {
                 Script.runSimpleBashScript("umount " + pool._mountDestPath);
-                storagePool.remove(uuid);
+                nfsstoragePool.remove(uuid);
+            }
+        }
+    }
+
+    public void removeRbdStoragePool(String uuid) {
+        synchronized (rbdstoragePool) {
+            RbdStoragePool pool = rbdstoragePool.get(uuid);
+            if (pool != null) {
+                Script.runSimpleBashScript("umount " + pool._mountDestPath);
+                rbdstoragePool.remove(uuid);
             }
         }
     }
 
     public List<NfsStoragePool> getStoragePools() {
-        synchronized (storagePool) {
-            return new ArrayList<>(storagePool.values());
+        synchronized (nfsstoragePool) {
+            return new ArrayList<>(nfsstoragePool.values());
+        }
+    }
+
+    public List<RbdStoragePool> getRbdStoragePools() {
+        synchronized (rbdstoragePool) {
+            return new ArrayList<>(rbdstoragePool.values());
         }
     }
 
     public NfsStoragePool getStoragePool(String uuid) {
-        synchronized (storagePool) {
-            return storagePool.get(uuid);
+        synchronized (nfsstoragePool) {
+            return nfsstoragePool.get(uuid);
+        }
+    }
+
+    public RbdStoragePool getRbdStoragePool(String uuid) {
+        synchronized (rbdstoragePool) {
+            return rbdstoragePool.get(uuid);
         }
     }
 
     protected void runHeartBeat() {
-        synchronized (storagePool) {
-            Set<String> removedPools = new HashSet<>();
-            for (String uuid : storagePool.keySet()) {
-                NfsStoragePool primaryStoragePool = storagePool.get(uuid);
-                StoragePool storage;
-                try {
-                    Connect conn = LibvirtConnection.getConnection();
-                    storage = conn.storagePoolLookupByUUIDString(uuid);
-                    if (storage == null || storage.getInfo().state != 
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
-                        if (storage == null) {
-                            s_logger.debug(String.format("Libvirt storage pool 
[%s] not found, removing from HA list.", uuid));
-                        } else {
-                            s_logger.debug(String.format("Libvirt storage pool 
[%s] found, but not running, removing from HA list.", uuid));
+        if(nfsstoragePool != null && !nfsstoragePool.isEmpty()) {
+            synchronized (nfsstoragePool) {
+                Set<String> removedPools = new HashSet<>();
+                for (String uuid : nfsstoragePool.keySet()) {
+                    NfsStoragePool primaryStoragePool = 
nfsstoragePool.get(uuid);
+                    StoragePool storage;
+                    try {
+                        Connect conn = LibvirtConnection.getConnection();
+                        storage = conn.storagePoolLookupByUUIDString(uuid);
+                        if (storage == null || storage.getInfo().state != 
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
+                            if (storage == null) {
+                                s_logger.debug(String.format("Libvirt storage 
pool [%s] not found, removing from HA list.", uuid));
+                            } else {
+                                s_logger.debug(String.format("Libvirt storage 
pool [%s] found, but not running, removing from HA list.", uuid));

Review comment:
       ```suggestion
                                   s_logger.debug(String.format("Libvirt 
storage pool [%s] was found, but it is not running, removing it from HA list.", 
uuid));
   ```

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVMActivityOnStoragePoolCommandWrapper.java
##########
@@ -43,9 +44,13 @@ public Answer execute(final 
CheckVMActivityOnStoragePoolCommand command, final L
         final ExecutorService executors = Executors.newSingleThreadExecutor();
         final KVMHAMonitor monitor = libvirtComputingResource.getMonitor();
         final StorageFilerTO pool = command.getPool();
-        if (Storage.StoragePoolType.NetworkFilesystem == pool.getType()){
+        if (Storage.StoragePoolType.NetworkFilesystem == pool.getType() || 
Storage.StoragePoolType.RBD == pool.getType()){
             final NfsStoragePool nfspool = 
monitor.getStoragePool(pool.getUuid());
-            final KVMHAVMActivityChecker ha = new 
KVMHAVMActivityChecker(nfspool, command.getHost().getPrivateNetwork().getIp(), 
command.getVolumeList(), libvirtComputingResource.getVmActivityCheckPath(), 
command.getSuspectTimeInSeconds());
+            final RbdStoragePool rbdpool = 
monitor.getRbdStoragePool(pool.getUuid());
+            String vmActivityCheckPath = "";
+            if (Storage.StoragePoolType.NetworkFilesystem == pool.getType())   
 vmActivityCheckPath = libvirtComputingResource.getVmActivityCheckPath();
+            else if (Storage.StoragePoolType.RBD == pool.getType())    
vmActivityCheckPath = libvirtComputingResource.getVmActivityCheckPathRbd();

Review comment:
       According to our [coding 
conventions](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Coding+conventions),
  blocks must follow the following layout:
   
   ```java
   if (condition) {
       statements;
   } else {
       statements;
   }
   ```
   

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java
##########
@@ -35,121 +35,221 @@
 public class KVMHAMonitor extends KVMHABase implements Runnable {
 
     private static final Logger s_logger = 
Logger.getLogger(KVMHAMonitor.class);
-    private final Map<String, NfsStoragePool> storagePool = new 
ConcurrentHashMap<>();
+    private final Map<String, NfsStoragePool> nfsstoragePool = new 
ConcurrentHashMap<>();
+    private final Map<String, RbdStoragePool> rbdstoragePool = new 
ConcurrentHashMap<>();
     private final boolean rebootHostAndAlertManagementOnHeartbeatTimeout;
 
     private final String hostPrivateIp;
 
-    public KVMHAMonitor(NfsStoragePool pool, String host, String scriptPath) {
+    public KVMHAMonitor(NfsStoragePool pool, RbdStoragePool rbdpool, String 
host, String scriptPath, String scriptPathRbd) {
         if (pool != null) {
-            storagePool.put(pool._poolUUID, pool);
+            nfsstoragePool.put(pool._poolUUID, pool);
+        }else if (rbdpool != null) {
+            rbdstoragePool.put(rbdpool._poolUUID, rbdpool);
         }
+        configureHeartBeatPath(scriptPath, scriptPathRbd);
         hostPrivateIp = host;
-        configureHeartBeatPath(scriptPath);
-
         _heartBeatUpdateTimeout = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HEARTBEAT_UPDATE_TIMEOUT);
         rebootHostAndAlertManagementOnHeartbeatTimeout = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.REBOOT_HOST_AND_ALERT_MANAGEMENT_ON_HEARTBEAT_TIMEOUT);
     }
 
-    private static synchronized void configureHeartBeatPath(String scriptPath) 
{
+    private static synchronized void configureHeartBeatPath(String scriptPath, 
String scriptPathRbd) {
         KVMHABase.s_heartBeatPath = scriptPath;
+        KVMHABase.s_heartBeatPathRbd = scriptPathRbd;
     }
 
     public void addStoragePool(NfsStoragePool pool) {
-        synchronized (storagePool) {
-            storagePool.put(pool._poolUUID, pool);
+        synchronized (nfsstoragePool) {
+            nfsstoragePool.put(pool._poolUUID, pool);
+        }
+    }
+
+    public void addStoragePool(RbdStoragePool pool) {
+        synchronized (rbdstoragePool) {
+            rbdstoragePool.put(pool._poolUUID, pool);
         }
     }
 
     public void removeStoragePool(String uuid) {
-        synchronized (storagePool) {
-            NfsStoragePool pool = storagePool.get(uuid);
+        synchronized (nfsstoragePool) {
+            NfsStoragePool pool = nfsstoragePool.get(uuid);
             if (pool != null) {
                 Script.runSimpleBashScript("umount " + pool._mountDestPath);
-                storagePool.remove(uuid);
+                nfsstoragePool.remove(uuid);
+            }
+        }
+    }
+
+    public void removeRbdStoragePool(String uuid) {
+        synchronized (rbdstoragePool) {
+            RbdStoragePool pool = rbdstoragePool.get(uuid);
+            if (pool != null) {
+                Script.runSimpleBashScript("umount " + pool._mountDestPath);
+                rbdstoragePool.remove(uuid);
             }
         }
     }
 
     public List<NfsStoragePool> getStoragePools() {
-        synchronized (storagePool) {
-            return new ArrayList<>(storagePool.values());
+        synchronized (nfsstoragePool) {
+            return new ArrayList<>(nfsstoragePool.values());
+        }
+    }
+
+    public List<RbdStoragePool> getRbdStoragePools() {
+        synchronized (rbdstoragePool) {
+            return new ArrayList<>(rbdstoragePool.values());
         }
     }
 
     public NfsStoragePool getStoragePool(String uuid) {
-        synchronized (storagePool) {
-            return storagePool.get(uuid);
+        synchronized (nfsstoragePool) {
+            return nfsstoragePool.get(uuid);
+        }
+    }
+
+    public RbdStoragePool getRbdStoragePool(String uuid) {
+        synchronized (rbdstoragePool) {
+            return rbdstoragePool.get(uuid);
         }
     }
 
     protected void runHeartBeat() {
-        synchronized (storagePool) {
-            Set<String> removedPools = new HashSet<>();
-            for (String uuid : storagePool.keySet()) {
-                NfsStoragePool primaryStoragePool = storagePool.get(uuid);
-                StoragePool storage;
-                try {
-                    Connect conn = LibvirtConnection.getConnection();
-                    storage = conn.storagePoolLookupByUUIDString(uuid);
-                    if (storage == null || storage.getInfo().state != 
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
-                        if (storage == null) {
-                            s_logger.debug(String.format("Libvirt storage pool 
[%s] not found, removing from HA list.", uuid));
-                        } else {
-                            s_logger.debug(String.format("Libvirt storage pool 
[%s] found, but not running, removing from HA list.", uuid));
+        if(nfsstoragePool != null && !nfsstoragePool.isEmpty()) {
+            synchronized (nfsstoragePool) {
+                Set<String> removedPools = new HashSet<>();
+                for (String uuid : nfsstoragePool.keySet()) {
+                    NfsStoragePool primaryStoragePool = 
nfsstoragePool.get(uuid);
+                    StoragePool storage;
+                    try {
+                        Connect conn = LibvirtConnection.getConnection();
+                        storage = conn.storagePoolLookupByUUIDString(uuid);
+                        if (storage == null || storage.getInfo().state != 
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
+                            if (storage == null) {
+                                s_logger.debug(String.format("Libvirt storage 
pool [%s] not found, removing from HA list.", uuid));
+                            } else {
+                                s_logger.debug(String.format("Libvirt storage 
pool [%s] found, but not running, removing from HA list.", uuid));
+                            }
+
+                            removedPools.add(uuid);
+                            continue;
+                        }
+
+                        s_logger.debug(String.format("Found NFS storage pool 
[%s] in libvirt, continuing.", uuid));
+
+                    } catch (LibvirtException e) {
+                        s_logger.debug(String.format("Failed to lookup libvirt 
storage pool [%s].", uuid), e);
+
+                        if (e.toString().contains("pool not found")) {
+                            s_logger.debug(String.format("Removing pool [%s] 
from HA monitor since it was deleted.", uuid));
+                            removedPools.add(uuid);
+                            continue;
                         }
 
-                        removedPools.add(uuid);
-                        continue;
                     }
 
-                    s_logger.debug(String.format("Found NFS storage pool [%s] 
in libvirt, continuing.", uuid));
+                    String result = null;
+                    for (int i = 1; i <= _heartBeatUpdateMaxTries; i++) {
+                        Script cmd = 
createHeartBeatCommand(primaryStoragePool, hostPrivateIp, true);
+                        result = cmd.execute();
+
+                        s_logger.debug(String.format("The command (%s), to the 
pool [%s], has the result [%s].", cmd.toString(), uuid, result));
 
-                } catch (LibvirtException e) {
-                    s_logger.debug(String.format("Failed to lookup libvirt 
storage pool [%s].", uuid), e);
+                        if (result != null) {
+                            s_logger.warn(String.format("Write heartbeat for 
pool [%s] failed: %s; try: %s of %s.", uuid, result, i, 
_heartBeatUpdateMaxTries));
+                            try {
+                                Thread.sleep(_heartBeatUpdateRetrySleep);
+                            } catch (InterruptedException e) {
+                                s_logger.debug("[IGNORED] Interrupted between 
heartbeat retries.", e);
+                            }
+                        } else {
+                            break;
+                        }
 
-                    if (e.toString().contains("pool not found")) {
-                        s_logger.debug(String.format("Removing pool [%s] from 
HA monitor since it was deleted.", uuid));
-                        removedPools.add(uuid);
-                        continue;
                     }
 
+                    if (result != null && 
rebootHostAndAlertManagementOnHeartbeatTimeout) {
+                        s_logger.warn(String.format("Write heartbeat for pool 
[%s] failed: %s; stopping cloudstack-agent.", uuid, result));
+                        Script cmd = 
createHeartBeatCommand(primaryStoragePool, null, false);
+                        result = cmd.execute();
+                    }
                 }
 
-                String result = null;
-                for (int i = 1; i <= _heartBeatUpdateMaxTries; i++) {
-                    Script cmd = createHeartBeatCommand(primaryStoragePool, 
hostPrivateIp, true);
-                    result = cmd.execute();
+                if (!removedPools.isEmpty()) {
+                    for (String uuid : removedPools) {
+                        removeStoragePool(uuid);
+                    }
+                }
+            }
+        }
 
-                    s_logger.debug(String.format("The command (%s), to the 
pool [%s], has the result [%s].", cmd.toString(), uuid, result));
+        if (rbdstoragePool != null && !rbdstoragePool.isEmpty()) {

Review comment:
       This code and the one above look quite similar, we could extract it to a 
method




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to