Skip to content

Fix NPE in NASBackupProvider when no running KVM host is available#12805

Open
jmsperu wants to merge 2 commits intoapache:4.22from
jmsperu:fix/nas-backup-null-host-npe
Open

Fix NPE in NASBackupProvider when no running KVM host is available#12805
jmsperu wants to merge 2 commits intoapache:4.22from
jmsperu:fix/nas-backup-null-host-npe

Conversation

@jmsperu
Copy link

@jmsperu jmsperu commented Mar 12, 2026

Rebased onto 4.22 as requested (previously #12680).

ResourceManager.findOneRandomRunningHostByHypervisor() can return null when no KVM host in the zone has status=Up (e.g. during management server startup, brief agent disconnections, or host state transitions).

NASBackupProvider.syncBackupStorageStats() and deleteBackup() call host.getId() without a null check, causing a NullPointerException that crashes the entire BackupSyncTask background job every sync interval.

This adds null checks in both methods:

  • syncBackupStorageStats: log a warning and return early
  • deleteBackup: throw CloudRuntimeException with a descriptive message

ResourceManager.findOneRandomRunningHostByHypervisor() can return null
when no KVM host in the zone has status=Up (e.g. during management
server startup, brief agent disconnections, or host state transitions).

NASBackupProvider.syncBackupStorageStats() and deleteBackup() call
host.getId() without a null check, causing a NullPointerException that
crashes the entire BackupSyncTask background job every sync interval.

This adds null checks in both methods:
- syncBackupStorageStats: log a warning and return early
- deleteBackup: throw CloudRuntimeException with a descriptive message
@DaanHoogland
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.60%. Comparing base (27bce46) to head (1481586).
⚠️ Report is 4 commits behind head on 4.22.

Files with missing lines Patch % Lines
...rg/apache/cloudstack/backup/NASBackupProvider.java 0.00% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               4.22   #12805      +/-   ##
============================================
- Coverage     17.61%   17.60%   -0.01%     
+ Complexity    15662    15661       -1     
============================================
  Files          5917     5917              
  Lines        531415   531437      +22     
  Branches      64973    64976       +3     
============================================
- Hits          93588    93586       -2     
- Misses       427271   427292      +21     
- Partials      10556    10559       +3     
Flag Coverage Δ
uitests 3.70% <ø> (ø)
unittests 18.68% <0.00%> (-0.01%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

@blueorangutan
Copy link

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 17121

@DaanHoogland
Copy link
Contributor

@jmsperu , probably due to the rebase:

20:18:12 [ERROR] /jenkins/workspace/acs-centos8-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.22.1.0-SNAPSHOT/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:[558,12] error: cannot find symbol
20:18:12   symbol:   variable CollectionUtils
20:18:12   location: class NASBackupProvider

seems an import is missing.

@jmsperu
Copy link
Author

jmsperu commented Mar 18, 2026

Good catch @DaanHoogland, the CollectionUtils import was dropped during the rebase. Fixed now — should build cleanly.

@DaanHoogland DaanHoogland added this to the 4.22.1 milestone Mar 19, 2026
@DaanHoogland
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

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

@DaanHoogland DaanHoogland requested review from abh1sar and Copilot March 19, 2026 15:37
@DaanHoogland
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a crash in the NAS backup provider’s background sync/deletion flows when no running KVM host is available in a zone (i.e., ResourceManager.findOneRandomRunningHostByHypervisor(...) returns null), preventing BackupSyncTask from dying due to NullPointerException.

Changes:

  • Add a null-host check in deleteBackup() and fail with a descriptive CloudRuntimeException.
  • Add early returns in syncBackupStorageStats() when there are no repositories and when no running KVM host can be found (with a warning log).

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

Comment on lines 63 to 68
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Collections;
import org.apache.commons.collections.CollectionUtils;
import java.util.Comparator;
import java.util.Date;
Comment on lines +475 to +477
if (host == null) {
throw new CloudRuntimeException(String.format("Unable to find a running KVM host in zone %d to delete backup %s", backup.getZoneId(), backup.getUuid()));
}
Comment on lines 562 to +566
final Host host = resourceManager.findOneRandomRunningHostByHypervisor(Hypervisor.HypervisorType.KVM, zoneId);
if (host == null) {
logger.warn("Unable to find a running KVM host in zone {} to sync backup storage stats", zoneId);
return;
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants