Enhancing storagepool operations and snapshot related fixes in NetApp ONTAP storage plugin

[sandeeplocharla]

Description

This PR has contributions from @rajiv-jain-netapp, @suryag1201, @piyush5netapp, @sathvikaragi, and @sandeeplocharla.

Summary

Bug Fixes

• Fixed NFS storage pool usage after EnterMaintenance and CancelMaintenance operations.
• Restored the PrimaryDataStoreLifeCycle.updateStoragePool() call required by the Resize StoragePool workflow.
• Gracefully handled 404 exceptions during deletion of:
  • ExportPolicy
  • ONTAP volumes
  • ONTAP snapshots
• Added validation to ensure that only Active IP addresses are selected during storage pool creation.
• Enabled instance and volume snapshots for volumes created with Create on storage.
• Added support for creating NFS3 and iSCSI primary storage pools with a minimum capacity of 20 MB.
• Fixed the issue preventing multiple VMs from being created on Oracle Linux hosts.
• Ensured that iSCSI root disks created using the NetApp ONTAP storage plugin always use the QCOW2 format.

Enhancements

• Hosts added after storage pool creation can now access the storage pool during instance creation.
• Instance snapshots can now include volumes from multiple storage pools belonging to the same ONTAP cluster and SVM.
• Added best-effort data LIF selection during storage pool creation.

Types of Changes

• Breaking change — Changes existing functionality
• New feature — Adds non-breaking functionality
• Bug fix — Fixes an existing issue
• Enhancement — Improves existing functionality
• Cleanup — Code refactoring and cleanup
• 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

---

Detailed Changes

Bug Fixes

1. NFS Storage Pool Usage After Maintenance Operations

NFS storage pools can now be used after executing the EnterMaintenance and CancelMaintenance calls.

Previously

Although the NFS mount was removed, libvirtd still retained the storage pool details.

Now

---

2. Restore the updateStoragePool() Lifecycle Call

Restored the PrimaryDataStoreLifeCycle.updateStoragePool() call in StorageManagerImpl.updateStoragePool().

This call is required by the Resize StoragePool workflow.

Background

• During the updateStoragePool API call, StorageManagerImpl.updateStoragePool() updated the database but did not invoke PrimaryDataStoreLifeCycle.updateStoragePool() on the storage plugin.
• The call was removed as part of Apache CloudStack PR #7889.
• This impacted storage vendors that implemented the API.
• This PR restores the missing lifecycle call.

---

3. Graceful Handling of 404 Exceptions During Delete Workflows

404 exceptions for ExportPolicy, ONTAP volumes, and ONTAP snapshots are now handled gracefully during their respective delete workflows.

---

4. Validate IP Address Status During Storage Pool Creation

Added validation to ensure that the IP address selected during storage pool creation is in the Active state.

Previously, an IP address that was not in the Active state could be selected.

Refer to the screenshots in Data LIF Selection During Storage Pool Creation.

---

5. Snapshots for Volumes Created with Create on Storage

Verified that users can take instance and volume snapshots for volumes created with the Create on storage option enabled.

Test Setup

Created a new volume named ADDN-DISK with Create on storage enabled and attached it to the testvm instance.

Additional Disk Created and Attached

Volume Snapshot Created for ADDN-DISK

Corresponding ONTAP Snapshot

---

6. Minimum Primary Storage Pool Capacity

Users can now create NFS3 or iSCSI primary storage pools with a minimum capacity of 20 MB.

Case 1: Capacity Less Than the Minimum

Storage Pool Capacity = 214 B

Case 2: Capacity Exactly 20 MB

Storage Pool Capacity = 20,971,520 B (20 MB)

---

7. Multiple VMs on Oracle Linux Hosts

Users can now create more than one VM on a host running Oracle Linux.

Previously

Now

---

8. QCOW2 Format for iSCSI Root Disks

The root disk created during instance creation on an iSCSI primary storage pool now always uses the QCOW2 format when the NetApp ONTAP storage plugin is used.

---

Enhancements

1. Host Access to Storage Pools Created Before the Host Was Added

A host added after storage pool creation can now access the storage pool and create volumes during instance creation.

Protocol-Specific Behavior

• NFS3: The host gets access to the storage pool immediately after being added to the cluster.
• iSCSI: The host gets access to the storage pool during instance creation.

Test Steps

Only One Host Present in the Cluster

Created an NFS3 Storage Pool with Cluster Scope

ONTAP Volume Created for the Storage Pool

Export Policy Rule and Client

