removing the changes in iscsiadmadapter which were introduced for ontap as new adapter for ontap exist now

Author: Srivastava, Piyush

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java

@@ -16,14 +16,9 @@
 // under the License.
 package com.cloud.hypervisor.kvm.storage;
 
-import java.io.File;
-import java.io.FileWriter;
-import java.nio.file.Path;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.nio.file.Files;
-import java.nio.file.Paths;
 
 import org.apache.cloudstack.utils.qemu.QemuImg;
 import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
@@ -100,8 +95,14 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<S
 
         String result = iScsiAdmCmd.execute();
 
-        if (!handleNodeCreateResult(result, volumeUuid)) {
+        if (result != null) {
+            logger.debug("Failed to add iSCSI target " + volumeUuid);
+            System.out.println("Failed to add iSCSI target " + volumeUuid);
+
             return false;
+        } else {
+            logger.debug("Successfully added iSCSI target " + volumeUuid);
+            System.out.println("Successfully added to iSCSI target " + volumeUuid);
         }
 
         String chapInitiatorUsername = details.get(DiskTO.CHAP_INITIATOR_USERNAME);
@@ -122,26 +123,24 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<S
             }
         }
 
-        final String host = pool.getSourceHost();
-        final int port = pool.getSourcePort();
-        final String iqn = getIqn(volumeUuid);
-
-        // Always try to login; treat benign outcomes as success (idempotent)
+        // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 --login
         iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger);
+
         iScsiAdmCmd.add("-m", "node");
-        iScsiAdmCmd.add("-T", iqn);
-        iScsiAdmCmd.add("-p", host + ":" + port);
+        iScsiAdmCmd.add("-T", getIqn(volumeUuid));
+        iScsiAdmCmd.add("-p", pool.getSourceHost() + ":" + pool.getSourcePort());
         iScsiAdmCmd.add("--login");
 
         result = iScsiAdmCmd.execute();
 
-        if (!handleLoginResult(result, volumeUuid)) {
-            return false;
-        }
-
-        // If the session already existed, a newly mapped LUN won't be visible until a rescan.
         if (result != null) {
-            rescanIscsiSessions(iqn, host, port);
+            logger.debug("Failed to log in to iSCSI target " + volumeUuid);
+            System.out.println("Failed to log in to iSCSI target " + volumeUuid);
+
+            return false;
+        } else {
+            logger.debug("Successfully logged in to iSCSI target " + volumeUuid);
+            System.out.println("Successfully logged in to iSCSI target " + volumeUuid);
         }
 
         // There appears to be a race condition where logging in to the iSCSI volume via iscsiadm
@@ -159,56 +158,6 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<S
         return true;
     }
 
