Skip to content

Commit a463f96

Browse files
piyush5netappSrivastava, Piyush
andauthored
feature/CSTACKEX-310: implement all method for ontap iscsi adapter (#105)
…d removed changes from common iscsiadmstorageadpater ### Description This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): ### How Has This Been Tested? Tested happy path testing in ubunut VM. All workflows working fine. Create zone->Pod-> Cluster->Host->Primary Storage->Secondary Storage. iSCSI Create Storage Pool - PASS Create disk and compute offering for the storage pool using only the tag. - PASS Create VM instance using the same disk and compute offering. - PASS Power Off the VM. - PASS Power on the VM on the same host again. - PASS Destroy the VM without expunge and no data volume selected. - PASS Attach the disk/volume to a already running VM. - PASS Detach the disk/volume from a running VM. - PASS Destroy the volume without expunge. - PASS Recover the destroyed volume again. - PASS Destroy the VM without expunge and all data volume selected.- PASS Destroy the VM with expunge and all data volume selected.- PASS Take instance snapshot without quiesce and without memory.- PASS <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> --------- Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
1 parent 02837ff commit a463f96

3 files changed

Lines changed: 719 additions & 273 deletions

File tree

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

Lines changed: 34 additions & 262 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,9 @@
1616
// under the License.
1717
package com.cloud.hypervisor.kvm.storage;
1818

19-
import java.io.File;
20-
import java.io.FileWriter;
21-
import java.nio.file.Path;
2219
import java.util.HashMap;
2320
import java.util.List;
2421
import java.util.Map;
25-
import java.nio.file.Files;
26-
import java.nio.file.Paths;
2722

2823
import org.apache.cloudstack.utils.qemu.QemuImg;
2924
import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
@@ -47,12 +42,6 @@ public class IscsiAdmStorageAdaptor implements StorageAdaptor {
4742

4843
private static final Map<String, KVMStoragePool> MapStorageUuidToStoragePool = new HashMap<>();
4944

50-
/** iscsiadm's ISCSI_ERR_NO_OBJS_FOUND: returned by "-m session" when no session is established. */
51-
private static final int ISCSI_ERR_NO_OBJS_FOUND = 21;
52-
53-
/** iscsiadm's ISCSI_ERR_SESS_EXISTS: returned by "--login" when the session is already logged in (e.g. Ubuntu). */
54-
private static final int ISCSI_SESSION_EXISTS_CODE = 15;
55-
5645
@Override
5746
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map<String, String> details, boolean isPrimaryStorage) {
5847
IscsiAdmStoragePool storagePool = new IscsiAdmStoragePool(uuid, host, port, storagePoolType, this);
@@ -96,22 +85,24 @@ public KVMPhysicalDisk createPhysicalDisk(String volumeUuid, KVMStoragePool pool
9685

9786
@Override
9887
public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<String, String> details, boolean isVMMigrate) {
99-
final String host = pool.getSourceHost();
100-
final int port = pool.getSourcePort();
101-
final String iqn = getIqn(volumeUuid);
102-
10388
// ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 -o new
10489
Script iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger);
10590

10691
iScsiAdmCmd.add("-m", "node");
107-
iScsiAdmCmd.add("-T", iqn);
108-
iScsiAdmCmd.add("-p", host + ":" + port);
92+
iScsiAdmCmd.add("-T", getIqn(volumeUuid));
93+
iScsiAdmCmd.add("-p", pool.getSourceHost() + ":" + pool.getSourcePort());
10994
iScsiAdmCmd.add("-o", "new");
11095

11196
String result = iScsiAdmCmd.execute();
11297

113-
if (!handleNodeCreateResult(result, volumeUuid)) {
98+
if (result != null) {
99+
logger.debug("Failed to add iSCSI target " + volumeUuid);
100+
System.out.println("Failed to add iSCSI target " + volumeUuid);
101+
114102
return false;
103+
} else {
104+
logger.debug("Successfully added iSCSI target " + volumeUuid);
105+
System.out.println("Successfully added to iSCSI target " + volumeUuid);
115106
}
116107

117108
String chapInitiatorUsername = details.get(DiskTO.CHAP_INITIATOR_USERNAME);
@@ -132,10 +123,24 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<S
132123
}
133124
}
134125

135-
// Login is always attempted (idempotent). Rescan runs only if the session already existed
136-
// before login (Oracle re-login exits 0; Ubuntu may return ISCSI_ERR_SESS_EXISTS).
137-
if (!loginOrRescanExistingSession(iqn, host, port, volumeUuid)) {
126+
// ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 --login
127+
iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger);
128+
129+
iScsiAdmCmd.add("-m", "node");
130+
iScsiAdmCmd.add("-T", getIqn(volumeUuid));
131+
iScsiAdmCmd.add("-p", pool.getSourceHost() + ":" + pool.getSourcePort());
132+
iScsiAdmCmd.add("--login");
133+
134+
result = iScsiAdmCmd.execute();
135+
136+
if (result != null) {
137+
logger.debug("Failed to log in to iSCSI target " + volumeUuid);
138+
System.out.println("Failed to log in to iSCSI target " + volumeUuid);
139+
138140
return false;
141+
} else {
142+
logger.debug("Successfully logged in to iSCSI target " + volumeUuid);
143+
System.out.println("Successfully logged in to iSCSI target " + volumeUuid);
139144
}
140145

141146
// There appears to be a race condition where logging in to the iSCSI volume via iscsiadm
@@ -148,137 +153,24 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<S
148153
// After a certain number of tries and a certain waiting period in between tries,
149154
// this method could still return (it should not block indefinitely) (the race condition
150155
// isn't solved here, but made highly unlikely to be a problem).
151-
// If the by-path is missing or is a regular file (not the iSCSI block symlink), size
152-
// stays 0. Return false so connect does not succeed and a raw file is not created at
153-
// that by-path in place of the real LUN device.
154-
if (!waitForDiskToBecomeAvailable(volumeUuid, pool)) {
155-
logger.warn("iSCSI device not ready for target {} at {}:{} after wait", volumeUuid, host, port);
156-
return false;
157-
}
156+
waitForDiskToBecomeAvailable(volumeUuid, pool);
158157

159158
return true;
160159
}
161160

