Skip to content

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

Closed
jmsperu wants to merge 2 commits into
apache:mainfrom
jmsperu:fix/nas-backup-null-host-npe
Closed

Fix NPE in NASBackupProvider when no running KVM host is available#12680
jmsperu wants to merge 2 commits into
apache:mainfrom
jmsperu:fix/nas-backup-null-host-npe

Conversation

@jmsperu

Copy link
Copy Markdown
Collaborator

Description

NASBackupProvider.syncBackupStorageStats() crashes with a NullPointerException when ResourceManager.findOneRandomRunningHostByHypervisor() returns null. This happens when no KVM host in the zone has status=Up at the moment the BackupSyncTask runs (e.g., during management server startup, brief agent disconnections, or host state transitions).

The NPE kills the entire BackupSyncTask background job every sync interval (default 300s), flooding the management server log with stack traces and preventing backup storage stats from being updated.

The same pattern exists in deleteBackup() where host can be null when the VM is removed and no running KVM host is available.

Stack Trace

ERROR [o.a.c.b.B.BackupSyncTask] Error trying to run backup-sync background task due to:
[Cannot invoke "com.cloud.host.Host.getId()" because "host" is null].
java.lang.NullPointerException: Cannot invoke "com.cloud.host.Host.getId()" because "host" is null
at org.apache.cloudstack.backup.NASBackupProvider.syncBackupStorageStats(NASBackupProvider.java:544)
at org.apache.cloudstack.backup.BackupManagerImpl$BackupSyncTask.runInContext(BackupManagerImpl.java:1947)

Changes

  • syncBackupStorageStats(): Add early return with warning log when no running KVM host is found
  • deleteBackup(): Add null check with descriptive CloudRuntimeException

How Has This Been Tested

Tested on a production CloudStack 4.22.0.0 deployment with 3 KVM hosts and NAS (NFS) backup provider. Before the fix, the NPE fired every 5 minutes after management server restart. After applying the patch, the BackupSyncTask runs cleanly — logging a warning during startup and resuming normally once hosts reconnect.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Screenshot / Video (if applicable)

Before (every 5 minutes):

ERROR [o.a.c.b.B.BackupSyncTask] Error trying to run backup-sync background task due to:
[Cannot invoke "com.cloud.host.Host.getId()" because "host" is null]

After:

WARN [o.a.c.b.NASBackupProvider] Unable to find a running KVM host in zone 1 to sync backup storage stats

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
@boring-cyborg

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 fixes a NullPointerException in NASBackupProvider that occurs when no running KVM host is available in a zone. The NPE was crashing the BackupSyncTask background job every sync interval and preventing backup storage stats from being updated.

Changes:

  • Added null check in syncBackupStorageStats() with early return and warning log when no running KVM host is found
  • Added null check in deleteBackup() with descriptive exception when no running KVM host is available

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

Comment on lines +570 to +572
if (CollectionUtils.isEmpty(repositories)) {
return;
}

CopilotAIFeb 23, 2026

Copy link

Choose a reason for hiding this comment

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

The empty repositories check is placed after the method starts processing but before the host lookup. If repositories are empty, the host lookup at line 573 becomes unnecessary. Consider moving this check before the host lookup, or better yet, to the beginning of the method to avoid any unnecessary operations when there are no repositories to process.

Copilot uses AI. Check for mistakes.
@codecov

codecovBot commented Feb 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.92%. Comparing base (30e6c22) to head (80435b4).
⚠️ Report is 23 commits behind head on main.

Files with missing linesPatch %Lines
...rg/apache/cloudstack/backup/NASBackupProvider.java0.00%4 Missing and 3 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #12680 +/- ##
============================================
- Coverage 17.92% 17.92% -0.01% 
Complexity 16154 16154 ============================================
Files 5939 5939 Lines 533181 533188 +7 Branches 65237 65240 +3 ============================================
+ Hits 95585 95586 +1 - Misses 426856 426860 +4 - Partials 10740 10742 +2 
FlagCoverage Δ
uitests3.67% <ø> (ø)
unittests19.03% <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.

@DaanHoogland

Copy link
Copy Markdown
Contributor

@jmsperu , would you want this included in the next 22 release (in which case you might to want to base it of the 4.22 branch)

@jmsperu

Copy link
Copy Markdown
CollaboratorAuthor

@jmsperu , would you want this included in the next 22 release (in which case you might to want to base it of the 4.22 branch)

Hello, am good with int he next release.

@DaanHooglandDaanHoogland added this to the 4.23.0 milestone Feb 25, 2026
@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland 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

Copy link
Copy Markdown

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

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

TestResultTime (s)Test File
test_LoginApiDomainError7.70test_accounts.py
ContextSuite context=TestListIdsParams>:teardownError1.15test_list_ids_parameter.py
test_01_verify_ipv6_networkFailure491.74test_network_ipv6.py
test_01_snapshot_root_diskError5.00test_snapshots.py
test_02_list_snapshots_with_removed_data_storeError46.85test_snapshots.py
test_02_list_snapshots_with_removed_data_storeError46.85test_snapshots.py
ContextSuite context=TestSnapshotStandaloneBackup>:teardownError27.62test_snapshots.py
test_01_snapshot_usageError28.00test_usage.py
test_01_vpn_usageError1.12test_usage.py

@sureshanapartisureshanaparti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clgtm

@sureshanaparti

Copy link
Copy Markdown
Contributor

Hi @jmsperu Can you re-base with 4.22 branch if this is relavent and can targeted for 4.22.1. Thanks.

@weizhouapache

Copy link
Copy Markdown
Member

@abh1sar
can you please review it ?

@abh1sarabh1sar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code LGTM

@abh1sar

Copy link
Copy Markdown
Contributor

@jmsperu Can you please rebase to 4.22?

@jmsperu

Copy link
Copy Markdown
CollaboratorAuthor

Hello, done.

@jmsperujmsperu closed this Mar 12, 2026
@DaanHoogland

Copy link
Copy Markdown
Contributor

Hello, done.

hello @jmsperu , what do you mean done? did you rebase or is your issue gone? Is this PR no longer relevant?

@jmsperu

Copy link
Copy Markdown
CollaboratorAuthor

Rebased onto 4.22 and opened a new PR since the original couldn't be reopened after the base branch change: #12805

@DaanHoogland

Copy link
Copy Markdown
Contributor

Rebased onto 4.22 and opened a new PR since the original couldn't be reopened after the base branch change: #12805

ok, thanks, that wouldn’t have been a problem @jmsperu .

Sign up for freeto 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.

NullPointerException in NASBackupProvider.syncBackupStorageStats when no KVM host is in "Up" state

7 participants

@jmsperu@DaanHoogland@blueorangutan@sureshanaparti@weizhouapache@abh1sar