backport: Incremental NAS backup support for KVM (#13074) - #13877
backport: Incremental NAS backup support for KVM (#13074)#13877abh1sar wants to merge 7 commits into
Conversation
Implements incremental backup support for the NAS backup provider on KVM, using QEMU dirty bitmaps and libvirt's backup-begin API. RFC: #12899. For large VMs this reduces daily backup storage 80–95% and shortens backup windows from hours to minutes (e.g. a 500 GB VM with moderate writes goes from ~500 GB/day to ~5–15 GB/day after the initial full backup). Signed-off-by: James Peru <jmsperu@gmail.com> Co-authored-by: jmsperu <jmsperu@users.noreply.github.com> Co-authored-by: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com>
boring-cyborg
Bot
added
component:api
component:backup
component:integration-test
component:kvm
component:vmware
Python
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.22 #13877 +/- ##
============================================
+ Coverage 17.79% 17.83% +0.04%
- Complexity 15995 16038 +43
============================================
Files 5928 5930 +2
Lines 534306 534706 +400
Branches 65383 65460 +77
============================================
+ Hits 95069 95383 +314
- Misses 428467 428513 +46
- Partials 10770 10810 +40
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
* Avoid concurrent deletes modifying the same backup chain in parallel - Backup deletes are serialized by using a per-VM GlobalLock Backup delete checks if it has a live dependent backup not just a live immediate child.
|
@blueorangutan package |
|
@abh1sar 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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18875 |
|
@blueorangutan package |
|
@abh1sar 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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18885 |
|
@blueorangutan test |
There was a problem hiding this comment.
Pull request overview
This PR backports incremental NAS backup support for KVM by introducing chain-aware orchestration in the NAS backup provider (bitmaps/checkpoints, chain metadata, and cascade delete semantics), plus agent-side support in nasbackup.sh and restore-side chain flattening.
Changes:
- Add incremental-chain decisioning and metadata persistence to the NAS backup provider (including zone-scoped config keys and tombstone/GC semantics for deletes).
- Extend KVM agent backup/restore wrappers and
nasbackup.shto supportbackup-begin --checkpointxml, rebase-backed incrementals, and flatten-on-restore. - Add/extend unit + smoke integration tests to cover incremental cadence, restore, and delete semantics.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/smoke/test_backup_recovery_nas.py | Adds smoke coverage for incremental chain cadence/restore/delete and enables incremental config for the suite |
| server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java | Hides tombstoned (Hidden) backups from user listing; adjusts delete flow for chain-aware providers |
| server/src/main/java/com/cloud/hypervisor/KVMGuru.java | Updates backup logging field selection (type) |
| scripts/vm/hypervisor/kvm/nasbackup.sh | Implements incremental/full/legacy-full modes, checkpoint handling, rebasing, and bitmap cleanup |
| plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java | Updates backup logging field selection (type) |
| plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java | Extends stubbing for incremental restore detection path |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java | Adds incremental args validation, script invocation flags, and fallback marker handling |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java | Flattens qcow2 backing chains on restore when needed |
| plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java | Adds unit coverage for incremental enablement, chain decisions, restore clearing checkpoint, and cascade delete rules |
| plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java | Implements incremental chain orchestration, chain-aware delete + resource accounting, and VM checkpoint tracking |
| plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java | Introduces constants for chain metadata stored in backup_details and VM checkpoint detail key |
| engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java | Renames backing field to type while keeping DB column mapping stable |
| core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java | Adds incremental parameters (mode/bitmaps/parent paths) to agent command |
| core/src/main/java/org/apache/cloudstack/backup/BackupAnswer.java | Returns bitmap-created + incremental-fallback markers from agent to MS |
| api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java | Adds handlesChainDeleteResourceAccounting() capability flag for providers |
| api/src/main/java/org/apache/cloudstack/backup/Backup.java | Adds Hidden backup status for tombstoned chain members |
Suppressed comments (2)
test/integration/smoke/test_backup_recovery_nas.py:69
- When enabling
nas.backup.incremental.enabledfor the test, the update should also be applied at zone scope; otherwise a pre-existing zone override can keep incrementals disabled and break the new incremental-chain tests.
if cls.backup_provider != "nas":
Configurations.update(cls.api_client, 'backup.framework.provider.plugin', value='nas')
if cls.incremental_backup_enabled == "false":
Configurations.update(cls.api_client, 'nas.backup.incremental.enabled', value='true')
test/integration/smoke/test_backup_recovery_nas.py:102
- Cleanup restores
nas.backup.incremental.enabledwithoutzoneid, which may leave a zone override enabled/disabled incorrectly on shared test environments. Restore it withzoneid=cls.zone.idto match the scope of the config key.
if cls.backup_provider != "nas":
Configurations.update(cls.api_client, 'backup.framework.provider.plugin', value=cls.backup_provider)
if cls.incremental_backup_enabled == "false":
Configurations.update(cls.api_client, 'nas.backup.incremental.enabled', value="false")
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
just as a heads up: I have already based on this PR an addition drafted for nasbackup that enables incremental backups for Linstor + libvirt pullmode (saves extra write on the nas storage) |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
jmsperu
left a comment
jmsperu
left a comment
There was a problem hiding this comment.
Went through the backport against #13074 and the 4.22 tree. Two notes in-thread: line 980 (sb.and("backupStatus", ...)) is dead code on 4.22 because the status filter from #13254 isn't there; harmless thanks to SearchCriteria skipping unset conditions, but worth dropping. The zoneid point in the smoke test is inherited from main rather than introduced here; I'll fix it on main separately. Otherwise this matches the merged change. Happy to re-check once the conflicts with 4.22 are resolved.
# Conflicts: # plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java # scripts/vm/hypervisor/kvm/nasbackup.sh
The backupStatus condition was carried over from main, where #13254 added a status filter to listBackups. 4.22 has neither ListBackupsCmd.getBackupStatus() nor a setParameters call for it, so the condition is never populated and SearchCriteria.getWhereClause() skips it. Dead code on this branch; the statusNeq clause that hides tombstoned chain members is the one in use.
Thanks @jmsperu |
|
@blueorangutan package |
|
@abh1sar 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. |
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19001 |
|
@blueorangutan test |
|
@abh1sar a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Implements incremental backup support for the NAS backup provider on KVM, using QEMU dirty bitmaps and libvirt's backup-begin API.
For large VMs this reduces daily backup storage 80–95% and shortens backup windows from hours to minutes (e.g. a 500 GB VM with moderate writes goes from ~500 GB/day to ~5–15 GB/day after the initial full backup).
Description
This PR...
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Repeated the manual testing from #13074 (comment)
And added a few more test cases.
nas.backup.full.everybackupsnas.backup.full.everyinterval)vm_instance_details; backup record removed from DB and file deleted from NFSnas.backup.incremental.enabledmid-chain, take a backup, re-enable, take backupsactive_checkpoint_iduntouched and produces a plain legacy full with no chain metadata; re-enabling forces a fresh FULL on the first backup (since the legacy full has no matching bitmap), then resumes incrementals normallyquiescevm=trueon a VM with a live QEMU Guest Agentthawedstate, no hangdetachVolumeon a VM assigned to a backup offering withbackup.enable.attach.detach.of.volumes=false(default)nas.chain_positiondetail row (DB-level corruption), then take the next backupBackedUp), then attemptrestoreBackupHOURLYand aDAILYbackup schedule on the same VM simultaneouslybackup_schedule_ids, independentmaxBackups); each fires on its own cadence without disturbing the othermaxBackupson an existing schedule below the current schedule-attributed backup countdeleteBackupcalled on an already-removed id)size/virtualsizefields against the actual on-disk backup directory size and the VM's total provisioned disk sizeHow did you try to break this feature and the system with this change?