-    /**
-     * Checks the result of an iscsiadm node-create command.
-     * Returns true if the node was created or already exists, false on failure.
-     */
-    boolean handleNodeCreateResult(String result, String volumeUuid) {
-        if (result == null) {
-            logger.debug("Successfully added iSCSI node for target {}", volumeUuid);
-            return true;
-        }
-        String msg = result.toLowerCase();
-        if (msg.contains("already exists") || msg.contains("database exists") || msg.contains("exists")) {
-            logger.debug("iSCSI node already exists for target {}, proceeding", volumeUuid);
-            return true;
-        }
-        logger.debug("Failed to add iSCSI node for target {}: {}", volumeUuid, result);
-        return false;
-    }
-
-    /**
-     * Checks the result of an iscsiadm login command.
-     * Returns true if the login succeeded or session already exists, false on failure.
-     */
-    boolean handleLoginResult(String result, String volumeUuid) {
-        if (result == null) {
-            logger.debug("Successfully logged in to iSCSI target {}", volumeUuid);
-            return true;
-        }
-        String msg = result.toLowerCase();
-        if (msg.contains("already present") || msg.contains("already logged in") || msg.contains("session exists")) {
-            logger.debug("iSCSI session already exists for target {}, proceeding", volumeUuid);
-            return true;
-        }
-        logger.debug("Failed to log in to iSCSI target {}: {}", volumeUuid, result);
-        return false;
-    }
-
-    private void rescanIscsiSessions(String iqn, String host, int port) {
-        Script rescanCmd = new Script(true, "iscsiadm", 0, logger);
-        rescanCmd.add("-m", "node");
-        rescanCmd.add("-T", iqn);
-        rescanCmd.add("-p", host + ":" + port);
-        rescanCmd.add("--rescan");
-        String rescanResult = rescanCmd.execute();
-        if (rescanResult != null) {
-            logger.warn("iSCSI session rescan returned: {}", rescanResult);
-        } else {
-            logger.debug("iSCSI session rescan completed successfully for {}@{}:{}", iqn, host, port);
-        }
-    }
-
     private void waitForDiskToBecomeAvailable(String volumeUuid, KVMStoragePool pool) {
         int numberOfTries = 10;
         int timeBetweenTries = 1000;
@@ -289,16 +238,6 @@ public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) {
     }
 
     private long getDeviceSize(String deviceByPath) {
-        try {
-            if (!Files.exists(Paths.get(deviceByPath))) {
-                logger.debug("Device by-path does not exist yet: " + deviceByPath);
-                return 0L;
-            }
-        } catch (Exception ex) {
-            // If FS check fails for any reason, fall back to blockdev call
-            logger.error("Error fetching device size for {}", deviceByPath, ex);
-        }
-
         Script iScsiAdmCmd = new Script(true, "blockdev", 0, logger);
 
         iScsiAdmCmd.add("--getsize64", deviceByPath);
@@ -341,96 +280,8 @@ private String getComponent(String path, int index) {
         return tmp[index].trim();
     }
 
-    /**
-     * Check if there are other LUNs on the same iSCSI target (IQN) that are still
-     * visible as block devices. This is needed because ONTAP uses a single IQN per
-     * SVM — logging out of the target would kill ALL LUNs, not just the one being
-     * disconnected.
-     *
-     * Checks /dev/disk/by-path/ for symlinks matching the same host:port + IQN but
-     * with a different LUN number.
-     */
-    private boolean hasOtherActiveLuns(String host, int port, String iqn, String lun) {
-        String prefix = "ip-" + host + ":" + port + "-iscsi-" + iqn + "-lun-";
-        File byPathDir = new File("/dev/disk/by-path");
-        if (!byPathDir.exists() || !byPathDir.isDirectory()) {
-            return false;
-        }
-        File[] entries = byPathDir.listFiles();
-        if (entries == null) {
-            return false;
-        }
-        for (File entry : entries) {
-            String name = entry.getName();
-            // Skip partition entries (e.g. lun-0-part1, lun-0-part2) — these are not
-            // independent LUNs, they are partition symlinks for the same LUN disk.
-            // Only count actual LUN entries (no "-part" suffix after the lun number).
-            if (name.startsWith(prefix) && !name.equals(prefix + lun) && !name.contains("-part")) {
-                logger.debug("Found other active LUN on same target: " + name);
-                return true;
-            }
-        }
-        return false;
-    }
-
-    /**
-     * Removes a single stale SCSI device from the kernel using the sysfs interface.
-     *
-     * When ONTAP unmaps a LUN from the host's igroup, the by-path symlink and the
-     * underlying SCSI device (/dev/sdX) remain present in the kernel until explicitly
-     * removed — the kernel does not auto-remove devices from live iSCSI sessions.
-     *
-     * This method resolves the by-path symlink to the real block device name (e.g. sdd),
-     * then writes "1" to /sys/block/<dev>/device/delete — the standard Linux kernel SCSI
-     * API for removing a single device without tearing down the entire iSCSI session.
-     * Once the kernel processes the delete, it also removes the by-path symlink.
-     *
-     * This is used instead of iscsiadm --logout when other LUNs on the same IQN are still
-     * active (ONTAP single-IQN-per-SVM model), since logout would tear down ALL LUNs.
-     */
-    private void removeStaleScsiDevice(String host, int port, String iqn, String lun) {
-        String byPath = getByPath(host, port, "/" + iqn + "/" + lun);
-        Path byPathLink = Paths.get(byPath);
-        if (!Files.exists(byPathLink)) {
-            logger.debug("by-path entry for LUN " + lun + " already gone, nothing to remove");
-            return;
-        }
-        try {
-            Path realDevice = byPathLink.toRealPath();
-            String devName = realDevice.getFileName().toString();
-            File deleteFile = new File("/sys/block/" + devName + "/device/delete");
-            if (!deleteFile.exists()) {
-                logger.warn("sysfs delete entry not found for device " + devName + " — cannot remove stale SCSI device");
-                return;
-            }
-            try (FileWriter fw = new FileWriter(deleteFile)) {
-                fw.write("1");
-            }
-            logger.info("Removed stale SCSI device " + devName + " for LUN /" + iqn + "/" + lun + " via sysfs");
-        } catch (Exception e) {
-            logger.warn("Failed to remove stale SCSI device for LUN /" + iqn + "/" + lun + ": " + e.getMessage());
-        }
-    }
-
     private boolean disconnectPhysicalDisk(String host, int port, String iqn, String lun) {
-        // Check if other LUNs on the same IQN target are still in use.
-        // ONTAP (and similar) uses a single IQN per SVM with multiple LUNs.
-        // Doing iscsiadm --logout tears down the ENTIRE target session,
-        // which would destroy access to ALL LUNs — not just the one being disconnected.
-        if (hasOtherActiveLuns(host, port, iqn, lun)) {
-            logger.info("Skipping iSCSI logout for /" + iqn + "/" + lun +
-                    " — other LUNs on the same target are still active. Removing stale SCSI device for this LUN only.");
-            removeStaleScsiDevice(host, port, iqn, lun);
-            // After removing this LUN's device, re-check: if no other LUNs remain active,
-            // If it is the last one then must logout to clean up the iSCSI session entirely.
-            if (hasOtherActiveLuns(host, port, iqn, lun)) {
-                logger.info("Other LUNs still active after removing /" + iqn + "/" + lun + " — session kept alive.");
-                return true;
-            }
-            logger.info("No more active LUNs on target after removing /" + iqn + "/" + lun + " — proceeding with iSCSI logout.");
-        }
-
-        // No other LUNs active on this target — safe to logout and delete the node record.
+        // use iscsiadm to log out of the iSCSI target and un-discover it
 
         // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 --logout
         Script iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger);
@@ -536,8 +387,8 @@ public List<KVMPhysicalDisk> listPhysicalDisks(String storagePoolUuid, KVMStorag
 
     @Override
     public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, String name, PhysicalDiskFormat format,
-            ProvisioningType provisioningType, long size,
-            KVMStoragePool destPool, int timeout, byte[] passphrase) {
+                                                  ProvisioningType provisioningType, long size,
+                                                  KVMStoragePool destPool, int timeout, byte[] passphrase) {
         throw new UnsupportedOperationException("Creating a disk from a template is not yet supported for this configuration.");
     }
 
@@ -571,19 +422,6 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk srcDisk, String destVolu
         try {
             QemuImg q = new QemuImg(timeout);
             q.convert(srcFile, destFile);
-            // Below fix is required when vendor depends on host based copy rather than storage CAN_CREATE_VOLUME_FROM_VOLUME capability
-            // When host based template copy is triggered , small size template sits in RAM(depending on host memory and RAM) and copy is marked successful and by the time flush to storage is triggered
-            // disconnectPhysicalDisk would disconnect the lun , hence template staying in RAM is not copied to storage lun. Below does flushing of data to storage and marking
-            // copy as successful once flush is complete.
-            Script flushCmd = new Script(true, "blockdev", 0, logger);
-            flushCmd.add("--flushbufs", destDisk.getPath());
-            String flushResult = flushCmd.execute();
-            if (flushResult != null) {
-                logger.warn("iSCSI copyPhysicalDisk: blockdev --flushbufs returned: {}", flushResult);
-            }
-            Script syncCmd = new Script(true, "sync", 0, logger);
-            syncCmd.execute();
-            logger.info("iSCSI copyPhysicalDisk: flush/sync completed ");
         } catch (QemuImgException | LibvirtException ex) {
             String msg = "Failed to copy data from " + srcDisk.getPath() + " to " +
                     destDisk.getPath() + ". The error was the following: " + ex.getMessage();