abh1sar commented on code in PR #13074:
URL: https://github.com/apache/cloudstack/pull/13074#discussion_r3528879718
##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -191,18 +357,69 @@ backup_running_vm() {
virsh -c qemu:///system domblklist "$VM" --details 2>/dev/null | awk
'$2=="disk"{print $3, $4}'
)
- rm -f $dest/backup.xml
+ rm -f $dest/backup.xml $dest/checkpoint.xml
sync
+ # Free the parent bitmap now that the incremental has been written and
rebased. The parent's
+ # delta is fully captured in this backup and BITMAP_NEW already tracks
changes going forward, so
+ # the parent bitmap is dead weight — left in place it accumulates metadata
and IO cost over a
+ # long chain. Remove it directly per-disk with block-dirty-bitmap-remove (a
clean free) rather
+ # than checkpoint-delete, which would MERGE the parent's dirty bits into
BITMAP_NEW and make the
+ # next incremental needlessly re-copy already-backed-up regions.
Best-effort: a failure here does
+ # not fail the backup (the data is already safe) — the bitmap is simply
reclaimed on a later run.
+ if [[ "$effective_mode" == "incremental" && -n "$BITMAP_PARENT" ]]; then
+ removed_from=0
+ parent_disk_count=0
+ while read -r node; do
+ [[ -z "$node" ]] && continue
+ parent_disk_count=$((parent_disk_count + 1))
+ if virsh -c qemu:///system qemu-monitor-command "$VM" \
+
"{\"execute\":\"block-dirty-bitmap-remove\",\"arguments\":{\"node\":\"$node\",\"name\":\"$BITMAP_PARENT\"}}"
\
+ > /dev/null 2>>"$logFile"; then
+ removed_from=$((removed_from + 1))
+ else
+ log -e "cleanup: failed to remove parent bitmap $BITMAP_PARENT on node
$node (non-fatal)"
+ fi
+ done < <(
+ virsh -c qemu:///system qemu-monitor-command "$VM"
'{"execute":"query-block"}' 2>/dev/null | python3 -c '
+import sys, json
+target = sys.argv[1]
+try:
+ data = json.load(sys.stdin)
+except Exception:
+ sys.exit(0)
+seen = set()
+for dev in data.get("return", []) or []:
+ inserted = dev.get("inserted") or {}
+ node = inserted.get("node-name")
+ if not node or node in seen:
+ continue
+ if any((b or {}).get("name") == target for b in
(inserted.get("dirty-bitmaps") or [])):
+ seen.add(node)
+ print(node)
+' "$BITMAP_PARENT" 2>/dev/null || true
+ )
+ if [[ "$parent_disk_count" -gt 0 && "$removed_from" -eq
"$parent_disk_count" ]]; then
+ # Signal the wrapper (mirrors the INCREMENTAL_FALLBACK marker
convention) so the management
+ # server can record that the parent bitmap was reclaimed. Printed before
the size line below.
+ echo "PARENT_BITMAP_DELETED=true"
+ log "cleanup: removed parent bitmap $BITMAP_PARENT from $removed_from
disk(s)"
+ fi
+ fi
Review Comment:
Don't need the PARENT_BITMAP_DELETED marker. Let's keep the bitmap delete
best effort to simplify the code. The else block should anyway catch and log
any errors.
We don't need to read the bitmap again.
--
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]