Added a New Host

Updated Client in the Export Policy

Removed the Host from the Cluster

Client Removed from the Export Policy

---

2. Instance Snapshots Across Multiple Storage Pools

Users can now take instance snapshots when the instance has volumes from different storage pools belonging to the same ONTAP cluster and SVM.

Test Steps

Created ontapNFS3 and ontapiSCSI Storage Pools

The storage pools use the NFS3 and iSCSI protocols, respectively.

Created an Instance with Volumes from Both Storage Pools

The root disk was created on ontapiSCSI, and the data disk was created on ontapNFS3.

Created an Instance Snapshot

Corresponding ONTAP Snapshot for the Root Disk

Corresponding ONTAP Snapshot for the Data Disk

---

3. Data LIF Selection During Storage Pool Creation

Added best-effort data LIF selection during storage pool creation.

The selection priority is:

Home-node LIF of the selected aggregate > Active LIF on the home node of the selected aggregate > Active LIF on the selected aggregate

Note: The following screenshots were captured for NFS3. The same behavior applies to iSCSI.

Based on the available free space, the plugin selects sti246_vsim_ocvs040d_aggr1 by default.

Scenario 1: pool_P1 — No LIFs Were Down

Result: The first available LIF with matching current and home nodes was selected.

Scenario 2: pool_P2_1 — LIFs on Node 040d Were Down

One LIF had 040d as its current node and 040c as its home node.

Scenario 3: pool_P3 — No LIFs on Node 040d Were Up

The first available LIF was selected from node 040c.

[Copilot]

Pull request overview

This PR enhances the NetApp ONTAP primary storage plugin’s snapshot, VM snapshot, and storage pool operations. It refines how CloudStack interacts with ONTAP for snapshot lifecycle (take/delete/revert), improves data LIF selection behavior, and adds consistency-group orchestration to support VM snapshots spanning multiple FlexVols.

Changes:

• Adjusts snapshot handling so ONTAP managed-primary volume snapshots remain on primary storage (skipping secondary-archive flows) and delegates ONTAP snapshot deletion to the strategy layer.
• Implements ONTAP VM snapshots using a temporary consistency group (two-phase start/commit) when VM volumes span multiple FlexVols; uses direct FlexVol snapshots for the single-FlexVol case.
• Improves operational robustness: idempotent “not found” handling for deletes, ONTAP-safe snapshot naming helpers, and node-affinity-aware data LIF selection with optional alerting.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java	Skips secondary archiving for ONTAP managed-primary snapshots based on payload location.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java	Delegates ONTAP snapshot delete/revert orchestration to strategy and normalizes snapshot naming/metadata usage.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java	Adds temporary CG two-phase flow for multi-FlexVol VM snapshots; direct snapshot flow for single FlexVol; improves testability.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java	Adds connect(ops-only) mode, node-affinity-aware data LIF selection (returns Pair), job polling helpers, and FlexVol snapshot delete helper.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java	Makes export-policy delete idempotent on 404 and implements updateAccessGroup for export-policy client maintenance.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java	Updates NFS export policy rules on host connect and host removal for NFS pools.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java	Persists SVM UUID and processes data-LIF selection warnings (optionally sends alerts).
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java	Adds ops-only strategy connect, ONTAP-safe snapshot naming helpers, and “object not found” detection.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java	Adds constants for SVM UUID, LIF/node-affinity fields, CG flows, and job polling.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java	Adds HostRuleAction (ADD/REMOVE) to drive export policy updates.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java	Adds ONTAP consistency-group and CG snapshot REST endpoints.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/AggregateFeignClient.java	Adds query-map support for requesting specific aggregate fields.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/IpInterface.java	Adds state/enabled/location fields for LIF selection and node affinity.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ExportRule.java	Improves enum JSON serialization/deserialization robustness.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Aggregate.java	Adds node/space setters and node model for aggregate node affinity.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ConsistencyGroup.java	Adds model for CG create/list requests.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ConsistencyGroupSnapshot.java	Adds model for CG snapshot start/commit and list responses.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ConsistencyGroupVolume.java	Adds model for CG volume members.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ConsistencyGroupVolumeProvisioningOptions.java	Adds model for CG volume provisioning options.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java	Expands VM snapshot unit tests to cover single-FlexVol and temporary-CG flows.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java	Adds tests for “object not found” error matching.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java	Adds coverage for updateAccessGroup behavior and 404 delete behavior.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java	Updates tests for new connect(false), LIF selection Pair, and new job/snapshot helpers.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java	Adds tests for data LIF warnings, failures, and alert integration.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java	Extends capabilities assertions and validates KVM format selection behavior.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java	Adds deleteStoragePool(uuid, details) override delegating to uuid-only delete.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java	Adjusts deleteStoragePool ordering and details-aware delete call.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java:1638

