Skip to content

feature/CSTACKEX-220: null check for volumeToAttach#81

Merged
piyush5netapp merged 1 commit into
mainfrom
feature/CSTACKEX-220
Jul 14, 2026
Merged

feature/CSTACKEX-220: null check for volumeToAttach#81
piyush5netapp merged 1 commit into
mainfrom
feature/CSTACKEX-220

Conversation

@piyush5netapp

Copy link
Copy Markdown

Description

This PR...

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?

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to prevent a NullPointerException during volume attach by adding a null-check around ONTAP-specific logic in VolumeApiServiceImpl, plus a minor formatting cleanup in the ONTAP primary datastore lifecycle implementation.

Changes:

  • Guard ONTAP-specific volume reload (grantAccess → DB reload) with a volumeToAttachStoragePool != null check.
  • Apply a small brace/whitespace formatting correction in OntapPrimaryDatastoreLifecycle.attachZone.

Reviewed changes

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

File Description
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Adds a null-check before using the storage pool provider name during attach flow.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java Minor formatting fix for an if statement in attachZone.
Comments suppressed due to low confidence (1)

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:4862

  • The new null-check prevents an NPE in the ONTAP-specific reload, but volumeToAttachStoragePool is still dereferenced multiple times in the sendCommand path (e.g. isManaged(), getHostAddress(), getId()). If _storagePoolDao.findById(volumeToAttach.getPoolId()) returns null for a running VM, the method will still throw a NullPointerException later. Consider failing fast when sendCommand is true and the storage pool cannot be resolved.
                if (volumeToAttachStoragePool != null && DataStoreProvider.ONTAP_PLUGIN_NAME.equals(volumeToAttachStoragePool.getStorageProviderName())) {
                    volumeToAttach = _volsDao.findById(volumeToAttach.getId());
                }
            }

            if (sendCommand) {
                if (host != null && host.getHypervisorType() == HypervisorType.KVM && volumeToAttachStoragePool.isManaged() && volumeToAttach.getPath() == null) {
                    volumeToAttach.setPath(volumeToAttach.get_iScsiName());

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

@piyush5netapp piyush5netapp merged commit d5406e5 into main Jul 14, 2026
20 checks passed
piyush5netapp added a commit that referenced this pull request Jul 14, 2026
### 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?

<!-- 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants