From 140da6fc4034cd87d7f8e6ca55807d0a7e31d110 Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Tue, 18 Aug 2026 15:50:13 +0200 Subject: [PATCH 1/7] nasbackup: content-based pull-mode backup path for raw block storage 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. --- scripts/vm/hypervisor/kvm/nasbackup.sh | 234 +++++++++++++++++++++++++ 1 file changed, 234 insertions(+) diff --git a/scripts/vm/hypervisor/kvm/nasbackup.sh b/scripts/vm/hypervisor/kvm/nasbackup.sh index 04cc7cd86f25..d648c31f91c3 100755 --- a/scripts/vm/hypervisor/kvm/nasbackup.sh +++ b/scripts/vm/hypervisor/kvm/nasbackup.sh @@ -42,6 +42,9 @@ PARENT_PATHS="" # For incremental: comma-separated list of parent backup f # is rebased onto its corresponding parent file. Required because # data-disk backup files don't share the root volume's UUID, so # each disk must be rebased onto its own parent. +SCRATCH_PARENT="/var/tmp" # Parent dir for the pull-mode scratch area (NBD socket + + # libvirt fleecing images). Host-local, never the NAS share. + # Overridable with -S (agent.properties nas.backup.pull.scratch.dir). logFile="/var/log/cloudstack/agent/agent.log" EXIT_CLEANUP_FAILED=20 @@ -183,6 +186,219 @@ thaw_guest() { return 1 } +# --------------------------------------------------------------------------- +# Content-based backup path (libvirt pull mode). +# +# Used for raw block-device disks such as LINSTOR/DRBD. Those 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. +# +# Instead libvirt 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 local +# fleecing scratch images. The backup is then derived from that export: +# +# content-full qemu-img convert -> sparse standalone qcow2. Zero detection +# happens while streaming, so the fully allocated push-mode +# output and its separate re-convert pass are both avoided. +# content-incremental a qcow2 overlay backed by the NBD export, then a SAFE rebase +# onto the parent backup. That merges exactly the clusters which +# differ between export and parent into the overlay, yielding a +# delta qcow2 backed by the parent -- the same chain shape the +# push-mode incremental produces, so restore/delete are unchanged. +# +# 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. +# +# Assumes mount_operation() already ran, so $dest and $mount_point are set. +# --------------------------------------------------------------------------- +backup_running_vm_pull() { + local effective_mode="$1" + + # Scratch area for the NBD socket and libvirt's fleecing images. Never on the NAS: + # copy-before-write runs inline with guest writes, so it wants fast local storage with + # room for the write churn during the backup. 711 lets the qemu process traverse in to + # the files libvirt creates as root. + local scratch_dir + scratch_dir=$(mktemp -d "$SCRATCH_PARENT/csbackup-scratch.XXXXXX") || { + echo "Failed to create scratch directory under $SCRATCH_PARENT"; cleanup; exit 1; + } + chmod 711 "$scratch_dir" + local nbd_sock="$scratch_dir/nbd.sock" + local backupxml="$scratch_dir/backup.xml" + + # The kernel caps UNIX socket paths at 108 bytes. A long scratch dir would otherwise + # fail deep inside qemu-img with an opaque error, so reject it up front. + if [[ ${#nbd_sock} -ge 108 ]]; then + echo "Pull-mode NBD socket path is too long (${#nbd_sock} bytes, limit 108): $nbd_sock" + echo "Set a shorter scratch dir via -S (agent.properties nas.backup.pull.scratch.dir)" + rm -rf "$scratch_dir" + cleanup + exit 1 + fi + + # Snapshot the disk list once so the backup XML and the read-back loop stay in sync. + local -a disks + mapfile -t disks < <( + virsh -c qemu:///system domblklist "$VM" --details 2>/dev/null | awk '$2=="disk"{print $3, $4}' + ) + if [[ ${#disks[@]} -eq 0 ]]; then + echo "No disks found for vm $VM" + rm -rf "$scratch_dir" + cleanup + exit 1 + fi + + # Resolve each disk's parent backup up front. PARENT_PATHS is comma separated, one entry + # per volume in the same order as the disk list. Anything missing degrades this run to a + # full backup rather than failing it, mirroring the push-mode fallback. + local -a parent_paths_arr=() + if [[ "$effective_mode" == "content-incremental" ]]; then + if [[ -z "$PARENT_PATHS" ]]; then + log -e "content-incremental: no parent paths supplied -- falling back to full" + echo "INCREMENTAL_FALLBACK=true" + effective_mode="content-full" + else + IFS=',' read -ra parent_paths_arr <<< "$PARENT_PATHS" + if [[ ${#parent_paths_arr[@]} -lt ${#disks[@]} ]]; then + log -e "content-incremental: parent path list shorter than disk list -- falling back to full" + echo "INCREMENTAL_FALLBACK=true" + effective_mode="content-full" + else + local pp + for pp in "${parent_paths_arr[@]}"; do + if [[ ! -f "$mount_point/$pp" ]]; then + log -e "content-incremental: parent $mount_point/$pp missing on NAS -- falling back to full" + echo "INCREMENTAL_FALLBACK=true" + effective_mode="content-full" + break + fi + done + fi + fi + fi + + # Pull-mode backup XML: one NBD export and one fleecing scratch image per disk. + { + echo "" + echo "" + echo "" + local entry + for entry in "${disks[@]}"; do + echo "" + done + echo "" + } > "$backupxml" + + local thaw=0 + local freeze_ok=0 + if [[ ${QUIESCE} == "true" ]]; then + # Always thaw after a freeze attempt, see backup_running_vm(). + thaw=1 + if freeze_err=$(qemu_agent_command '{"execute":"guest-fsfreeze-freeze"}' 2>&1 > /dev/null); then + freeze_ok=1 + else + log -e "Failed to freeze the filesystem for vm $VM, continuing without quiescing: $freeze_err" + fi + fi + + # backup-begin establishes the point-in-time and starts the NBD server. + local backup_begin=0 + local backup_out + if backup_out=$(virsh -c qemu:///system backup-begin --domain "$VM" --backupxml "$backupxml" 2>&1); then + backup_begin=1 + fi + + if [[ $thaw -eq 1 ]] && ! thaw_guest "$freeze_ok"; then + if [[ $backup_begin -eq 1 ]]; then + virsh -c qemu:///system domjobabort "$VM" > /dev/null 2>&1 || true + fi + rm -rf "$scratch_dir" + cleanup + exit 1 + fi + + if [[ $backup_begin -ne 1 ]]; then + echo "Failed to start pull-mode backup for $VM: $backup_out" + rm -rf "$scratch_dir" + cleanup + exit 1 + fi + + # Backup domain information + virsh -c qemu:///system dumpxml $VM > $dest/domain-config.xml 2>/dev/null + virsh -c qemu:///system dominfo $VM > $dest/dominfo.xml 2>/dev/null + virsh -c qemu:///system domiflist $VM > $dest/domiflist.xml 2>/dev/null + virsh -c qemu:///system domblklist $VM > $dest/domblklist.xml 2>/dev/null + + local name="root" + local disk_idx=0 + local entry disk fullpath volUuid output export_uri parent_abs parent_rel + for entry in "${disks[@]}"; do + disk="${entry%% *}" + fullpath="${entry#* }" + if [[ "$fullpath" == /dev/drbd/by-res/* ]]; then + volUuid=$(get_linstor_uuid_from_path "$fullpath") + elif [[ "$fullpath" == /dev/drbd[0-9]* ]]; then + if ! volUuid=$(get_linstor_uuid_from_device "$fullpath"); then + echo "Failed to resolve LINSTOR volume UUID for $fullpath" + virsh -c qemu:///system domjobabort "$VM" > /dev/null 2>&1 || true + rm -rf "$scratch_dir" + cleanup + exit 1 + fi + else + volUuid="${fullpath##*/}" + fi + output="$dest/$name.$volUuid.qcow2" + # The export name defaults to the disk target (e.g. vda) and presents raw bytes. + export_uri="nbd+unix:///$disk?socket=$nbd_sock" + + if [[ "$effective_mode" == "content-incremental" ]]; then + parent_abs="$mount_point/${parent_paths_arr[$disk_idx]}" + parent_rel=$(realpath --relative-to="$dest" "$parent_abs") + # Overlay over the export, safe-rebase onto the parent to merge the differing + # clusters in, then a metadata-only rebase to store the parent as a relative path + # so the chain survives being mounted at a different mount point. + if ! qemu-img create -f qcow2 -b "$export_uri" -F raw "$output" >> "$logFile" 2> >(cat >&2) \ + || ! qemu-img rebase -b "$parent_abs" -F qcow2 "$output" >> "$logFile" 2> >(cat >&2) \ + || ! qemu-img rebase -u -b "$parent_rel" -F qcow2 "$output" >> "$logFile" 2> >(cat >&2); then + echo "qemu-img incremental delta failed for $disk -> $output" + rm -f "$output" + virsh -c qemu:///system domjobabort "$VM" > /dev/null 2>&1 || true + rm -rf "$scratch_dir" + cleanup + exit 1 + fi + else + # Zero detection while streaming gives a sparse qcow2 in a single pass. + if ! qemu-img convert -f raw -O qcow2 "$export_uri" "$output" >> "$logFile" 2> >(cat >&2); then + echo "qemu-img convert failed for pull export $disk -> $output" + virsh -c qemu:///system domjobabort "$VM" > /dev/null 2>&1 || true + rm -rf "$scratch_dir" + cleanup + exit 1 + fi + fi + name="datadisk" + disk_idx=$((disk_idx + 1)) + done + + # End the pull job. Aborting the active job is how a pull backup finishes (there is no + # backup-end command): libvirt tears down the NBD server and deletes the scratch files. + # The data already pulled is a complete backup. + if ! virsh -c qemu:///system domjobabort "$VM" > /dev/null 2>&1; then + log -e "warning: failed to end (domjobabort) backup job for $VM" + fi + rm -rf "$scratch_dir" + sync + + # Statistics -- the size on the last stdout line is parsed by the Java wrapper. + du -sb $dest | cut -f1 + + umount $mount_point + rmdir $mount_point +} + backup_running_vm() { mount_operation mkdir -p "$dest" || { echo "Failed to create backup directory $dest"; exit 1; } @@ -197,6 +413,12 @@ backup_running_vm() { incremental|full) make_checkpoint=1 ;; + content-full|content-incremental) + # Raw block-device storage (LINSTOR/DRBD) cannot carry persistent dirty bitmaps, + # so checkpoints are unavailable. Use the content-based pull-mode path instead. + backup_running_vm_pull "$effective_mode" + return + ;; legacy-full) make_checkpoint=0 ;; @@ -624,6 +846,13 @@ function usage { echo " Requires --bitmap-parent, --bitmap-new, and --parent-paths (comma-separated list, one" echo " parent qcow2 path per disk: root..qcow2, datadisk..qcow2, … same order" echo " as -d|--disks)." + echo " -M|--mode content-full Full backup via libvirt pull mode; qemu-img convert writes a sparse qcow2." + echo " -M|--mode content-incremental Delta backup via libvirt pull mode: a qcow2 overlay on the NBD export is" + echo " safe-rebased onto the parent, keeping only the clusters that differ." + echo " Requires --parent-paths. Used for raw block-device storage (LINSTOR/DRBD)," + echo " which cannot carry the persistent dirty bitmaps checkpoints need." + echo " -S|--scratch-dir Host-local parent dir for pull-mode scratch (NBD socket + fleecing images)." + echo " Defaults to /var/tmp. Must not be on the NAS share." echo " Without -M, behaves as legacy full-only backup with no checkpoint creation." echo "" exit 1 @@ -696,6 +925,11 @@ while [[ $# -gt 0 ]]; do shift shift ;; + -S|--scratch-dir) + SCRATCH_PARENT="$2" + shift + shift + ;; -h|--help) usage shift From e8ff16066af8acca39e11d296bd6049e36742848 Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Tue, 18 Aug 2026 16:01:15 +0200 Subject: [PATCH 2/7] nasbackup: route LINSTOR VMs to the content-based incremental chain 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. --- .../agent/properties/AgentProperties.java | 17 ++ .../cloudstack/backup/NASBackupChainKeys.java | 14 ++ .../cloudstack/backup/NASBackupProvider.java | 147 ++++++++++++-- .../backup/NASBackupProviderTest.java | 191 ++++++++++++++++++ .../LibvirtTakeBackupCommandWrapper.java | 26 +++ 5 files changed, 383 insertions(+), 12 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java index 1cb9232eec14..984924371b38 100644 --- a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java +++ b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java @@ -636,6 +636,23 @@ public class AgentProperties{ */ public static final Property KVM_SCRIPTS_DIR = new Property<>("kvm.scripts.dir", "scripts/vm/hypervisor/kvm"); + /** + * Host-local parent directory for the NAS backup pull-mode scratch area: the NBD socket and + * the libvirt-managed fleecing (copy-before-write) images used while backing up a running VM + * on raw block-device storage such as LINSTOR/DRBD. + *

