backport: Incremental NAS backup support for KVM (#13074) - #13877
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% 18.12% +0.33%
- Complexity 15995 16329 +334
============================================
Files 5928 5930 +2
Lines 534306 544571 +10265
Branches 65383 69384 +4001
============================================
+ Hits 95069 98704 +3635
- Misses 428467 434735 +6268
- Partials 10770 11132 +362
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. |
|
[SF] Trillian test result (tid-16836)
|
|
What a wonderful backport. |
|
The incremental NAS backup backport (apache#13877) makes a restore first ask qemu-img whether the backup references a parent, and flattens the chain with qemu-img convert when it does; only a backup without a backing file is rsynced. The volume mapping tests stubbed runSimpleBashScriptForExitValue to return 0 for every command, so the probe reported a backing chain, the restore took the convert path, and the rsync invocations these tests assert on never happened. Answer the probe with "no backing chain", as the existing restore tests do, and keep 0 for the other checks.
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?