• updateSnapshotPayload is now called with 4 parameters, but the only method in this class still requires (storagePoolId, payload, isKvmAndFileBasedStorage, poolType, clusterId). This is a compile-time error; restore the missing poolType argument (and keep the local variable if needed later).

        boolean isKvmAndFileBasedStorage = isHypervisorKvmAndFileBasedStorage(volume, storagePool);
        boolean backupSnapToSecondary = isBackupSnapshotToSecondaryForZone(volume.getDataCenterId());

        updateSnapshotPayload(volume.getPoolId(), payload, isKvmAndFileBasedStorage, clusterId);

[codecov]

Codecov Report

❌ Patch coverage is 50.23787% with 523 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.79%. Comparing base (ba67a62) to head (77f078d).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...ck/storage/vmsnapshot/OntapVMSnapshotStrategy.java	59.85%	94 Missing and 18 partials ⚠️
...cloudstack/storage/listener/OntapHostListener.java	0.00%	80 Missing ⚠️
...hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java	0.00%	58 Missing ⚠️
...ck/storage/driver/OntapPrimaryDatastoreDriver.java	16.07%	44 Missing and 3 partials ⚠️
.../storage/feign/model/ConsistencyGroupSnapshot.java	24.59%	45 Missing and 1 partial ⚠️
...he/cloudstack/storage/service/StorageStrategy.java	70.54%	25 Missing and 18 partials ⚠️
...he/cloudstack/storage/utils/OntapStorageUtils.java	44.28%	20 Missing and 19 partials ⚠️
...om/cloud/storage/snapshot/SnapshotManagerImpl.java	0.00%	18 Missing ⚠️
...ache/cloudstack/storage/feign/model/Aggregate.java	50.00%	16 Missing ⚠️
...ain/java/com/cloud/storage/StorageManagerImpl.java	0.00%	14 Missing ⚠️
... and 10 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #13897      +/-   ##
============================================
+ Coverage     19.72%   19.79%   +0.07%     
- Complexity    19944    20075     +131     
============================================
  Files          6371     6375       +4     
  Lines        575738   576529     +791     
  Branches      70471    70592     +121     
============================================
+ Hits         113587   114150     +563     
- Misses       449805   449974     +169     
- Partials      12346    12405      +59     

