jmsperu commented on PR #13074: URL: https://github.com/apache/cloudstack/pull/13074#issuecomment-4878871669
Thanks @abh1sar — both addressed in 4552c441c3. **1. Storage guard in `decideChain`:** added `allVolumesOnCheckpointCapableStorage()` — if any of the VM's volumes sits on storage that cannot carry a per-disk checkpoint (Ceph-RBD, Linstor), the backup falls back to legacy-full, so those storages are never regressed by an incremental attempt. Covered by 3 new unit tests. **2. Parent-bitmap cleanup:** implemented, with one deliberate deviation from the sketch I want to flag. Rather than `checkpoint-delete` in the wrapper, the cleanup runs in `nasbackup.sh` after a successful incremental and frees the parent **per-disk via `block-dirty-bitmap-remove`**. Reason: `checkpoint-delete` on a parent that has a child makes libvirt **merge** the parent's dirty bits into the new bitmap, which would make the *next* incremental re-copy already-backed-up regions. `block-dirty-bitmap-remove` is a clean free with no merge. It is gated exactly as you suggested (`!incrementalFallback && bitmapParent != null`) and is best-effort (a failure logs a warning and never fails the backup, since the data is already written). The reclaim is surfaced to the orchestrator via a `PARENT_BITMAP_DELETED` marker -> `BackupAnswer.parentBitmapDeleted` -> provider log, mirroring the existing `INCREMENTAL_FALLBACK` pattern, so it is auditable. Validated on a live host (libvirt/QEMU 10.0.0): after the incremental the parent bitmap is gone and only the new one remains, the **next incremental still succeeds** (chain intact), and incrementals stay small — confirming the free-not-merge behavior. `NASBackupProviderTest` (18 tests) is green. -- 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]
