Conversation
Raw block-device disks such as LINSTOR/DRBD cannot carry QEMU persistent dirty
bitmaps -- persistence is a qcow2-only feature -- so libvirt checkpoints, and
with them the push-mode incremental path, are unavailable there. Those VMs were
therefore pinned to full-only backups, and push mode additionally wrote a fully
allocated qcow2 that needed a second re-convert pass to reclaim space.
Add a content-based path driven by libvirt pull mode. libvirt exposes a
point-in-time NBD view of each disk, held consistent by copy-before-write into
local fleecing scratch images, and the backup is derived from that export:
content-full qemu-img convert -> sparse qcow2 in a single pass, so the
bloated push output and its re-convert pass both go away.
content-incremental a qcow2 overlay backed by the NBD export, then a safe
rebase onto the parent, merging in exactly the clusters
that differ -> a delta qcow2 backed by the parent.
The delta has the same chain shape the push-mode incremental produces (relative
backing path), so the existing restore and cascade-delete logic applies
unchanged. A missing or short parent list degrades to a full and emits
INCREMENTAL_FALLBACK=true, mirroring the push-mode fallback.
Unlike a storage-snapshot based diff there is no trailing DRBD metadata to clip:
the DRBD device, and therefore the NBD export, is already net-sized.
The scratch dir is host-local and configurable with -S, defaulting to /var/tmp;
it must not sit on the NAS share since copy-before-write runs inline with guest
writes. NBD socket paths are validated against the kernel's 108-byte limit up
front to avoid an opaque qemu-img failure later.
The incremental NAS backup feature gates on QEMU dirty bitmaps, which can only be persisted inside a qcow2. LINSTOR volumes are raw DRBD block devices, so allVolumesOnCheckpointCapableStorage() rejected them and every LINSTOR VM was pinned to full-only backups regardless of the configured cadence. Route those VMs to the content-based path instead, which derives the delta by comparing the disk's point-in-time NBD export against the parent backup rather than by tracking dirty blocks: - decideContentChain() mirrors the bitmap cadence logic (master switch, stopped VMs, nas.backup.full.every, chain length) but anchors on backup history alone. There is no host-side bitmap to invalidate, so unlike the bitmap chain a VM restart, live migration or restore cannot break it -- content comparison is stateless and self-correcting. - The path is taken even when the incremental master switch is off, because a content-full is still a single-pass sparse write, strictly better than the fully allocated push-mode output it replaces. - ChainDecision carries the parent backup uuid directly, since a content chain has no bitmap name to resolve the parent by. - Mixed-storage VMs stay off the content path: one backup run uses one mode. The agent wrapper accepts the two new modes -- content-incremental requires only its per-disk parent paths, no bitmap -- and passes the host-local pull-mode scratch directory from the new nas.backup.pull.scratch.dir agent property. Adds 6 unit tests covering the routing and cadence decisions. All 21 pre-existing NASBackupProviderTest cases and the KVM wrapper tests still pass.
rp-
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14194 +/- ##
============================================
+ Coverage 17.97% 19.91% +1.94%
- Complexity 16184 20191 +4007
============================================
Files 5930 6372 +442
Lines 535615 577250 +41635
Branches 65582 70717 +5135
============================================
+ Hits 96271 114983 +18712
- Misses 428377 449687 +21310
- Partials 10967 12580 +1613
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:
|
Contributor
|
@rp- are the changes here relavant for 4.22 branch? if so, please re-target to 4.22. |
Contributor
Author
thanks, missed the wrong base branch |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds incremental NAS backup support for KVM VMs whose disks live on raw block-device storage such as LINSTOR/DRBD.
#13877 (backport of #13074) implements incremental NAS backups with QEMU dirty bitmaps and libvirt checkpoints. Bitmap persistence is a qcow2-only feature, so
allVolumesOnCheckpointCapableStorage()excludes raw block devices and every LINSTOR VM stays pinned to full-only backups regardless of the configured cadence. On top of that, the push-mode full backup of a raw device writes a fully allocated qcow2 which then needs a secondqemu-img convertpass to reclaim space.This PR adds a content-based backup path driven by libvirt pull mode and routes VMs on content-diff-capable storage (currently LINSTOR) to it:
nasbackup.sh– two new modeslibvirt is asked for a pull-mode backup: it starts an NBD server exposing a point-in-time view of each disk, held consistent by copy-before-write into host-local fleecing scratch images. The backup is then derived from that export:
content-full:qemu-img convertfrom the NBD export straight into a sparse qcow2 in a single pass. This replaces the bloated push-mode output and its re-convert pass.content-incremental: a qcow2 overlay backed by the NBD export, then a safeqemu-img rebaseonto the parent backup. This merges in exactly the clusters that differ and yields a delta qcow2 backed by the parent, with a relative backing path. The chain has the same shape as a bitmap incremental, so the existing restore and cascade-delete logic applies unchanged.A missing or short parent list degrades to a full and emits
INCREMENTAL_FALLBACK=true, mirroring the push-mode fallback. Also handles the raw/dev/drbdNdevice paths of long-running VMs the same way the push/stopped paths do (viaget_linstor_uuid_from_device).The scratch dir is host-local and configurable with
-S(default/var/tmp). It must not be on the NAS share, since copy-before-write runs inline with guest writes. NBD socket paths are validated against the kernel's 108-byte UNIX socket limit up front instead of failing opaquely inqemu-imglater.NASBackupProvider– routing and cadencedecideContentChain()mirrors the bitmap cadence logic (incremental master switch, stopped VMs,nas.backup.full.every, chain length) but anchors on backup history alone. There is no host-side bitmap that can be invalidated, so a VM restart, live migration or restore cannot break the chain; content comparison is stateless and self-correcting.nas.backup.incremental.enabledis off, because acontent-fullis still strictly better than the fully allocated push output it replaces. Incrementals themselves remain gated by the master switch.ChainDecisioncarries the parent backup uuid directly, since a content chain has no bitmap name to resolve the parent by.LibvirtTakeBackupCommandWrapper/ agentAccepts the two new modes (
content-incrementalrequires only its per-disk parent paths, no bitmap) and passes the scratch directory from the newagent.propertieskeynas.backup.pull.scratch.dir(default/var/tmp).Behaviour for VMs on qcow2/NFS/Ceph storage is unchanged; they keep using the bitmap path from #13877.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Unit tests
Six new
NASBackupProviderTestcases cover the routing and cadence decisions:decideChainReturnsContentFullForLinstorWhenIncrementalDisableddecideChainReturnsContentFullForStoppedLinstorVmdecideChainReturnsContentIncrementalForLinstorWithValidParentdecideChainReturnsContentFullForLinstorAtChainEndallVolumesOnContentDiffCapableStorageFalseForMixedStorageallVolumesOnContentDiffCapableStorageFalseForNoVolumesAll pre-existing
NASBackupProviderTestand KVM backup wrapper tests still pass (27/27 and 20/20 after rebasing onto current 4.22).End-to-end on a 3-host KVM/LINSTOR cluster (management server + 3 agents, NFS backup repository,
nas.backup.incremental.enabled=true,nas.backup.full.every=3):content-full: single sparse qcow2 per disk, no bitmap/checkpoint, restore worksfull.every=3qemu-img checkHow did you try to break this feature and the system with this change?
nas.backup.pull.scratch.dirat a deep directory so the NBD socket path exceeded 108 bytes. The script fails up front with a clear message and a hint to shorten the dir, instead of an opaqueqemu-imgconnection error mid-backup; cleanup runs and no partial backup dir is left behind.INCREMENTAL_FALLBACK=true, and the DB records a full.qemu-imgduring the pull): the backup job is aborted, scratch dir and partial output removed, mount released; the next backup succeeds./dev/drbdNinstead of/dev/drbd/by-res/…in live XML): resolved via udev like the push path; if the UUID cannot be derived the backup fails hard rather than producing a file restore cannot find.