CSTACKEX-212: fix for snapshot failure for attached cs volumes nfs an… by sathvikaragi · Pull Request #77 · NetApp/cloudstack

sathvikaragi · 2026-07-10T04:58:56Z

Description

Fix snapshot failure for CloudStack volumes attached to running VMs on ONTAP primary storage (both NFS3 and iSCSI protocols).

This PR...

When a volume was created and attached to a running VM in a single step,by enabling create on storage and choose the storage pool tag the volume format was not being set correctly. The format is now determined by the hypervisor type (KVM → QCOW2) in ontapdriver via [getImageFormatByHypervisor(HypervisorType] mirroring the [getSupportedImageFormatForCluster] in VolumeOrchestrator file.

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

How Has This Been Tested?

Tested on a dev setup against these scenarios:

Scenario A — Attach data disk to running VM, then snapshot

Create ONTAP primary storage pool (NFS3 or iSCSI)
Deploy VM with data disk using pool-tagged disk offering → VM reaches Running state
Create volume attached to the running VM by enabling create on storage and choose the storage pool tag
Take snapshot of attached volume — ✅ succeeds (was failing before fix)

Scenario B — Attach volume to root-disk-only VM, then snapshot

Create ONTAP primary storage pool (NFS3 or iSCSI)
Deploy VM without data disk → VM reaches Running state
Create and attach volume to the running VM by enabling create on storage and choose the storage pool tag
Take snapshot of attached volume — ✅ succeeds (was failing before fix)

scenarios-C-create a volume on storage pool but not attach to any vm

Create ONTAP primary storage pool (NFS3 or iSCSI)
Create volume by enabling create on storage and choose the storage pool tag
Take snapshot- — ✅ succeeds (was failing before fix)

Screenshots (if appropriate):

snapshots for when cs volume is attached to NFS and ISCSI instance that has both root and data disk and create on storage enabled:

verifying on ontap and db:

snapshots for when cs volume is attached to NFS and ISCSI instance that has only root and also case when they are not attached to any instance and create on storage enabled:

How did you try to break this feature and the system with this change?

…d iscsi

Copilot

Pull request overview

Fixes snapshot failures for ONTAP-backed volumes that are created-and-attached to running VMs in a single step by ensuring the volume’s image format is set consistently (KVM → QCOW2) during ONTAP volume creation.

Changes:

Set VolumeVO.format during createAsync() for both iSCSI and NFS3 ONTAP-managed volumes based on the pool hypervisor.
Add unit-test stubbing for StoragePool.getHypervisor() to support the new format-resolution logic.

Reviewed changes

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

File	Description
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java	Set volume format during creation and add hypervisor→image-format resolution helper.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java	Update tests to provide pool hypervisor needed by the new format-setting logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sandeeplocharla

Awesome work!!!

Copilot

Pull request overview

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

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

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

+        VolumeVO volVO = _volsDao.findById(volumeInfo.getId());
+        if (volVO.getFormat() == null) {
+            HypervisorType hyperType = storagePool.getHypervisor();
+            volVO.setFormat(getSupportedImageFormatForHypervisor(hyperType));
+        }
+        _volsDao.update(volVO.getId(), volVO);
+        return volVO;


Copilot

Pull request overview

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

+            HypervisorType hyperType = storagePool.getHypervisor();
+            ImageFormat format = getSupportedImageFormatForHypervisor(hyperType);
+            if (format != null) {


+    private ImageFormat getSupportedImageFormatForHypervisor(HypervisorType hyperType) {
+        if (hyperType == HypervisorType.XenServer) {
+            return ImageFormat.VHD;
+        } else if (hyperType == HypervisorType.KVM) {
+            return ImageFormat.QCOW2;
+        } else if (hyperType == HypervisorType.VMware) {
+            return ImageFormat.OVA;
+        } else if (hyperType == HypervisorType.Ovm) {
+            return ImageFormat.RAW;
+        } else if (hyperType == HypervisorType.Hyperv) {
+            return ImageFormat.VHDX;
+        } else {
+            return null;
+        }


sandeeplocharla

'DefaultPrimary' plugin currently supports multiple hypervisors and storage protocols. It has snapshot capabilities for a long time. Can you check if even it has the same issue as our plugin and if these changes fix it?
If it works without these changes, maybe we need to understand how its handling it.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:1014

getImageFormatByHypervisor can throw a NullPointerException when storagePool.getHypervisor() returns null because it calls hypervisorType.equals(...). Since StoragePoolVO.getHypervisor() can be null, this can break volume creation with an NPE instead of a controlled failure. Guard against null and compare enums with == (or call equals on the constant).

    private Storage.ImageFormat getImageFormatByHypervisor(HypervisorType hypervisorType) {
        if (hypervisorType.equals(HypervisorType.KVM)) {
            return Storage.ImageFormat.QCOW2;
        }
        throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution");

suryag1201

also add assertion to check the format is set or not after createAsync

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java:1015

getImageFormatByHypervisor uses hypervisorType.equals(...), which will throw a NullPointerException if StoragePool.getHypervisor() returns null. Since this method is used during createAsync, that would fail volume creation with an unhelpful NPE. Prefer enum-safe comparisons (==) and explicitly handle null with a clearer exception message.

        if (hypervisorType.equals(HypervisorType.KVM)) {
            return Storage.ImageFormat.QCOW2;
        }
        throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution");
    }

                    volumeVO.setPoolType(storagePool.getPoolType());
                    volumeVO.setPoolId(storagePool.getId());
+                    volumeVO.setFormat(getImageFormatByHypervisor(storagePool.getHypervisor()));
+                    logger.info("createAsync: Volume format set to [{}] for hypervisor [{}]", volumeVO.getFormat(), storagePool.getHypervisor());


Copilot

Pull request overview

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

CSTACKEX-212: fix for snapshot failure for attached cs volumes nfs an…

1e71f85

…d iscsi

sathvikaragi requested review from piyush5netapp, rajiv-jain-netapp and sandeeplocharla as code owners July 10, 2026 04:58

Copilot AI review requested due to automatic review settings July 10, 2026 04:58

sathvikaragi requested a review from suryag1201 as a code owner July 10, 2026 04:58

Copilot started reviewing on behalf of sathvikaragi July 10, 2026 04:59 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread ...me/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java

rajiv-jain-netapp previously approved these changes Jul 10, 2026

View reviewed changes

sandeeplocharla previously approved these changes Jul 10, 2026

View reviewed changes

suryag1201 reviewed Jul 10, 2026

View reviewed changes

Comment thread ...me/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java Outdated

Comment thread ...me/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java Outdated

CSTACKEX-212: resolving comments

8856437

Copilot AI review requested due to automatic review settings July 10, 2026 10:39

sathvikaragi dismissed stale reviews from sandeeplocharla and rajiv-jain-netapp via 8856437 July 10, 2026 10:39

Copilot started reviewing on behalf of sathvikaragi July 10, 2026 10:39 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

rajiv-jain-netapp previously approved these changes Jul 10, 2026

View reviewed changes

CSTACKEX-212: format set in volumeapiserviceimpl instead of ontap driver

7c7f5a1

Copilot AI review requested due to automatic review settings July 14, 2026 19:06

sathvikaragi dismissed rajiv-jain-netapp’s stale review via 7c7f5a1 July 14, 2026 19:06

Copilot started reviewing on behalf of sathvikaragi July 14, 2026 19:07 View session

CSTACKEX-212: small fix

7c7fa12

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Outdated

Comment thread server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Outdated

Comment thread server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Outdated

Copilot AI review requested due to automatic review settings July 14, 2026 19:11

Copilot started reviewing on behalf of sathvikaragi July 14, 2026 19:12 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

CSTACKEX-212: resolving copilot comments

2561002

Copilot AI review requested due to automatic review settings July 14, 2026 20:10

Copilot started reviewing on behalf of sathvikaragi July 14, 2026 20:11 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

sandeeplocharla reviewed Jul 16, 2026

View reviewed changes

CSTACKEX-212: move format handling back to ONTAP driver

13bef9e

Copilot AI review requested due to automatic review settings July 22, 2026 05:20

Copilot started reviewing on behalf of sathvikaragi July 22, 2026 05:20 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread ...me/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java

suryag1201 reviewed Jul 22, 2026

View reviewed changes

Comment thread ...ntap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java

suryag1201 reviewed Jul 22, 2026

View reviewed changes

CSTACKEX-212: adding assertions to test file

0009b31

Copilot AI review requested due to automatic review settings July 22, 2026 06:00

Copilot started reviewing on behalf of sathvikaragi July 22, 2026 06:01 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

CSTACKEX-212: resolving copilot comment pt-2

f2a1d4c

Copilot AI review requested due to automatic review settings July 22, 2026 06:29

Copilot started reviewing on behalf of sathvikaragi July 22, 2026 06:30 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

rajiv-jain-netapp approved these changes Jul 22, 2026

View reviewed changes

suryag1201 approved these changes Jul 22, 2026

View reviewed changes

sandeeplocharla approved these changes Jul 22, 2026

View reviewed changes

sathvikaragi merged commit 9ed47cd into main Jul 22, 2026
20 checks passed

Uh oh!

Conversation

sathvikaragi commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

How Has This Been Tested?

Screenshots (if appropriate):

How did you try to break this feature and the system with this change?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

sandeeplocharla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

sandeeplocharla left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

suryag1201 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sathvikaragi commented Jul 10, 2026 •

edited

Loading