+ * Must be on a real on-disk filesystem with room for the guest's write churn during a backup, + * and must NOT be on the NAS share: copy-before-write runs inline with guest writes, so a slow + * or remote location stalls the VM. Keep the path short — the resulting socket path is subject + * to the kernel's 108-byte UNIX socket limit. + *

+ * Data type: String.
+ * Default value: /var/tmp + * + * @since 4.22.0 + */ + public static final Property NAS_BACKUP_PULL_SCRATCH_DIR = new Property<>("nas.backup.pull.scratch.dir", "/var/tmp"); + /** * Specifies start MAC address for private IP range.
* Data type: String.
diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java index 511f0ccb7114..55a09b639ff1 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java @@ -48,6 +48,20 @@ public final class NASBackupChainKeys { // "legacy-full" mode token (which sets make_checkpoint=0). public static final String TYPE_LEGACY_FULL = "legacy-full"; + /** + * Backup taken through the content-based (libvirt pull mode) path, used for raw + * block-device storage such as LINSTOR/DRBD which cannot carry QEMU persistent dirty + * bitmaps. A full standalone qcow2, written sparse in a single pass. + */ + public static final String TYPE_CONTENT_FULL = "content-full"; + + /** + * Delta backup produced by content comparison rather than dirty bitmaps: a qcow2 overlay + * on the disk's NBD export, safe-rebased onto the parent so it holds only the clusters + * that differ. Same chain shape as {@link #TYPE_INCREMENTAL}. + */ + public static final String TYPE_CONTENT_INCREMENTAL = "content-incremental"; + /** * VM-scoped detail (stored in {@code vm_instance_details}) holding the QEMU dirty-bitmap * name that currently exists on the running VM and is therefore the only valid parent diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java index a72920fbae88..7a15d1d18d43 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java @@ -237,15 +237,24 @@ static final class ChainDecision { final List parentPaths; final String chainId; // chain identifier this backup belongs to final int chainPosition; // 0 for full, N for the Nth incremental in the chain + // Content-based chains have no bitmap to look the parent up by, so the parent backup's + // uuid is resolved when the decision is made. null for bitmap-based decisions. + final String parentBackupUuid; private ChainDecision(String mode, String bitmapNew, String bitmapParent, List parentPaths, String chainId, int chainPosition) { + this(mode, bitmapNew, bitmapParent, parentPaths, chainId, chainPosition, null); + } + + private ChainDecision(String mode, String bitmapNew, String bitmapParent, List parentPaths, + String chainId, int chainPosition, String parentBackupUuid) { this.mode = mode; this.bitmapNew = bitmapNew; this.bitmapParent = bitmapParent; this.parentPaths = parentPaths; this.chainId = chainId; this.chainPosition = chainPosition; + this.parentBackupUuid = parentBackupUuid; } static ChainDecision fullStart(String bitmapName) { @@ -268,8 +277,30 @@ static ChainDecision incremental(String bitmapNew, String bitmapParent, List parentPaths, String chainId, + int chainPosition, String parentBackupUuid) { + return new ChainDecision(NASBackupChainKeys.TYPE_CONTENT_INCREMENTAL, null, null, + parentPaths, chainId, chainPosition, parentBackupUuid); + } + boolean isIncremental() { - return NASBackupChainKeys.TYPE_INCREMENTAL.equals(mode); + return NASBackupChainKeys.TYPE_INCREMENTAL.equals(mode) + || NASBackupChainKeys.TYPE_CONTENT_INCREMENTAL.equals(mode); + } + + /** True for the pull-mode content-diff path (no dirty bitmaps involved). */ + boolean isContentBased() { + return NASBackupChainKeys.TYPE_CONTENT_FULL.equals(mode) + || NASBackupChainKeys.TYPE_CONTENT_INCREMENTAL.equals(mode); } boolean isLegacyFull() { @@ -294,14 +325,25 @@ protected ChainDecision decideChain(VirtualMachine vm) { // behaves exactly like the pre-incremental full-only path: no bitmap is generated and no // chain/checkpoint metadata is created, sent to the agent, or persisted (legacy-full). Boolean incrementalEnabled = NASBackupIncrementalEnabled.valueIn(vm.getDataCenterId()); - if (incrementalEnabled == null || !incrementalEnabled) { + final boolean incrementalOn = incrementalEnabled != null && incrementalEnabled; + + // Raw block-device storage (LINSTOR/DRBD) cannot carry QEMU persistent dirty bitmaps, so + // libvirt checkpoints — and with them the push-mode incremental path — are unavailable. + // Those VMs take the content-based pull-mode path instead. It is chosen regardless of the + // master switch, because even a full backup benefits: pull mode writes a sparse qcow2 in a + // single pass rather than a fully allocated one needing a second re-convert pass. + if (allVolumesOnContentDiffCapableStorage(vm)) { + return decideContentChain(vm, incrementalOn); + } + + if (!incrementalOn) { return ChainDecision.legacyFull(); } // Incremental backups rely on QEMU dirty bitmaps / libvirt checkpoints, which only exist - // on file-based qcow2 storage. Storage such as Ceph-RBD and Linstor cannot carry per-disk - // checkpoints, so a VM with any volume on such a pool must stay on the full-only (legacy) - // path — otherwise an incremental attempt would fail or regress those storages. + // on file-based qcow2 storage. Storage such as Ceph-RBD cannot carry per-disk checkpoints + // and has no content-diff path either, so such a VM stays on the full-only (legacy) path — + // otherwise an incremental attempt would fail or regress those storages. if (!allVolumesOnCheckpointCapableStorage(vm)) { return ChainDecision.legacyFull(); } @@ -365,6 +407,78 @@ protected ChainDecision decideChain(VirtualMachine vm) { parentChainId, parentChainPosition + 1); } + /** + * Chain decision for content-based (pull mode) storage such as LINSTOR/DRBD. + * + *

Unlike the bitmap path there is no host-side state to anchor on. The delta is derived by + * comparing the disk's point-in-time NBD export against the parent backup, so the chain cannot + * be invalidated by a VM restart, live migration or restore — the comparison is stateless. The + * parent is therefore simply the most recent BackedUp backup of the same chain, and the agent + * independently verifies the parent files still exist on the NAS, degrading to a full if they + * do not (INCREMENTAL_FALLBACK).

+ */ + protected ChainDecision decideContentChain(VirtualMachine vm, boolean incrementalEnabled) { + if (!incrementalEnabled) { + return ChainDecision.contentFull(); + } + + // Stopped VMs are backed up straight from the disk with qemu-img convert; that path has no + // point-in-time export to diff against a parent, so it is always a full. + if (VirtualMachine.State.Stopped.equals(vm.getState())) { + return ChainDecision.contentFull(); + } + + Integer fullEvery = NASBackupFullEvery.valueIn(vm.getDataCenterId()); + if (fullEvery == null || fullEvery <= 1) { + return ChainDecision.contentFull(); + } + + Backup parent = findLatestBackedUpBackup(vm.getId()); + if (parent == null) { + return ChainDecision.contentFull(); + } + + String parentChainId = readDetail(parent, NASBackupChainKeys.CHAIN_ID); + int parentChainPosition = chainPosition(parent); + if (parentChainId == null || parentChainPosition == Integer.MAX_VALUE) { + return ChainDecision.contentFull(); + } + + // Force a fresh full when the chain has reached the configured length. + if (parentChainPosition + 1 >= fullEvery) { + return ChainDecision.contentFull(); + } + + List parentPaths = composeParentBackupPaths(parent, vm.getId()); + if (parentPaths == null) { + LOG.debug("VM {} parent backup {} volume layout no longer matches current VM — forcing full", + vm.getInstanceName(), parent.getUuid()); + return ChainDecision.contentFull(); + } + return ChainDecision.contentIncremental(parentPaths, parentChainId, parentChainPosition + 1, + parent.getUuid()); + } + + /** + * True when EVERY volume of the VM sits on storage whose backups must be derived by content + * comparison instead of dirty bitmaps — currently LINSTOR, whose volumes are raw DRBD block + * devices and so cannot persist a bitmap (persistence is a qcow2-only feature). A VM with a + * mix of such volumes and others is not eligible, since one backup run uses a single mode. + */ + protected boolean allVolumesOnContentDiffCapableStorage(VirtualMachine vm) { + List volumes = volumeDao.findByInstance(vm.getId()); + if (CollectionUtils.isEmpty(volumes)) { + return false; + } + for (VolumeVO volume : volumes) { + StoragePoolVO pool = primaryDataStoreDao.findById(volume.getPoolId()); + if (pool == null || !Storage.StoragePoolType.Linstor.equals(pool.getPoolType())) { + return false; + } + } + return true; + } + /** * Incremental backups require QEMU dirty bitmaps / libvirt checkpoints, which are only * possible on file-based qcow2 storage. Returns {@code true} only when EVERY volume of the @@ -502,7 +616,10 @@ private void persistChainMetadata(Backup backup, ChainDecision decision, String // source of truth. Not duplicated into backup_details. if (decision.isIncremental()) { // Resolve the parent backup's UUID so restore can walk the chain by id, not by path. - String parentUuid = lookupParentBackupUuid(backup.getVmId(), decision.bitmapParent); + // Content-based decisions carry the parent uuid directly (no bitmap to look it up by). + String parentUuid = decision.parentBackupUuid != null + ? decision.parentBackupUuid + : lookupParentBackupUuid(backup.getVmId(), decision.bitmapParent); if (parentUuid != null) { backupDetailsDao.persist(new BackupDetailVO(backup.getId(), NASBackupChainKeys.PARENT_BACKUP_ID, parentUuid, true)); } @@ -628,7 +745,9 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce // backup as a full and start a new chain. ChainDecision effective = decision; if (answer.getIncrementalFallback()) { - effective = ChainDecision.fullStart(decision.bitmapNew); + effective = decision.isContentBased() + ? ChainDecision.contentFull() + : ChainDecision.fullStart(decision.bitmapNew); backupVO.setType("FULL"); } List volumes = new ArrayList<>(volumeDao.findByInstance(vm.getId())); @@ -642,11 +761,15 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce // created — the only valid parent for the next incremental (see decideChain). // If the agent reports no bitmap (bitmapCreated=null), clear any stale detail // so the next backup starts a fresh full. - String confirmedBitmap = answer.getBitmapCreated(); - if (confirmedBitmap != null) { - upsertVmActiveCheckpoint(vm.getId(), confirmedBitmap); - } else { - clearVmActiveCheckpoint(vm.getId()); + // The content-based path creates no bitmaps, so there is no active + // checkpoint to track — its chain is anchored purely on backup history. + if (!decision.isContentBased()) { + String confirmedBitmap = answer.getBitmapCreated(); + if (confirmedBitmap != null) { + upsertVmActiveCheckpoint(vm.getId(), confirmedBitmap); + } else { + clearVmActiveCheckpoint(vm.getId()); + } } } return new Pair<>(true, backupVO); diff --git a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java index 787d3ff94567..c985f9ca94db 100644 --- a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java +++ b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java @@ -49,6 +49,7 @@ import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; +import java.util.Date; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; @@ -1099,4 +1100,194 @@ public void sweepContinuesPastFailedTombstoneDelete() Mockito.verify(backupDao, Mockito.never()).remove(51L); Mockito.verify(backupDao).remove(50L); } + + // -- content-based (pull mode) chain decisions for raw block storage ----------------- + + /** + * A LINSTOR-backed VM must take the content-based path even when the incremental master + * switch is off: pull mode still writes a sparse qcow2 in one pass, which is strictly better + * than the fully allocated push output. It must NOT fall back to legacy-full. + */ + @Test + public void decideChainReturnsContentFullForLinstorWhenIncrementalDisabled() { + Long vmId = 70L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.lenient().when(vm.getDataCenterId()).thenReturn(1L); + stubAllVolumesOnLinstor(vmId, 1); + + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupIncrementalEnabled", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Boolean.class, + "nas.backup.incremental.enabled", "false", + "test override — disabled", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertEquals(NASBackupChainKeys.TYPE_CONTENT_FULL, decision.mode); + Assert.assertTrue(decision.isContentBased()); + Assert.assertFalse(decision.isIncremental()); + Assert.assertNull("content path uses no bitmaps", decision.bitmapNew); + Assert.assertNotNull("content-full still anchors a chain", decision.chainId); + } + + /** + * Stopped VMs are imaged straight from the disk by qemu-img, with no point-in-time export to + * diff against a parent, so a LINSTOR VM that is stopped must take a content-full. + */ + @Test + public void decideChainReturnsContentFullForStoppedLinstorVm() { + Long vmId = 71L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Stopped); + stubAllVolumesOnLinstor(vmId, 1); + enableIncrementals(); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertEquals(NASBackupChainKeys.TYPE_CONTENT_FULL, decision.mode); + } + + /** + * Running LINSTOR VM with a healthy parent inside the cadence => content-incremental, carrying + * the parent's chain id, the next position, the parent uuid (resolved directly since there is + * no bitmap to look it up by) and the per-volume parent paths the script rebases onto. + */ + @Test + public void decideChainReturnsContentIncrementalForLinstorWithValidParent() { + Long vmId = 72L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Running); + stubAllVolumesOnLinstor(vmId, 1); + enableIncrementals(); + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupFullEvery", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Integer.class, + "nas.backup.full.every", "3", "test override", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + + Backup parent = mock(Backup.class); + Mockito.when(parent.getId()).thenReturn(900L); + Mockito.when(parent.getStatus()).thenReturn(Backup.Status.BackedUp); + Mockito.lenient().when(parent.getDate()).thenReturn(new Date()); + Mockito.when(parent.getUuid()).thenReturn("parent-uuid"); + Mockito.when(parent.getExternalId()).thenReturn("i-2-3-VM/2026.01.01.00.00.00"); + Backup.VolumeInfo pv = mock(Backup.VolumeInfo.class); + Mockito.when(pv.getPath()).thenReturn("volpath1"); + Mockito.>when(parent.getBackedUpVolumes()).thenReturn(List.of(pv)); + Mockito.when(backupDao.listByVmId(null, vmId)).thenReturn(List.of(parent)); + + BackupDetailVO chainId = mock(BackupDetailVO.class); + Mockito.when(chainId.getValue()).thenReturn("chain-1"); + BackupDetailVO chainPos = mock(BackupDetailVO.class); + Mockito.when(chainPos.getValue()).thenReturn("0"); + Mockito.when(backupDetailsDao.findDetail(900L, NASBackupChainKeys.CHAIN_ID)).thenReturn(chainId); + Mockito.when(backupDetailsDao.findDetail(900L, NASBackupChainKeys.CHAIN_POSITION)).thenReturn(chainPos); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertEquals(NASBackupChainKeys.TYPE_CONTENT_INCREMENTAL, decision.mode); + Assert.assertTrue(decision.isIncremental()); + Assert.assertTrue(decision.isContentBased()); + Assert.assertNull("content-incremental carries no bitmap", decision.bitmapNew); + Assert.assertNull(decision.bitmapParent); + Assert.assertEquals("chain-1", decision.chainId); + Assert.assertEquals(1, decision.chainPosition); + Assert.assertEquals("parent-uuid", decision.parentBackupUuid); + Assert.assertEquals(List.of("i-2-3-VM/2026.01.01.00.00.00/root.volpath1.qcow2"), + decision.parentPaths); + } + + /** + * Once the chain reaches nas.backup.full.every, the next LINSTOR backup must anchor a new + * chain with a content-full rather than extending the existing one. + */ + @Test + public void decideChainReturnsContentFullForLinstorAtChainEnd() { + Long vmId = 73L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getDataCenterId()).thenReturn(1L); + Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Running); + stubAllVolumesOnLinstor(vmId, 1); + enableIncrementals(); + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupFullEvery", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Integer.class, + "nas.backup.full.every", "2", "test override", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + + Backup parent = mock(Backup.class); + Mockito.when(parent.getId()).thenReturn(901L); + Mockito.when(parent.getStatus()).thenReturn(Backup.Status.BackedUp); + Mockito.lenient().when(parent.getDate()).thenReturn(new Date()); + Mockito.when(backupDao.listByVmId(null, vmId)).thenReturn(List.of(parent)); + BackupDetailVO chainId = mock(BackupDetailVO.class); + Mockito.when(chainId.getValue()).thenReturn("chain-9"); + BackupDetailVO chainPos = mock(BackupDetailVO.class); + Mockito.when(chainPos.getValue()).thenReturn("1"); // 1 + 1 >= 2 => new full + Mockito.when(backupDetailsDao.findDetail(901L, NASBackupChainKeys.CHAIN_ID)).thenReturn(chainId); + Mockito.when(backupDetailsDao.findDetail(901L, NASBackupChainKeys.CHAIN_POSITION)).thenReturn(chainPos); + + NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm); + Assert.assertEquals(NASBackupChainKeys.TYPE_CONTENT_FULL, decision.mode); + } + + /** + * A VM straddling LINSTOR and file-based storage is not content-diff capable: one backup run + * uses a single mode, so it must not be routed down the content path. + */ + @Test + public void allVolumesOnContentDiffCapableStorageFalseForMixedStorage() { + Long vmId = 74L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + + VolumeVO linVol = mock(VolumeVO.class); + Mockito.when(linVol.getPoolId()).thenReturn(1L); + VolumeVO nfsVol = mock(VolumeVO.class); + Mockito.when(nfsVol.getPoolId()).thenReturn(2L); + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(linVol, nfsVol)); + + StoragePoolVO lin = mock(StoragePoolVO.class); + Mockito.when(lin.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + StoragePoolVO nfs = mock(StoragePoolVO.class); + Mockito.when(nfs.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + Mockito.when(storagePoolDao.findById(1L)).thenReturn(lin); + Mockito.when(storagePoolDao.findById(2L)).thenReturn(nfs); + + Assert.assertFalse(nasBackupProvider.allVolumesOnContentDiffCapableStorage(vm)); + } + + /** A VM with no volumes is not content-diff capable (safe default). */ + @Test + public void allVolumesOnContentDiffCapableStorageFalseForNoVolumes() { + Long vmId = 75L; + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of()); + Assert.assertFalse(nasBackupProvider.allVolumesOnContentDiffCapableStorage(vm)); + } + + private void enableIncrementals() { + ReflectionTestUtils.setField(nasBackupProvider, "NASBackupIncrementalEnabled", + new org.apache.cloudstack.framework.config.ConfigKey<>("Advanced", Boolean.class, + "nas.backup.incremental.enabled", "true", + "test override — enabled", true, + org.apache.cloudstack.framework.config.ConfigKey.Scope.Zone)); + } + + /** Point every volume of {@code vmId} at a LINSTOR pool. */ + private void stubAllVolumesOnLinstor(Long vmId, int count) { + List vols = new java.util.ArrayList<>(); + StoragePoolVO lin = mock(StoragePoolVO.class); + Mockito.lenient().when(lin.getPoolType()).thenReturn(Storage.StoragePoolType.Linstor); + for (int i = 0; i < count; i++) { + VolumeVO v = mock(VolumeVO.class); + long poolId = 100L + i; + Mockito.when(v.getPoolId()).thenReturn(poolId); + Mockito.when(storagePoolDao.findById(poolId)).thenReturn(lin); + vols.add(v); + } + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(vols); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java index b7e609587650..66f46fa655e3 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java @@ -20,6 +20,8 @@ package com.cloud.hypervisor.kvm.resource.wrapper; import com.cloud.agent.api.Answer; +import com.cloud.agent.properties.AgentProperties; +import com.cloud.agent.properties.AgentPropertiesFileHandler; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; import com.cloud.hypervisor.kvm.storage.KVMStoragePool; @@ -52,6 +54,13 @@ public class LibvirtTakeBackupCommandWrapper extends CommandWrapper runBackupScript(LibvirtComputingResource libvirtCo argv.add("--parent-paths"); argv.add(String.join(",", parentPaths)); } + // Host-local scratch dir for the pull-mode NBD socket and fleecing images. Sent for every + // mode: harmless for the push-mode paths, which ignore it. + String scratchDir = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.NAS_BACKUP_PULL_SCRATCH_DIR); + if (StringUtils.isNotBlank(scratchDir)) { + argv.add("-S"); + argv.add(scratchDir); + } List commands = new ArrayList<>(); commands.add(argv.toArray(new String[0])); @@ -212,6 +228,16 @@ private String validateBackupArgs(TakeBackupCommand command) { if (MODE_LEGACY_FULL.equals(mode)) { return null; // feature-off full backup, no bitmap or chain args expected } + if (MODE_CONTENT_FULL.equals(mode)) { + return null; // pull-mode full — no bitmap, no parent + } + if (MODE_CONTENT_INCREMENTAL.equals(mode)) { + // No bitmap is involved: the delta comes from comparing the disk against the parent. + if (CollectionUtils.isEmpty(command.getParentPaths())) { + return "content-incremental mode requires parentPaths"; + } + return null; + } return "Unknown backup mode: " + mode; } From 217102318ad4e72346a2a80ba6f78fe5d61eae46 Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Tue, 13 Oct 2026 14:59:57 +0200 Subject: [PATCH 3/7] nasbackup: always end the pull-mode backup job on a failed backup A pull-mode backup job never finishes on its own; nasbackup.sh ends it with domjobabort once the data is read. Any exit the script did not plan for -- an unexpected error under "set -e", SIGINT/SIGTERM/SIGHUP -- left the job running, keeping the NBD server and the growing fleecing images alive and blocking every later backup or migration of the VM. Arm an EXIT trap once the scratch dir exists: it aborts the job (through #14305's BACKUP_JOB_ACTIVE/abort_backup_job), removes the scratch dir and runs cleanup(). This replaces the per-error-path abort/cleanup copies. The agent kills a timed-out script with SIGKILL, which no trap can catch, so LibvirtTakeBackupCommandWrapper aborts the job itself after a pull-mode timeout. --- .../LibvirtTakeBackupCommandWrapper.java | 22 ++++++++ .../LibvirtTakeBackupCommandWrapperTest.java | 41 ++++++++++++++ scripts/vm/hypervisor/kvm/nasbackup.sh | 54 +++++++++++-------- 3 files changed, 94 insertions(+), 23 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java index 66f46fa655e3..dd0eac43092a 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java @@ -102,6 +102,9 @@ public Answer execute(TakeBackupCommand command, LibvirtComputingResource libvir if (result.first() != 0) { logger.debug("Failed to take VM backup: " + result.second()); + if (Script.ERR_TIMEOUT.equals(result.second()) && isPullMode(command.getMode())) { + abortPullBackupJob(vmName); + } BackupAnswer answer = new BackupAnswer(command, false, StringUtils.trimToEmpty(result.second())); if (EXIT_CLEANUP_FAILED.equals(result.first())) { logger.debug("Backup cleanup failed"); @@ -127,6 +130,25 @@ public Answer execute(TakeBackupCommand command, LibvirtComputingResource libvir return answer; } + private boolean isPullMode(String mode) { + return MODE_CONTENT_FULL.equals(mode) || MODE_CONTENT_INCREMENTAL.equals(mode); + } + + /** + * A pull-mode backup job never ends on its own: nasbackup.sh ends it once the data is read, + * and its EXIT trap does so on failure. A timed-out script is killed with SIGKILL, which + * no trap can catch, so end the job here. Otherwise it keeps the NBD server and the growing + * fleecing images alive and blocks every later backup or migration of the VM. + */ + private void abortPullBackupJob(String vmName) { + logger.warn("Backup of VM {} timed out, aborting its pull-mode backup job", vmName); + int exitValue = Script.executeCommandForExitValue(Script.getExecutableAbsolutePath("virsh"), + "-c", "qemu:///system", "domjobabort", vmName); + if (exitValue != 0) { + logger.warn("Failed to abort the backup job of VM {} (exit code {})", vmName, exitValue); + } + } + /** Remove nasbackup.sh's stdout signalling marker lines so they don't pollute size parsing. */ private String stripMarkerLines(String stdout) { if (StringUtils.isBlank(stdout)) { diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java index 7ef64fd53b50..2fb195170c60 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java @@ -18,7 +18,9 @@ import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.when; import java.util.Arrays; @@ -99,4 +101,43 @@ public void executeOmitsQuiesceTimeoutWhenNotPositive() { Assert.assertFalse(runAndCaptureArgv().contains("--quiesce-timeout")); } + + private void runFailing(Pair result, boolean expectJobAbort) { + try (MockedStatic