diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java index 3ec7407d1098..311154801b98 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java +++ b/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; @@ -47,12 +42,6 @@ public class IscsiAdmStorageAdaptor implements StorageAdaptor { private static final Map MapStorageUuidToStoragePool = new HashMap<>(); - /** iscsiadm's ISCSI_ERR_NO_OBJS_FOUND: returned by "-m session" when no session is established. */ - private static final int ISCSI_ERR_NO_OBJS_FOUND = 21; - - /** iscsiadm's ISCSI_ERR_SESS_EXISTS: returned by "--login" when the session is already logged in (e.g. Ubuntu). */ - private static final int ISCSI_SESSION_EXISTS_CODE = 15; - @Override public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map details, boolean isPrimaryStorage) { IscsiAdmStoragePool storagePool = new IscsiAdmStoragePool(uuid, host, port, storagePoolType, this); @@ -96,22 +85,24 @@ public KVMPhysicalDisk createPhysicalDisk(String volumeUuid, KVMStoragePool pool @Override public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map details, boolean isVMMigrate) { - final String host = pool.getSourceHost(); - final int port = pool.getSourcePort(); - final String iqn = getIqn(volumeUuid); - // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 -o new Script 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("-o", "new"); 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); @@ -132,10 +123,24 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map 0) { + while (getPhysicalDisk(volumeUuid, pool).getSize() == 0 && numberOfTries > 0) { numberOfTries--; try { Thread.sleep(timeBetweenTries); - } catch (InterruptedException ex) { - logger.warn("Interrupted while waiting for iSCSI device {} to become available", volumeUuid, ex); - return false; + } catch (Exception ex) { + // don't do anything } } - - return deviceSize > 0; } private void waitForDiskToBecomeUnavailable(String host, int port, String iqn, String lun) { @@ -346,25 +238,6 @@ public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) { } private long getDeviceSize(String deviceByPath) { - try { - Path devicePath = Paths.get(deviceByPath); - if (!Files.exists(devicePath)) { - logger.debug("Device by-path does not exist yet: {}", deviceByPath); - return 0L; - } - if (Files.isRegularFile(devicePath)) { - logger.warn("Found a corrupt regular file at iSCSI by-path {} (expected block device symlink); it must be removed manually", deviceByPath); - return 0L; - } - if (!Files.isSymbolicLink(devicePath)) { - logger.warn("Path {} exists but is not an iSCSI block device symlink", 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); @@ -407,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//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); @@ -602,8 +387,8 @@ public List 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."); } @@ -637,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(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/OntapIscsiStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/OntapIscsiStorageAdaptor.java index 73e8cb4026c7..c3b5a8f619b6 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/OntapIscsiStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/OntapIscsiStorageAdaptor.java @@ -16,23 +16,680 @@ // 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; +import org.apache.cloudstack.utils.qemu.QemuImgException; +import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LogManager; +import org.libvirt.LibvirtException; + +import com.cloud.agent.api.to.DiskTO; +import com.cloud.storage.Storage; +import com.cloud.storage.Storage.ProvisioningType; import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.OutputInterpreter; +import com.cloud.utils.script.Script; /** - * Serves {@link StoragePoolType#OntapiSCSI} pools, which are ONTAP FlexVols exposed over iSCSI with one - * LUN per CloudStack volume. The host-side handling is identical to a generic iSCSI target - * - * The class exists so that ONTAP-specific host behaviour can diverge here without altering the storage - * path of the other vendors that register as {@link StoragePoolType#Iscsi} which all share the superclass. - * - * This must stay in the {@code com.cloud.hypervisor.kvm.storage} package: {@link KVMStoragePoolManager} - * discovers adaptors by a Reflections scan of that package alone, and an unregistered type silently - * falls back to {@link LibvirtStorageAdaptor} rather than failing at startup. + * Host-side adaptor for {@link StoragePoolType#OntapiSCSI} + * Must remain in {@code com.cloud.hypervisor.kvm.storage}: {@link KVMStoragePoolManager} + * discovers adaptors only in that package. An unregistered type falls back to + * {@link LibvirtStorageAdaptor}. */ -public class OntapIscsiStorageAdaptor extends IscsiAdmStorageAdaptor { +public class OntapIscsiStorageAdaptor implements StorageAdaptor { + protected Logger logger = LogManager.getLogger(getClass()); + + private static final Map MapStorageUuidToStoragePool = new HashMap<>(); + + /** iscsiadm's ISCSI_ERR_NO_OBJS_FOUND: returned by "-m session" when no session is established. */ + private static final int ISCSI_ERR_NO_OBJS_FOUND = 21; + + /** iscsiadm's ISCSI_ERR_SESS_EXISTS: returned by "--login" when the session is already logged in (e.g. Ubuntu). */ + private static final int ISCSI_SESSION_EXISTS_CODE = 15; + + @Override + public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map details, boolean isPrimaryStorage) { + IscsiAdmStoragePool storagePool = new IscsiAdmStoragePool(uuid, host, port, storagePoolType, this); + + MapStorageUuidToStoragePool.put(uuid, storagePool); + + return storagePool; + } @Override public StoragePoolType getStoragePoolType() { return StoragePoolType.OntapiSCSI; } + + @Override + public KVMStoragePool getStoragePool(String uuid) { + return MapStorageUuidToStoragePool.get(uuid); + } + + @Override + public KVMStoragePool getStoragePool(String uuid, boolean refreshInfo) { + return MapStorageUuidToStoragePool.get(uuid); + } + + @Override + public boolean deleteStoragePool(String uuid) { + return MapStorageUuidToStoragePool.remove(uuid) != null; + } + + @Override + public boolean deleteStoragePool(KVMStoragePool pool) { + return deleteStoragePool(pool.getUuid()); + } + + // called from LibvirtComputingResource.execute(CreateCommand) + // does not apply for OntapIscsiStorageAdaptor + @Override + public KVMPhysicalDisk createPhysicalDisk(String volumeUuid, KVMStoragePool pool, PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, byte[] passphrase) { + throw new UnsupportedOperationException("Creating a physical disk is not supported."); + } + + @Override + public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map details, boolean isVMMigrate) { + final String host = pool.getSourceHost(); + final int port = pool.getSourcePort(); + final String iqn = getIqn(volumeUuid); + + // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 -o new + Script iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger); + + iScsiAdmCmd.add("-m", "node"); + iScsiAdmCmd.add("-T", iqn); + iScsiAdmCmd.add("-p", host + ":" + port); + iScsiAdmCmd.add("-o", "new"); + + String result = iScsiAdmCmd.execute(); + + if (!handleNodeCreateResult(result, volumeUuid)) { + return false; + } + + String chapInitiatorUsername = details.get(DiskTO.CHAP_INITIATOR_USERNAME); + String chapInitiatorSecret = details.get(DiskTO.CHAP_INITIATOR_SECRET); + + if (StringUtils.isNoneBlank(chapInitiatorUsername, chapInitiatorSecret)) { + try { + // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 --op update -n node.session.auth.authmethod -v CHAP + executeChapCommand(volumeUuid, pool, "node.session.auth.authmethod", "CHAP", null); + + // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 --op update -n node.session.auth.username -v username + executeChapCommand(volumeUuid, pool, "node.session.auth.username", chapInitiatorUsername, "username"); + + // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 --op update -n node.session.auth.password -v password + executeChapCommand(volumeUuid, pool, "node.session.auth.password", chapInitiatorSecret, "password"); + } catch (Exception ex) { + return false; + } + } + + // Login is always attempted (idempotent). Rescan runs only if the session already existed + // before login (Oracle re-login exits 0; Ubuntu may return ISCSI_ERR_SESS_EXISTS). + if (!loginOrRescanExistingSession(iqn, host, port, volumeUuid)) { + return false; + } + + // There appears to be a race condition where logging in to the iSCSI volume via iscsiadm + // returns success before the device has been added to the OS. + // What happens is you get logged in and the device shows up, but the device may not + // show up before we invoke Libvirt to attach the device to a VM. + // waitForDiskToBecomeAvailable(String, KVMStoragePool) invokes blockdev + // via getPhysicalDisk(String, KVMStoragePool) and checks if the size came back greater + // than 0. + // After a certain number of tries and a certain waiting period in between tries, + // this method could still return (it should not block indefinitely) (the race condition + // isn't solved here, but made highly unlikely to be a problem). + // If the by-path is missing or is a regular file (not the iSCSI block symlink), size + // stays 0. Return false so connect does not succeed and a raw file is not created at + // that by-path in place of the real LUN device. + if (!waitForDiskToBecomeAvailable(volumeUuid, pool)) { + logger.warn("iSCSI device not ready for target {} at {}:{} after wait", volumeUuid, host, port); + return false; + } + + 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 existing session state, performs login, and rescans only if the session already existed. + * + * Login is always attempted (idempotent). A pre-login session check is required on Oracle, + * where re-login often exits 0; Ubuntu may instead return ISCSI_ERR_SESS_EXISTS (15). + * Session-preexisted must be treated as success first: on Ubuntu, re-login exits 15 with a + * non-null error message that would otherwise be treated as failure. + * + * @return true if login succeeded (and rescan ran when needed), false on login failure + */ + private boolean loginOrRescanExistingSession(String iqn, String host, int port, String volumeUuid) { + boolean sessionAlreadyActive = isIscsiSessionActive(iqn, host, port); + logger.debug("iSCSI session active check for target {} at {}:{}: {}", iqn, host, port, sessionAlreadyActive); + + Script iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger); + iScsiAdmCmd.add("-m", "node"); + iScsiAdmCmd.add("-T", iqn); + iScsiAdmCmd.add("-p", host + ":" + port); + iScsiAdmCmd.add("--login"); + + String result = iScsiAdmCmd.execute(); + boolean sessionPreExisted = (iScsiAdmCmd.getExitValue() == ISCSI_SESSION_EXISTS_CODE) || sessionAlreadyActive; + + if (sessionPreExisted) { + logger.debug("iSCSI session for target {} at {}:{} pre-existed, performing rescan", iqn, host, port); + rescanIscsiSessions(iqn, host, port); + return true; + } + if (result == null) { + logger.debug("Successfully logged in to iSCSI target {}", volumeUuid); + return true; + } + logger.debug("Failed to log in to iSCSI target {}: {}", volumeUuid, result); + return false; + } + + /** + * Checks whether a session to the given target and portal is already established. + * + * "iscsiadm -m session" exits with ISCSI_ERR_NO_OBJS_FOUND when no session exists, which is a + * normal outcome here. Any other non-zero exit is logged and treated as not confirmed active. + */ + private boolean isIscsiSessionActive(String iqn, String host, int port) { + Script sessionCmd = new Script(true, "iscsiadm", 0, logger); + sessionCmd.add("-m", "session"); + + OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser(); + sessionCmd.executeIgnoreExitValue(parser, ISCSI_ERR_NO_OBJS_FOUND); + int exitValue = sessionCmd.getExitValue(); + if (exitValue != 0 && exitValue != ISCSI_ERR_NO_OBJS_FOUND) { + logger.warn("Unable to determine iSCSI session state for target {} at {}:{}: 'iscsiadm -m session' exited with {}", + iqn, host, port, exitValue); + return false; + } + + String sessions = parser.getLines(); + if (StringUtils.isBlank(sessions)) { + return false; + } + // AllLinesParser uses BufferedReader.readLine() (strips \n, \r\n, and \r) and then + // appends "\n" after each session. split("\n") depends on that separator to walk + // one session per line when multiple sessions are listed. + for (String line : sessions.split("\n")) { + if (line.contains(iqn) && line.contains(host)) { + return true; + } + } + + 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 boolean waitForDiskToBecomeAvailable(String volumeUuid, KVMStoragePool pool) { + int numberOfTries = 10; + int timeBetweenTries = 1000; + long deviceSize = 0; + + while ((deviceSize = getPhysicalDisk(volumeUuid, pool).getSize()) == 0 && numberOfTries > 0) { + numberOfTries--; + + try { + Thread.sleep(timeBetweenTries); + } catch (InterruptedException ex) { + logger.warn("Interrupted while waiting for iSCSI device {} to become available", volumeUuid, ex); + return false; + } + } + + return deviceSize > 0; + } + + private void waitForDiskToBecomeUnavailable(String host, int port, String iqn, String lun) { + int numberOfTries = 10; + int timeBetweenTries = 1000; + + String deviceByPath = getByPath(host, port, "/" + iqn + "/" + lun); + + while (getDeviceSize(deviceByPath) > 0 && numberOfTries > 0) { + numberOfTries--; + + try { + Thread.sleep(timeBetweenTries); + } catch (Exception ex) { + // don't do anything + } + } + } + + private void executeChapCommand(String path, KVMStoragePool pool, String nParameter, String vParameter, String detail) throws Exception { + Script iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger); + + iScsiAdmCmd.add("-m", "node"); + iScsiAdmCmd.add("-T", getIqn(path)); + iScsiAdmCmd.add("-p", pool.getSourceHost() + ":" + pool.getSourcePort()); + iScsiAdmCmd.add("--op", "update"); + iScsiAdmCmd.add("-n", nParameter); + iScsiAdmCmd.add("-v", vParameter); + + String result = iScsiAdmCmd.execute(); + + boolean useDetail = detail != null && detail.trim().length() > 0; + + detail = useDetail ? detail.trim() + " " : detail; + + if (result != null) { + logger.debug("Failed to execute CHAP " + (useDetail ? detail : "") + "command for iSCSI target " + path + " : message = " + result); + System.out.println("Failed to execute CHAP " + (useDetail ? detail : "") + "command for iSCSI target " + path + " : message = " + result); + + throw new Exception("Failed to execute CHAP " + (useDetail ? detail : "") + "command for iSCSI target " + path + " : message = " + result); + } else { + logger.debug("CHAP " + (useDetail ? detail : "") + "command executed successfully for iSCSI target " + path); + System.out.println("CHAP " + (useDetail ? detail : "") + "command executed successfully for iSCSI target " + path); + } + } + + // example by-path: /dev/disk/by-path/ip-192.168.233.10:3260-iscsi-iqn.2012-03.com.solidfire:storagepool2-lun-0 + private String getByPath(String host, int port, String path) { + return "/dev/disk/by-path/ip-" + host + ":" + port + "-iscsi-" + getIqn(path) + "-lun-" + getLun(path); + } + + @Override + public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) { + String deviceByPath = getByPath(pool.getSourceHost(), pool.getSourcePort(), volumeUuid); + KVMPhysicalDisk physicalDisk = new KVMPhysicalDisk(deviceByPath, volumeUuid, pool); + + physicalDisk.setFormat(PhysicalDiskFormat.RAW); + + long deviceSize = getDeviceSize(deviceByPath); + + physicalDisk.setSize(deviceSize); + physicalDisk.setVirtualSize(deviceSize); + + return physicalDisk; + } + + private long getDeviceSize(String deviceByPath) { + try { + Path devicePath = Paths.get(deviceByPath); + if (!Files.exists(devicePath)) { + logger.debug("Device by-path does not exist yet: {}", deviceByPath); + return 0L; + } + if (Files.isRegularFile(devicePath)) { + logger.warn("Found a corrupt regular file at iSCSI by-path {} (expected block device symlink); it must be removed manually", deviceByPath); + return 0L; + } + if (!Files.isSymbolicLink(devicePath)) { + logger.warn("Path {} exists but is not an iSCSI block device symlink", 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); + + OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser(); + + String result = iScsiAdmCmd.execute(parser); + + if (result != null) { + logger.warn("Unable to retrieve the size of device (resource may have moved to a different host)" + deviceByPath); + + return 0; + } + else { + logger.info("Successfully retrieved the size of device " + deviceByPath); + } + + return Long.parseLong(parser.getLine()); + } + + private String getIqn(String path) { + return getComponent(path, 1); + } + + private String getLun(String path) { + return getComponent(path, 2); + } + + private String getComponent(String path, int index) { + String[] tmp = path.split("/"); + + if (tmp.length != 3) { + String msg = "Wrong format for iScsi path: " + path + ". It should be formatted as '/targetIQN/LUN'."; + + logger.warn(msg); + + throw new CloudRuntimeException(msg); + } + + 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//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. + + // 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); + + iScsiAdmCmd.add("-m", "node"); + iScsiAdmCmd.add("-T", iqn); + iScsiAdmCmd.add("-p", host + ":" + port); + iScsiAdmCmd.add("--logout"); + + String result = iScsiAdmCmd.execute(); + + if (result != null) { + logger.debug("Failed to log out of iSCSI target /" + iqn + "/" + lun + " : message = " + result); + System.out.println("Failed to log out of iSCSI target /" + iqn + "/" + lun + " : message = " + result); + + return false; + } else { + logger.debug("Successfully logged out of iSCSI target /" + iqn + "/" + lun); + System.out.println("Successfully logged out of iSCSI target /" + iqn + "/" + lun); + } + + // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 -o delete + iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger); + + iScsiAdmCmd.add("-m", "node"); + iScsiAdmCmd.add("-T", iqn); + iScsiAdmCmd.add("-p", host + ":" + port); + iScsiAdmCmd.add("-o", "delete"); + + result = iScsiAdmCmd.execute(); + + if (result != null) { + logger.debug("Failed to remove iSCSI target /" + iqn + "/" + lun + " : message = " + result); + System.out.println("Failed to remove iSCSI target /" + iqn + "/" + lun + " : message = " + result); + + return false; + } else { + logger.debug("Removed iSCSI target /" + iqn + "/" + lun); + System.out.println("Removed iSCSI target /" + iqn + "/" + lun); + } + + waitForDiskToBecomeUnavailable(host, port, iqn, lun); + + return true; + } + + @Override + public boolean disconnectPhysicalDisk(String volumeUuid, KVMStoragePool pool) { + return disconnectPhysicalDisk(pool.getSourceHost(), pool.getSourcePort(), getIqn(volumeUuid), getLun(volumeUuid)); + } + + @Override + public boolean disconnectPhysicalDisk(Map volumeToDisconnect) { + String host = volumeToDisconnect.get(DiskTO.STORAGE_HOST); + String port = volumeToDisconnect.get(DiskTO.STORAGE_PORT); + String path = volumeToDisconnect.get(DiskTO.IQN); + + if (host != null && port != null && path != null) { + return disconnectPhysicalDisk(host, Integer.parseInt(port), getIqn(path), getLun(path)); + } + + return false; + } + + @Override + public boolean disconnectPhysicalDiskByPath(String localPath) { + String search1 = "/dev/disk/by-path/ip-"; + String search2 = ":"; + String search3 = "-iscsi-"; + String search4 = "-lun-"; + + if (!localPath.contains(search3)) { + return false; + } + + int index = localPath.indexOf(search2); + + String host = localPath.substring(search1.length(), index); + + int index2 = localPath.indexOf(search3); + + String port = localPath.substring(index + search2.length(), index2); + + index = localPath.indexOf(search4); + + String iqn = localPath.substring(index2 + search3.length(), index); + + String lun = localPath.substring(index + search4.length()); + + return disconnectPhysicalDisk(host, Integer.parseInt(port), iqn, lun); + } + + @Override + public boolean deletePhysicalDisk(String volumeUuid, KVMStoragePool pool, Storage.ImageFormat format) { + throw new UnsupportedOperationException("Deleting a physical disk is not supported."); + } + + // does not apply for OntapIscsiStorageAdaptor + @Override + public List listPhysicalDisks(String storagePoolUuid, KVMStoragePool pool) { + throw new UnsupportedOperationException("Listing disks is not supported for this configuration."); + } + + @Override + public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, String name, PhysicalDiskFormat format, + 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."); + } + + @Override + public KVMPhysicalDisk createTemplateFromDisk(KVMPhysicalDisk disk, String name, PhysicalDiskFormat format, long size, KVMStoragePool destPool) { + throw new UnsupportedOperationException("Creating a template from a disk is not yet supported for this configuration."); + } + + @Override + public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMStoragePool destPool, int timeout) { + return copyPhysicalDisk(disk, name, destPool, timeout, null, null, null); + } + + @Override + public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk srcDisk, String destVolumeUuid, KVMStoragePool destPool, int timeout, byte[] srcPassphrase, byte[] destPassphrase, ProvisioningType provisioningType) { + + QemuImgFile srcFile; + + KVMStoragePool srcPool = srcDisk.getPool(); + + if (srcPool.getType() == StoragePoolType.RBD) { + srcFile = new QemuImgFile(KVMPhysicalDisk.RBDStringBuilder(srcPool, srcDisk.getPath()), srcDisk.getFormat()); + } else { + srcFile = new QemuImgFile(srcDisk.getPath(), srcDisk.getFormat()); + } + + KVMPhysicalDisk destDisk = destPool.getPhysicalDisk(destVolumeUuid); + + QemuImgFile destFile = new QemuImgFile(destDisk.getPath(), destDisk.getFormat()); + + 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(); + + logger.error(msg); + + throw new CloudRuntimeException(msg); + } + + return destPool.getPhysicalDisk(destVolumeUuid); + } + + @Override + public boolean refresh(KVMStoragePool pool) { + return true; + } + + @Override + public boolean createFolder(String uuid, String path) { + return createFolder(uuid, path, null); + } + + @Override + public boolean createFolder(String uuid, String path, String localPath) { + throw new UnsupportedOperationException("A folder cannot be created in this configuration."); + } + + @Override + public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, String name, PhysicalDiskFormat format, long size, KVMStoragePool destPool, int timeout, byte[] passphrase) { + return null; + } + + @Override + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, String destTemplatePath, KVMStoragePool destPool, Storage.ImageFormat format, int timeout) { + return null; + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/OntapIscsiStorageAdaptorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/OntapIscsiStorageAdaptorTest.java index c89094081d35..3624fdedcfa0 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/OntapIscsiStorageAdaptorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/OntapIscsiStorageAdaptorTest.java @@ -72,7 +72,24 @@ public void adaptorSatisfiesThePoolManagerDiscoveryContract() throws ReflectiveO StorageAdaptor adaptor = OntapIscsiStorageAdaptor.class.getDeclaredConstructor().newInstance(); assertEquals(StoragePoolType.OntapiSCSI, adaptor.getStoragePoolType()); - assertEquals("The superclass must keep serving the other iSCSI vendors", + assertFalse("Must implement StorageAdaptor directly, not extend IscsiAdmStorageAdaptor", + IscsiAdmStorageAdaptor.class.isAssignableFrom(OntapIscsiStorageAdaptor.class)); + assertEquals("IscsiAdmStorageAdaptor must keep serving the other iSCSI vendors", StoragePoolType.Iscsi, new IscsiAdmStorageAdaptor().getStoragePoolType()); } + + @Test + public void ontapAndGenericIscsiAdaptorsDoNotShareThePoolMap() { + String uuid = "shared-looking-uuid"; + OntapIscsiStorageAdaptor ontap = new OntapIscsiStorageAdaptor(); + IscsiAdmStorageAdaptor generic = new IscsiAdmStorageAdaptor(); + + KVMStoragePool ontapPool = ontap.createStoragePool(uuid, "10.0.0.1", 3260, null, null, + StoragePoolType.OntapiSCSI, null, true); + + assertSame(ontapPool, ontap.getStoragePool(uuid)); + assertEquals(null, generic.getStoragePool(uuid)); + assertTrue(ontap.deleteStoragePool(uuid)); + assertEquals(null, ontap.getStoragePool(uuid)); + } }