Flag	Coverage Δ
uitests	3.41% <ø> (ø)
unittests	21.08% <50.23%> (+0.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java:1259

• The final @Test method block is indented as if it were nested, which is inconsistent with the rest of the class and can trigger Checkstyle indentation rules.

        @Test
        void testDeleteFlexVolSnapshotForCloudStackVolume_Feign404_TreatedAsSuccess() {
                FeignException notFoundException = mock(FeignException.class);
                when(notFoundException.status()).thenReturn(404);
                when(snapshotFeignClient.deleteSnapshot(anyString(), eq("fv-uuid-1"), eq("snap-uuid-1")))

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java:215

• buildOntapSnapshotName(): when the uniqueness suffix is very long (suffix length >= MAX_SNAPSHOT_NAME_LENGTH), maxBaseLength becomes <= 0 and the code calls normalizedBase.substring(0, maxLength). If normalizedBase is shorter than maxLength (e.g., "snapshot"), this throws StringIndexOutOfBoundsException. This can surface unexpectedly if a caller passes a long suffix (UUIDs, paths, etc.).

        int maxLength = OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH;
        int maxBaseLength = maxLength - suffix.length();
        if (maxBaseLength <= 0) {
            return normalizedBase.substring(0, maxLength);
        }

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

Suppressed comments (4)

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java:536

• getNetworkInterface() dereferences iface.getIp().getAddress() without guarding against iface, iface.getIp(), or the address being null. A partial/unexpected ONTAP response would cause a NullPointerException and prevent pool creation.

            for (IpInterface iface : response.getRecords()) {
                if (!Boolean.TRUE.equals(iface.getEnabled()) || !OntapStorageConstants.LIF_STATE_UP.equals(iface.getState())) {
                    continue;
                }
                if (!isIPv4Address(iface.getIp().getAddress())) {
                    continue;
                }

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java:449

• createExportPolicyRequest() can build an export client match of "null/32" when a Host has neither storageIpAddress nor privateIpAddress (because ip + "/32" string-concats null). That would push an invalid client entry to ONTAP.

            String hostStorageIp = host.getStorageIpAddress() != null ? host.getStorageIpAddress().trim() : null;
            String ip = (hostStorageIp != null && !hostStorageIp.isEmpty())
                    ? hostStorageIp
                    : (host.getPrivateIpAddress() != null ? host.getPrivateIpAddress().trim() : null);
            String ipToUse = ip + "/32";

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java:213

• hostDisconnected() is invoked by StorageManagerImpl.disconnectHostFromSharedPool (server/src/main/java/com/cloud/storage/StorageManagerImpl.java:1935), but this implementation currently does nothing and always returns false. This prevents proper detach/unmount cleanup on host disconnect (e.g., maintenance flows) and the in-code note claiming it isn't called is incorrect.

    public boolean hostDisconnected(long hostId, long poolId) {
        logger.info("hostDisconnected: Disconnecting host {} from pool {}", hostId, poolId);
        // Note: This is not currently being called for NetApp ONTAP storage plugin.
        return false;
    }

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java:213

• validateAndSelectAggregatesForVolumeCreation overwrites this.aggregates on every eligible aggregate and ends up keeping only the last eligible entry. This prevents createStorageVolume() from selecting the best aggregate across all eligible candidates and can lead to an unintended aggregate being used for pool provisioning.

            logger.debug("Found aggregate: " + aggr.getName() + " with UUID: " + aggr.getUuid());
            Aggregate aggrResp = aggregateFeignClient.getAggregateByUUID(authHeader, aggr.getUuid(),
                        Map.of(OntapStorageConstants.FIELDS, OntapStorageConstants.AGGREGATE_NODE
                                + OntapStorageConstants.COMMA + OntapStorageConstants.AGGREGATE_SPACE
                                + OntapStorageConstants.COMMA + OntapStorageConstants.STATE));

[Copilot]

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.

Suppressed comments (1)

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java:535

• getNetworkInterface assumes iface.getIp() and iface.getIp().getAddress() are non-null. If ONTAP returns an interface record without an IP payload (or a null address), this will throw a NullPointerException before filtering.

                if (!Boolean.TRUE.equals(iface.getEnabled()) || !OntapStorageConstants.LIF_STATE_UP.equals(iface.getState())) {
                    continue;
                }
                if (!isIPv4Address(iface.getIp().getAddress())) {
                    continue;

[Copilot]

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:290

• processDataLifSelection assumes lifResult is non-null; if getNetworkInterface() ever returns null (or a future refactor changes behavior), this will throw a NullPointerException before producing the intended CloudRuntimeException. Guard against a null Pair explicitly.

    private void processDataLifSelection(Pair<String, String> lifResult, Map<String, String> details,
                                         String storagePoolName, Long zoneId, Long podId) {
        String dataLIF = lifResult.first();
        if (dataLIF == null || dataLIF.isEmpty()) {
            throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP, cannot create primary storage");

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

• When catching InterruptedException, the thread's interrupted status should be restored (Thread.currentThread().interrupt()), otherwise higher-level code may not observe the interrupt and shutdown/cancellation can be delayed.

            } catch (InterruptedException ex) {
                logger.warn("Interrupted while waiting for iSCSI device {} to become available", volumeUuid, ex);
                return false;

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java:213

• Inside validateAndSelectAggregatesForVolumeCreation, once an eligible aggregate is found, the loop continues and this.aggregates keeps getting overwritten; the final selection therefore depends on the iteration order of svm.getAggregates(). Also, the method currently stores the unvalidated aggr object rather than the fully populated/validated aggrResp returned by ONTAP.

            logger.info("Selected aggregate: " + aggr.getName() + " for volume operations.");
            this.aggregates = List.of(aggr);
        }

[Copilot]

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.

Suppressed comments (5)

Previously missed (3) — in code that hasn't changed since the last review.

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java:215

• buildOntapSnapshotName(): when suffix length is >= MAX_SNAPSHOT_NAME_LENGTH, maxBaseLength <= 0 and normalizedBase.substring(0, maxLength) can throw if normalizedBase is shorter than maxLength. Guard the substring to avoid StringIndexOutOfBoundsException.

        int maxLength = OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH;
        int maxBaseLength = maxLength - suffix.length();
        if (maxBaseLength <= 0) {
            return normalizedBase.substring(0, maxLength);
        }

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java:128

• initialize(): validateInitializeInputs(...) is called twice with the same arguments (once before building parameters, and again immediately after). This is redundant and makes future changes riskier if the method gains side effects.

        validateInitializeInputs(capacityBytes, podId, clusterId, zoneId, storagePoolName, providerName, managed, details);

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java:801

• jobPollUntilSuccess(): the loop sleeps after every poll, even when the current poll already returned JOB_SUCCESS. This adds an unnecessary delay (one extra poll interval) to otherwise-fast operations. Return immediately when state becomes SUCCESS before sleeping.

                jobRetryCount++;
                Thread.sleep(sleepTimeInMilliSecs);
            }

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

• waitForDiskToBecomeAvailable(): InterruptedException is caught and the method returns false, but the thread interrupt flag is not restored. This can break higher-level shutdown/cancellation logic.

            try {
                Thread.sleep(timeBetweenTries);
            } catch (InterruptedException ex) {
                logger.warn("Interrupted while waiting for iSCSI device {} to become available", volumeUuid, ex);
                return false;
            }

plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java:957

• File ends with an extra closing brace (}}), which will not compile.

    }}

