GabrielBrascher commented on a change in pull request #3819: Clean up inactive
iscsi sessions when VMs get moved due to crashes
URL: https://github.com/apache/cloudstack/pull/3819#discussion_r370260437
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiStorageCleanupMonitor.java
##########
@@ -0,0 +1,127 @@
+package com.cloud.hypervisor.kvm.storage;
+
+import com.cloud.hypervisor.kvm.resource.LibvirtConnection;
+import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser;
+import com.cloud.hypervisor.kvm.resource.LibvirtVMDef;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.LibvirtException;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class IscsiStorageCleanupMonitor implements Runnable{
+ private static final Logger s_logger =
Logger.getLogger(IscsiStorageCleanupMonitor.class);
+ private static final int CLEANUP_INTERVAL_SEC = 60; // check every X
seconds
+ private static final String ISCSI_PATH_PREFIX = "/dev/disk/by-path";
+ private static final String KEYWORD_ISCSI = "iscsi";
+ private static final String KEYWORD_IQN = "iqn";
+
+ private IscsiAdmStorageAdaptor iscsiStorageAdaptor;
+
+ private Map<String, Boolean> diskStatusMap;
+
+ public IscsiStorageCleanupMonitor() {
+ diskStatusMap = new HashMap<>();
+ s_logger.debug("Initialize cleanup thread");
+ iscsiStorageAdaptor = new IscsiAdmStorageAdaptor();
+ }
+
+
+ private class Monitor extends ManagedContextRunnable {
+
+ @Override
+ protected void runInContext() {
+ Connect conn = null;
+ try {
+ conn = LibvirtConnection.getConnection();
+
+ //populate all the iscsi disks currently attached to this host
+ diskStatusMap.clear();
+ File[] iscsiVolumes = new File(ISCSI_PATH_PREFIX).listFiles();
+
+ if (iscsiVolumes == null || iscsiVolumes.length == 0) {
+ s_logger.debug("No iscsi sessions found for cleanup");
+ return;
+ }
+
+ for( File v : iscsiVolumes) {
+ if (isIscsiDisk(v.getAbsolutePath())) {
+ s_logger.debug("found iscsi disk by cleanup thread,
marking inactive:" + v.getAbsolutePath());
+ diskStatusMap.put(v.getAbsolutePath(), false);
+ }
+ }
+
+ // check if they belong to any VM
+ int[] domains = conn.listDomains();
+ s_logger.debug(String.format("found %d domains",
domains.length));
+ for (int domId : domains) {
+ Domain dm = conn.domainLookupByID(domId);
+ final String domXml = dm.getXMLDesc(0);
+ final LibvirtDomainXMLParser parser = new
LibvirtDomainXMLParser();
+ parser.parseDomainXML(domXml);
+ List<LibvirtVMDef.DiskDef> disks = parser.getDisks();
+
+ //check the volume map. If an entry exists change the
status to True
+ for (final LibvirtVMDef.DiskDef disk : disks) {
+ if (diskStatusMap.containsKey(disk.getDiskPath())) {
+ diskStatusMap.put(disk.getDiskPath(), true);
+ s_logger.debug("active disk found by cleanup
thread" + disk.getDiskPath());
+ }
+ }
Review comment:
I would recommend creating a few methods (e.g.
`checkIfIscsSessionBelongAnyVm(Connect)`, etc) which would allow documenting
(Javadoc) and creating unit tests (JUnit).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
