Skip to content

feature/CSTACKEX-46: Create, Delete iSCSI type Cloudstack volumes, Enter, Cancel Maintenance mode #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sandeeplocharla wants to merge 27 commits into
base: main
Choose a base branch
from
Open

feature/CSTACKEX-46: Create, Delete iSCSI type Cloudstack volumes, Enter, Cancel Maintenance mode #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9c85c6b
CSTACKEX-46: Disable, Re-Enable, Delete Storage pool and Enter, Exit …
Nov 10, 2025
41e8324
CSTACKEX-50: Fixed some issues seen while testing
Nov 18, 2025
7f47d93
CSTACKEX-46 Create Async, Attach Cluster/Zone and Grant/Revoke Access
Nov 5, 2025
d6e1d70
CSTACKEX-46 Added Logging
Nov 5, 2025
eff6d97
CSTACKEX-46 Resolve Copilot review comments
Nov 6, 2025
faf8dbf
CSTACKEX-46 Resolve review comments
Nov 6, 2025
8852bb4
CSTACKEX-46 Resolve review bot comments
Nov 6, 2025
dc5f01a
CSTACKEX-46 Change method name
Nov 7, 2025
14ec034
CSTACKEX-46 Resolve check styling issues
Nov 7, 2025
423aee2
CSTACKEX-46 Testing: Resolve fetch Storage details issue
Nov 7, 2025
905ae4a
CSTACKEX-46 Testing: Protocol Type fetch issue
Nov 7, 2025
3ee5ead
CSTACKEX-46 Testing: Added debug log
Nov 7, 2025
dc0e84e
CSTACKEX-46 Testing: Added debug log
Nov 7, 2025
b2056ff
CSTACKEX-46 Testing: Corrected the URL for Create Lun and Igroup
Nov 7, 2025
67bf958
CSTACKEX-46 Resolve bot review comments
Nov 7, 2025
8732f7f
CSTACKEX-46 Resolve review comments
Nov 10, 2025
f1b7dbd
CSTACKEX-46 Resolve review comments again
Nov 10, 2025
e9ba8b3
CSTACKEX-46 Change log level
Nov 10, 2025
f067b50
CSTACKEX-46 added valiodation for lun name
Nov 10, 2025
dac753f
CSTACKEX-46 resolve check style issues
Nov 10, 2025
716fcaf
CSTACKEX-46 Resolve import issues
Nov 10, 2025
e6ece89
CSTACKEX-46 Resolve copilot comments
Nov 10, 2025
d111436
CSTACKEX-46: Create, Delete iSCSI type Cloudstack volumes, Enter, Can…
Jan 21, 2026
a5fa9d2
CSTACKEX-46: Fixed a check style issue which got introduced during re…
Jan 22, 2026
3eea5c4
CSTACKEX-46: Fixed a check style issue which got introduced during re…
Jan 22, 2026
63addcd
CSTACKEX-46: Fixed a couple of issues observed while testing NFS3 sto…
Jan 27, 2026
dfc1ee9
CSTACKEX-46: Refactored driver class
Jan 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 49 additions & 11 deletions ...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java
View file
Open in desktop
Copy link

@rajiv-jain-netapp rajiv-jain-netapp Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciated the method-level comments that reference the algorithms used in each method. However, since this is open-source code, I recommend moving these details to the design specification instead of keeping them in the code. You can add a separate section in the spec to capture this information.
In the code, we can retain comments only for complex conditions or specific use cases for future reference.

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
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;
Expand Down Expand Up @@ -96,10 +98,15 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<S
String result = iScsiAdmCmd.execute();

if (result != null) {
logger.debug("Failed to add iSCSI target " + volumeUuid);
System.out.println("Failed to add iSCSI target " + volumeUuid);
// Node record may already exist from a previous run; accept and proceed
if (isNonFatalNodeCreate(result)) {
logger.debug("iSCSI node already exists for {}@{}:{}, proceeding", getIqn(volumeUuid), pool.getSourceHost(), pool.getSourcePort());
} else {
logger.debug("Failed to add iSCSI target " + volumeUuid);
System.out.println("Failed to add iSCSI target " + volumeUuid);

return false;
return false;
}
} else {
logger.debug("Successfully added iSCSI target " + volumeUuid);
System.out.println("Successfully added to iSCSI target " + volumeUuid);
Expand All @@ -123,21 +130,28 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<S
}
}

// 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);
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)
iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger);
iScsiAdmCmd.add("-m", "node");
iScsiAdmCmd.add("-T", getIqn(volumeUuid));
iScsiAdmCmd.add("-p", pool.getSourceHost() + ":" + pool.getSourcePort());
iScsiAdmCmd.add("-T", iqn);
iScsiAdmCmd.add("-p", host + ":" + port);
iScsiAdmCmd.add("--login");

result = iScsiAdmCmd.execute();

if (result != null) {
logger.debug("Failed to log in to iSCSI target " + volumeUuid);
System.out.println("Failed to log in to iSCSI target " + volumeUuid);
if (isNonFatalLogin(result)) {
logger.debug("iSCSI login returned benign message for {}@{}:{}: {}", iqn, host, port, result);
} else {
logger.debug("Failed to log in to iSCSI target " + volumeUuid + ": " + result);
System.out.println("Failed to log in to iSCSI target " + volumeUuid);

return false;
return false;
}
} else {
logger.debug("Successfully logged in to iSCSI target " + volumeUuid);
System.out.println("Successfully logged in to iSCSI target " + volumeUuid);
Expand All @@ -158,8 +172,23 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<S
return true;
}

// Removed sessionExists() call to avoid noisy sudo/iscsiadm session queries that may fail on some setups

private boolean isNonFatalLogin(String result) {
if (result == null) return true;
String msg = result.toLowerCase();
// Accept messages where the session already exists
return msg.contains("already present") || msg.contains("already logged in") || msg.contains("session exists");
}

private boolean isNonFatalNodeCreate(String result) {
if (result == null) return true;
String msg = result.toLowerCase();
return msg.contains("already exists") || msg.contains("database exists") || msg.contains("exists");
}

private void waitForDiskToBecomeAvailable(String volumeUuid, KVMStoragePool pool) {
int numberOfTries = 10;
int numberOfTries = 30;
int timeBetweenTries = 1000;

while (getPhysicalDisk(volumeUuid, pool).getSize() == 0 && numberOfTries > 0) {
Expand Down Expand Up @@ -238,6 +267,15 @@ 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 ignore) {
// If FS check fails for any reason, fall back to blockdev call
Copy link

@rajiv-jain-netapp rajiv-jain-netapp Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets add a logger here

}

Script iScsiAdmCmd = new Script(true, "blockdev", 0, logger);

iScsiAdmCmd.add("--getsize64", deviceByPath);
Expand Down
Loading
Loading