abh1sar commented on code in PR #14194:
URL: https://github.com/apache/cloudstack/pull/14194#discussion_r4250058877
##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -134,6 +137,217 @@ get_linstor_uuid_from_device() {
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 "<domainbackup mode='pull'>"
+ echo "<server transport='unix' socket='$nbd_sock'/>"
+ echo "<disks>"
+ local entry
+ for entry in "${disks[@]}"; do
+ echo "<disk name='${entry%% *}' backup='yes' type='file'
backupmode='full'><scratch file='$scratch_dir/${entry%% *}.scratch'/></disk>"
+ done
+ echo "</disks></domainbackup>"
+ } > "$backupxml"
+
+ local thaw=0
+ if [[ ${QUIESCE} == "true" ]]; then
+ if virsh -c qemu:///system qemu-agent-command "$VM"
'{"execute":"guest-fsfreeze-freeze"}' > /dev/null 2>/dev/null; then
+ thaw=1
+ 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
+
Review Comment:
The backup job will be left forever running if there is an interrupt or
error before calling domjobabort.
Consider adding a trap and cleanup.
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -255,8 +264,30 @@ static ChainDecision incremental(String bitmapNew, String
bitmapParent, List<Str
parentPaths, chainId, chainPosition);
}
+ /**
+ * Full backup on the content-based (pull mode) path: no bitmap, no
checkpoint, but it
+ * does anchor a chain so later content-incrementals can hang off it.
+ */
+ static ChainDecision contentFull() {
+ return new ChainDecision(NASBackupChainKeys.TYPE_CONTENT_FULL,
null, null, null,
Review Comment:
Instead of overloading the `mode` for each ChainDecision, I suggest adding a
new `method` field to it.
The `method` can be qcow2 / content / none / rbd_diff(ceph - can be added
later).
The mode will still say full and incremental for each method.
#14386 also adds rbd-full and rbd-incremental. I think a new method will be
a better choice for both PRs.
cc @weizhouapache
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]