CloudStack Volume support with ONTAP storage

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

@@ -77,6 +77,8 @@ public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStra
 
     private static final List<Storage.StoragePoolType> supportedStoragePoolTypes = List.of(Storage.StoragePoolType.Filesystem, Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.SharedMountPoint);
 
+    private static final String ONTAP_PROVIDER_NAME = "NetApp ONTAP";
+
     @Inject
     protected SnapshotDataStoreDao snapshotDataStoreDao;
 
@@ -325,6 +327,11 @@ public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMe
         List<VolumeVO> volumes = volumeDao.findByInstance(vmId);
         for (VolumeVO volume : volumes) {
             StoragePoolVO storagePoolVO = storagePool.findById(volume.getPoolId());
+            if (storagePoolVO.isManaged() && ONTAP_PROVIDER_NAME.equals(storagePoolVO.getStorageProviderName())) {
+                logger.debug(String.format("%s as the VM has a volume on ONTAP managed storage pool [%s]. " +
+                        "ONTAP managed storage has its own dedicated VM snapshot strategy.", cantHandleLog, storagePoolVO.getName()));
+                return StrategyPriority.CANT_HANDLE;
+            }
             if (!supportedStoragePoolTypes.contains(storagePoolVO.getPoolType())) {
                 logger.debug(String.format("%s as the VM has a volume that is in a storage with unsupported type [%s].", cantHandleLog, storagePoolVO.getPoolType()));
                 return StrategyPriority.CANT_HANDLE;
@@ -503,8 +510,9 @@ protected VMSnapshot takeVmSnapshotInternal(VMSnapshot vmSnapshot, Map<VolumeInf
             return processCreateVmSnapshotAnswer(vmSnapshot, volumeInfoToSnapshotObjectMap, createDiskOnlyVMSnapshotAnswer, userVm, vmSnapshotVO, virtualSize, parentSnapshotVo);
         }
 
-        logger.error("Disk-only VM snapshot for VM [{}] failed{}.", userVm.getUuid(), answer != null ? " due to" + answer.getDetails() : "");
-        throw new CloudRuntimeException(String.format("Disk-only VM snapshot for VM [%s] failed.", userVm.getUuid()));
+        String details = answer != null ? answer.getDetails() : String.format("No answer received from host [%s]. The host may be unreachable.", hostId);
+        logger.error("Disk-only VM snapshot for VM [{}] failed due to: {}.", userVm.getUuid(), details);
+        throw new CloudRuntimeException(String.format("Disk-only VM snapshot for VM [%s] failed due to: %s.", userVm.getUuid(), details));
     }
 
     /**

engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java

@@ -1340,6 +1340,15 @@ private void createManagedVolumeCopyTemplateAsync(VolumeInfo volumeInfo, Primary
             primaryDataStore.setDetails(details);
 
             grantAccess(volumeInfo, destHost, primaryDataStore);
+            volumeInfo = volFactory.getVolume(volumeInfo.getId(), primaryDataStore);
+            // For Netapp ONTAP iscsiName or Lun path  is available only after grantAccess
+            String managedStoreTarget = volumeInfo.get_iScsiName() != null ? volumeInfo.get_iScsiName() : volumeInfo.getUuid();
+            details.put(PrimaryDataStore.MANAGED_STORE_TARGET, managedStoreTarget);
+            primaryDataStore.setDetails(details);
+            // Update destTemplateInfo with the iSCSI path from volumeInfo
+            if (destTemplateInfo instanceof TemplateObject) {
+                ((TemplateObject)destTemplateInfo).setInstallPath(volumeInfo.getPath());
+            }
 
             try {
                 motionSrv.copyAsync(srcTemplateInfo, destTemplateInfo, destHost, caller);

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java

@@ -106,6 +106,10 @@ protected Answer takeDiskOnlyVmSnapshotOfRunningVm(CreateDiskOnlyVmSnapshotComma
                 return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, errorMsg, null);
             }
             return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, e.getMessage(), null);
+        } catch (Exception e) {
+            String errorMsg = String.format("Creation of disk-only VM snapshot for VM [%s] failed due to %s.", vmName, e.getMessage());
+            logger.error(errorMsg, e);
+            return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, errorMsg, null);
         } finally {
             if (dm != null) {
                 try {
@@ -146,21 +150,13 @@ protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotComma
             }
         } catch (LibvirtException | QemuImgException e) {
             logger.error("Exception while creating disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e);
-            for (VolumeObjectTO volumeObjectTO : volumeObjectTos) {
-                Pair<Long, String> volSizeAndNewPath = mapVolumeToSnapshotSizeAndNewVolumePath.get(volumeObjectTO.getUuid());
-                PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) volumeObjectTO.getDataStore();
-                KVMStoragePool kvmStoragePool = storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid());
-
-                if (volSizeAndNewPath == null) {
-                    continue;
-                }
-                try {
-                    Files.deleteIfExists(Path.of(kvmStoragePool.getLocalPathFor(volSizeAndNewPath.second())));
-                } catch (IOException ex) {
-                    logger.warn("Tried to delete leftover snapshot at [{}] failed.", volSizeAndNewPath.second(), ex);
-                }
-            }
+            cleanupLeftoverDeltas(volumeObjectTos, mapVolumeToSnapshotSizeAndNewVolumePath, storagePoolMgr);
             return new Answer(cmd, e);
+        } catch (Exception e) {
+            logger.error("Unexpected exception while creating disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e);
+            cleanupLeftoverDeltas(volumeObjectTos, mapVolumeToSnapshotSizeAndNewVolumePath, storagePoolMgr);
+            return new CreateDiskOnlyVmSnapshotAnswer(cmd, false,
+                    String.format("Creation of disk-only VM snapshot for VM [%s] failed due to %s.", vmName, e.getMessage()), null);
         }
 
         return new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, mapVolumeToSnapshotSizeAndNewVolumePath);
@@ -192,6 +188,23 @@ protected Pair<String, Map<String, Pair<Long, String>>> createSnapshotXmlAndNewV
         return new Pair<>(snapshotXml, volumeObjectToNewPathMap);
     }
 
+    protected void cleanupLeftoverDeltas(List<VolumeObjectTO> volumeObjectTos, Map<String, Pair<Long, String>> mapVolumeToSnapshotSizeAndNewVolumePath, KVMStoragePoolManager storagePoolMgr) {
+        for (VolumeObjectTO volumeObjectTO : volumeObjectTos) {
+            Pair<Long, String> volSizeAndNewPath = mapVolumeToSnapshotSizeAndNewVolumePath.get(volumeObjectTO.getUuid());
+            PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) volumeObjectTO.getDataStore();
+            KVMStoragePool kvmStoragePool = storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid());
+
+            if (volSizeAndNewPath == null) {
+                continue;
+            }
+            try {
+                Files.deleteIfExists(Path.of(kvmStoragePool.getLocalPathFor(volSizeAndNewPath.second())));
+            } catch (IOException ex) {
+                logger.warn("Tried to delete leftover snapshot at [{}] failed.", volSizeAndNewPath.second(), ex);
+            }
+        }
+    }
+
     protected long getFileSize(String path) {
         return new File(path).length();
     }

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

@@ -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;
@@ -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);
@@ -123,21 +130,39 @@ 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);
+                // Session already exists — a newly mapped LUN won't be visible until
+                // the kernel's next periodic SCSI scan (~30-60s).
+                Script rescanCmd = new Script(true, "iscsiadm", 0, logger);
+                rescanCmd.add("-m", "session");
+                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);
+                }
+            } 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);
@@ -158,8 +183,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) {
@@ -238,6 +278,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
+        }
+
         Script iScsiAdmCmd = new Script(true, "blockdev", 0, logger);
 
         iScsiAdmCmd.add("--getsize64", deviceByPath);
@@ -280,8 +329,47 @@ 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-";
+        java.io.File byPathDir = new java.io.File("/dev/disk/by-path");
+        if (!byPathDir.exists() || !byPathDir.isDirectory()) {
+            return false;
+        }
+        java.io.File[] entries = byPathDir.listFiles();
+        if (entries == null) {
+            return false;
+        }
+        for (java.io.File entry : entries) {
+            String name = entry.getName();
+            if (name.startsWith(prefix) && !name.equals(prefix + lun)) {
+                logger.debug("Found other active LUN on same target: " + name);
+                return true;
+            }
+        }
+        return false;
+    }
+
     private boolean disconnectPhysicalDisk(String host, int port, String iqn, String lun) {
-        // use iscsiadm to log out of the iSCSI target and un-discover it
+        // 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");
+            return true;
+        }
+
+        // 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);
@@ -422,6 +510,19 @@ 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();

plugins/storage/volume/ontap/pom.xml

@@ -39,6 +39,7 @@
         <junit-jupiter.version>5.8.1</junit-jupiter.version>
         <mockito.version>3.12.4</mockito.version>
         <mockito-junit-jupiter.version>5.2.0</mockito-junit-jupiter.version>
+        <byte-buddy-agent.version>1.11.13</byte-buddy-agent.version>
     </properties>
     <dependencyManagement>
         <dependencies>
@@ -121,12 +122,24 @@
             <version>${mockito.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>net.bytebuddy</groupId>
+            <artifactId>byte-buddy-agent</artifactId>
+            <version>${byte-buddy-agent.version}</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.assertj</groupId>
             <artifactId>assertj-core</artifactId>
             <version>${assertj.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.cloudstack</groupId>
+            <artifactId>cloud-engine-storage-snapshot</artifactId>
+            <version>4.23.0.0-SNAPSHOT</version>
+            <scope>compile</scope>
+        </dependency>
     </dependencies>
     <repositories>
         <repository>
@@ -151,6 +164,7 @@
                 <version>${maven-surefire-plugin.version}</version>
                 <configuration>
                     <skipTests>false</skipTests>
+                    <argLine>-javaagent:${settings.localRepository}/net/bytebuddy/byte-buddy-agent/${byte-buddy-agent.version}/byte-buddy-agent-${byte-buddy-agent.version}.jar</argLine>
                     <includes>
                         <include>**/*Test.java</include>
                     </includes>