[Copilot]

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

server/src/main/java/com/cloud/storage/StorageManagerImpl.java:1277

• capacityBytes is a Long, but the update check uses reference comparison (!=) against pool.getCapacityBytes(). This can incorrectly treat equal values as different (or vice-versa) depending on boxing/caching, causing unintended updates and plugin lifecycle calls.

            if (capacityBytes != pool.getCapacityBytes()) {

[sandeeplocharla]

@blueorangutan package

[blueorangutan]

@sandeeplocharla a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18957

[sandeeplocharla]

@blueorangutan test

[sandeeplocharla]

Hi @DaanHoogland
Can you please help me trigger the tests for this PR?

[DaanHoogland]

@blueorangutan test

[blueorangutan]

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

[blueorangutan]

[SF] Trillian test result (tid-16809)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 60368 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13897-t16809-kvm-ol8.zip
Smoke tests completed. 152 look OK, 4 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestClusterDRS>:setup	Error	0.00	test_cluster_drs.py
test_list_system_vms_metrics_history	Failure	0.87	test_metrics_api.py
test_01_ssl_offloading_isolated_network	Failure	729.45	test_ssl_offloading.py
test_02_ssl_offloading_project_vpc	Failure	728.57	test_ssl_offloading.py
test_1_userdata_on_ssvm	Failure	127.75	test_systemvm_userdata.py

[sandeeplocharla]

Hi @DaanHoogland @winterhazel @nvazquez @weizhouapache
Could you folks please review this PR and let me know your thoughts?

Also, I'm not able see the option to request review from people and blueorangutan test command doesn't work when I comment. Just curious on how they work here?

[weizhouapache]

thanks @sandeeplocharla @rajiv-jain-netapp and all NetApp contributors
I added this to 24.0.0 milestone as we will create 4.23.0 RC3 soon.

@sandeeplocharla @blueorangutan test can be only kicked by few authorized users.

[blueorangutan]

@weizhouapache [SL] unsupported parameters provided. Supported mgmt server os are: suse15, alma10, ol10, rocky10, alma9, centos7, centos6, rocky9, alma8, ubuntu18, ol9, ol8, ubuntu22, debian12, ubuntu20, ubuntu26, rocky8, ubuntu24. Supported hypervisors are: kvm-centos6, kvm-centos7, kvm-rocky8, kvm-rocky9, kvm-rocky10, kvm-ol8, kvm-ol9, kvm-ol10, kvm-alma8, kvm-alma9, kvm-alma10, kvm-ubuntu18, kvm-ubuntu20, kvm-ubuntu22, kvm-ubuntu24, kvm-ubuntu26, kvm-debian12, kvm-suse15, vmware-55u3, vmware-60u2, vmware-65u2, vmware-67u3, vmware-70u1, vmware-70u2, vmware-70u3, vmware-80, vmware-80u1, vmware-80u2, vmware-80u3, vmware-80u3e, xenserver-65sp1, xenserver-71, xenserver-74, xenserver-84, xcpng74, xcpng76, xcpng80, xcpng81, xcpng82, xcpng83

[rajiv-jain-netapp]

thanks @sandeeplocharla @rajiv-jain-netapp and all NetApp contributors I added this to 24.0.0 milestone as we will create 4.23.0 RC3 soon.

@sandeeplocharla @blueorangutan test can be only kicked by few authorized users.

Appreciate @weizhouapache