162-
/**
163-
* Checks the result of an iscsiadm node-create command.
164-
* Returns true if the node was created or already exists, false on failure.
165-
*/
166-
boolean handleNodeCreateResult(String result, String volumeUuid) {
167-
if (result == null) {
168-
logger.debug("Successfully added iSCSI node for target {}", volumeUuid);
169-
return true;
170-
}
171-
String msg = result.toLowerCase();
172-
if (msg.contains("already exists") || msg.contains("database exists") || msg.contains("exists")) {
173-
logger.debug("iSCSI node already exists for target {}, proceeding", volumeUuid);
174-
return true;
175-
}
176-
logger.debug("Failed to add iSCSI node for target {}: {}", volumeUuid, result);
177-
return false;
178-
}
179-
180-
/**
181-
* Checks existing session state, performs login, and rescans only if the session already existed.
182-
*
183-
* Login is always attempted (idempotent). A pre-login session check is required on Oracle,
184-
* where re-login often exits 0; Ubuntu may instead return ISCSI_ERR_SESS_EXISTS (15).
185-
* Session-preexisted must be treated as success first: on Ubuntu, re-login exits 15 with a
186-
* non-null error message that would otherwise be treated as failure.
187-
*
188-
* @return true if login succeeded (and rescan ran when needed), false on login failure
189-
*/
190-
private boolean loginOrRescanExistingSession(String iqn, String host, int port, String volumeUuid) {
191-
boolean sessionAlreadyActive = isIscsiSessionActive(iqn, host, port);
192-
logger.debug("iSCSI session active check for target {} at {}:{}: {}", iqn, host, port, sessionAlreadyActive);
193-
194-
Script iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger);
195-
iScsiAdmCmd.add("-m", "node");
196-
iScsiAdmCmd.add("-T", iqn);
197-
iScsiAdmCmd.add("-p", host + ":" + port);
198-
iScsiAdmCmd.add("--login");
199-
200-
String result = iScsiAdmCmd.execute();
201-
boolean sessionPreExisted = (iScsiAdmCmd.getExitValue() == ISCSI_SESSION_EXISTS_CODE) || sessionAlreadyActive;
202-
203-
if (sessionPreExisted) {
204-
logger.debug("iSCSI session for target {} at {}:{} pre-existed, performing rescan", iqn, host, port);
205-
rescanIscsiSessions(iqn, host, port);
206-
return true;
207-
}
208-
if (result == null) {
209-
logger.debug("Successfully logged in to iSCSI target {}", volumeUuid);
210-
return true;
211-
}
212-
logger.debug("Failed to log in to iSCSI target {}: {}", volumeUuid, result);
213-
return false;
214-
}
215-
216-
/**
217-
* Checks whether a session to the given target and portal is already established.
218-
*
219-
* "iscsiadm -m session" exits with ISCSI_ERR_NO_OBJS_FOUND when no session exists, which is a
220-
* normal outcome here. Any other non-zero exit is logged and treated as not confirmed active.
221-
*/
222-
private boolean isIscsiSessionActive(String iqn, String host, int port) {
223-
Script sessionCmd = new Script(true, "iscsiadm", 0, logger);
224-
sessionCmd.add("-m", "session");
225-
226-
OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();
227-
sessionCmd.executeIgnoreExitValue(parser, ISCSI_ERR_NO_OBJS_FOUND);
228-
int exitValue = sessionCmd.getExitValue();
229-
if (exitValue != 0 && exitValue != ISCSI_ERR_NO_OBJS_FOUND) {
230-
logger.warn("Unable to determine iSCSI session state for target {} at {}:{}: 'iscsiadm -m session' exited with {}",
231-
iqn, host, port, exitValue);
232-
return false;
233-
}
234-
235-
String sessions = parser.getLines();
236-
if (StringUtils.isBlank(sessions)) {
237-
return false;
238-
}
239-
// AllLinesParser uses BufferedReader.readLine() (strips \n, \r\n, and \r) and then
240-
// appends "\n" after each session. split("\n") depends on that separator to walk
241-
// one session per line when multiple sessions are listed.
242-
for (String line : sessions.split("\n")) {
243-
if (line.contains(iqn) && line.contains(host)) {
244-
return true;
245-
}
246-
}
247-
248-
return false;
249-
}
250-
251-
private void rescanIscsiSessions(String iqn, String host, int port) {
252-
Script rescanCmd = new Script(true, "iscsiadm", 0, logger);
253-
rescanCmd.add("-m", "node");
254-
rescanCmd.add("-T", iqn);
255-
rescanCmd.add("-p", host + ":" + port);
256-
rescanCmd.add("--rescan");
257-
String rescanResult = rescanCmd.execute();
258-
if (rescanResult != null) {
259-
logger.warn("iSCSI session rescan returned: {}", rescanResult);
260-
} else {
261-
logger.debug("iSCSI session rescan completed successfully for {}@{}:{}", iqn, host, port);
262-
}
263-
}
264-
265-
private boolean waitForDiskToBecomeAvailable(String volumeUuid, KVMStoragePool pool) {
161+
private void waitForDiskToBecomeAvailable(String volumeUuid, KVMStoragePool pool) {
266162
int numberOfTries = 10;
267163
int timeBetweenTries = 1000;
268-
long deviceSize = 0;
269164

270-
while ((deviceSize = getPhysicalDisk(volumeUuid, pool).getSize()) == 0 && numberOfTries > 0) {
165+
while (getPhysicalDisk(volumeUuid, pool).getSize() == 0 && numberOfTries > 0) {
271166
numberOfTries--;
272167

273168
try {
274169
Thread.sleep(timeBetweenTries);
275-
} catch (InterruptedException ex) {
276-
logger.warn("Interrupted while waiting for iSCSI device {} to become available", volumeUuid, ex);
277-
return false;
170+
} catch (Exception ex) {
171+
// don't do anything
278172
}
279173
}
280-
281-
return deviceSize > 0;
282174
}
283175

284176
private void waitForDiskToBecomeUnavailable(String host, int port, String iqn, String lun) {
@@ -346,25 +238,6 @@ public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) {
346238
}
347239

348240
private long getDeviceSize(String deviceByPath) {
349-
try {
350-
Path devicePath = Paths.get(deviceByPath);
351-
if (!Files.exists(devicePath)) {
352-
logger.debug("Device by-path does not exist yet: {}", deviceByPath);
353-
return 0L;
354-
}
355-
if (Files.isRegularFile(devicePath)) {
356-
logger.warn("Found a corrupt regular file at iSCSI by-path {} (expected block device symlink); it must be removed manually", deviceByPath);
357-
return 0L;
358-
}
359-
if (!Files.isSymbolicLink(devicePath)) {
360-
logger.warn("Path {} exists but is not an iSCSI block device symlink", deviceByPath);
361-
return 0L;
362-
}
363-
} catch (Exception ex) {
364-
// If FS check fails for any reason, fall back to blockdev call
365-
logger.error("Error fetching device size for {}", deviceByPath, ex);
366-
}
367-
368241
Script iScsiAdmCmd = new Script(true, "blockdev", 0, logger);
369242

370243
iScsiAdmCmd.add("--getsize64", deviceByPath);
@@ -407,96 +280,8 @@ private String getComponent(String path, int index) {
407280
return tmp[index].trim();
408281
}
409282

410-
/**
411-
* Check if there are other LUNs on the same iSCSI target (IQN) that are still
412-
* visible as block devices. This is needed because ONTAP uses a single IQN per
413-
* SVM — logging out of the target would kill ALL LUNs, not just the one being
414-
* disconnected.
415-
*
416-
* Checks /dev/disk/by-path/ for symlinks matching the same host:port + IQN but
417-
* with a different LUN number.
418-
*/
419-
private boolean hasOtherActiveLuns(String host, int port, String iqn, String lun) {
420-
String prefix = "ip-" + host + ":" + port + "-iscsi-" + iqn + "-lun-";
421-
File byPathDir = new File("/dev/disk/by-path");
422-
if (!byPathDir.exists() || !byPathDir.isDirectory()) {
423-
return false;
424-
}
425-
File[] entries = byPathDir.listFiles();
426-
if (entries == null) {
427-
return false;
428-
}
429-
for (File entry : entries) {
430-
String name = entry.getName();
431-
// Skip partition entries (e.g. lun-0-part1, lun-0-part2) — these are not
432-
// independent LUNs, they are partition symlinks for the same LUN disk.
433-
// Only count actual LUN entries (no "-part" suffix after the lun number).
434-
if (name.startsWith(prefix) && !name.equals(prefix + lun) && !name.contains("-part")) {
435-
logger.debug("Found other active LUN on same target: " + name);
436-
return true;
437-
}
438-
}
439-
return false;
440-
}
441-
442-
/**
443-
* Removes a single stale SCSI device from the kernel using the sysfs interface.
444-
*
445-
* When ONTAP unmaps a LUN from the host's igroup, the by-path symlink and the
446-
* underlying SCSI device (/dev/sdX) remain present in the kernel until explicitly
447-
* removed — the kernel does not auto-remove devices from live iSCSI sessions.
448-
*
449-
* This method resolves the by-path symlink to the real block device name (e.g. sdd),
450-
* then writes "1" to /sys/block/<dev>/device/delete — the standard Linux kernel SCSI
451-
* API for removing a single device without tearing down the entire iSCSI session.
452-
* Once the kernel processes the delete, it also removes the by-path symlink.
453-
*
454-
* This is used instead of iscsiadm --logout when other LUNs on the same IQN are still
455-
* active (ONTAP single-IQN-per-SVM model), since logout would tear down ALL LUNs.
456-
*/
457-
private void removeStaleScsiDevice(String host, int port, String iqn, String lun) {
458-
String byPath = getByPath(host, port, "/" + iqn + "/" + lun);
459-
Path byPathLink = Paths.get(byPath);
460-
if (!Files.exists(byPathLink)) {
461-
logger.debug("by-path entry for LUN " + lun + " already gone, nothing to remove");
462-
return;
463-
}
464-
try {
465-
Path realDevice = byPathLink.toRealPath();
466-
String devName = realDevice.getFileName().toString();
467-
File deleteFile = new File("/sys/block/" + devName + "/device/delete");
468-
if (!deleteFile.exists()) {
469-
logger.warn("sysfs delete entry not found for device " + devName + " — cannot remove stale SCSI device");
470-
return;
471-
}
472-
try (FileWriter fw = new FileWriter(deleteFile)) {
473-
fw.write("1");
474-
}
475-
logger.info("Removed stale SCSI device " + devName + " for LUN /" + iqn + "/" + lun + " via sysfs");
476-
} catch (Exception e) {
477-
logger.warn("Failed to remove stale SCSI device for LUN /" + iqn + "/" + lun + ": " + e.getMessage());
478-
}
479-
}
480-
481283
private boolean disconnectPhysicalDisk(String host, int port, String iqn, String lun) {
482-
// Check if other LUNs on the same IQN target are still in use.
483-
// ONTAP (and similar) uses a single IQN per SVM with multiple LUNs.
484-
// Doing iscsiadm --logout tears down the ENTIRE target session,
485-
// which would destroy access to ALL LUNs — not just the one being disconnected.
486-
if (hasOtherActiveLuns(host, port, iqn, lun)) {
487-
logger.info("Skipping iSCSI logout for /" + iqn + "/" + lun +
488-
" — other LUNs on the same target are still active. Removing stale SCSI device for this LUN only.");
489-
removeStaleScsiDevice(host, port, iqn, lun);
490-
// After removing this LUN's device, re-check: if no other LUNs remain active,
491-
// If it is the last one then must logout to clean up the iSCSI session entirely.
492-
if (hasOtherActiveLuns(host, port, iqn, lun)) {
493-
logger.info("Other LUNs still active after removing /" + iqn + "/" + lun + " — session kept alive.");
494-
return true;
495-
}
496-
logger.info("No more active LUNs on target after removing /" + iqn + "/" + lun + " — proceeding with iSCSI logout.");
497-
}
498-
499-
// No other LUNs active on this target — safe to logout and delete the node record.
284+
// use iscsiadm to log out of the iSCSI target and un-discover it
500285

501286
// ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 --logout
502287
Script iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger);
@@ -602,8 +387,8 @@ public List<KVMPhysicalDisk> listPhysicalDisks(String storagePoolUuid, KVMStorag
602387

603388
@Override
604389
public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, String name, PhysicalDiskFormat format,
605-
ProvisioningType provisioningType, long size,
606-
KVMStoragePool destPool, int timeout, byte[] passphrase) {
390+
ProvisioningType provisioningType, long size,
391+
KVMStoragePool destPool, int timeout, byte[] passphrase) {
607392
throw new UnsupportedOperationException("Creating a disk from a template is not yet supported for this configuration.");
608393
}
609394

@@ -637,19 +422,6 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk srcDisk, String destVolu
637422
try {
638423
QemuImg q = new QemuImg(timeout);
639424
q.convert(srcFile, destFile);
640-
// Below fix is required when vendor depends on host based copy rather than storage CAN_CREATE_VOLUME_FROM_VOLUME capability
641-
// 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
642-
// 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
643-
// copy as successful once flush is complete.
644-
Script flushCmd = new Script(true, "blockdev", 0, logger);
645-
flushCmd.add("--flushbufs", destDisk.getPath());
646-
String flushResult = flushCmd.execute();
647-
if (flushResult != null) {
648-
logger.warn("iSCSI copyPhysicalDisk: blockdev --flushbufs returned: {}", flushResult);
649-
}
650-
Script syncCmd = new Script(true, "sync", 0, logger);
651-
syncCmd.execute();
652-
logger.info("iSCSI copyPhysicalDisk: flush/sync completed ");
653425
} catch (QemuImgException | LibvirtException ex) {
654426
String msg = "Failed to copy data from " + srcDisk.getPath() + " to " +
655427
destDisk.getPath() + ". The error was the following: " + ex.getMessage();

0 commit comments

Comments
 (0)