jmsperu commented on code in PR #13074:
URL: https://github.com/apache/cloudstack/pull/13074#discussion_r3253145051
##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -495,24 +693,244 @@ public boolean deleteBackup(Backup backup, boolean
forced) {
throw new CloudRuntimeException(String.format("Unable to find a
running KVM host in zone %d to delete backup %s", backup.getZoneId(),
backup.getUuid()));
}
- DeleteBackupCommand command = new
DeleteBackupCommand(backup.getExternalId(), backupRepository.getType(),
- backupRepository.getAddress(),
backupRepository.getMountOptions());
+ // Repair the chain (if any) before removing the backup file. For
chained backups,
Review Comment:
Addressed in b7b74c4f88 — `deleteBackup` now follows the snapshot-style
pattern you described. Walked through
`DefaultSnapshotStrategy#deleteSnapshotChain` first to make sure the semantics
line up (children alive ⇒ tombstone, no children ⇒ delete file and sweep up
tombstoned ancestors). Specifically:
* If B1 has live children ⇒ persist `nas.delete_pending=true` in
`backup_details`, leave the on-NAS file and DB row untouched.
* If B1 has no live children ⇒ delete its qcow2 directory and DB row via
`DeleteBackupCommand`, then walk up the chain and collect any ancestor where
`isDeletePending && findLiveChildren().isEmpty()`.
* `forced=true` cascades the whole subtree leaf-first in one go (operator
opt-in).
Dropped the rebase / chain-repair path entirely from the provider — no more
`RebaseStep` / `ChainRepairPlan` / `PositionShift` inner classes. See
`plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:757-961`.
Unit tests `deleteWithLiveChildMarksDeletePendingAndPreservesFile` and
`deletingLeafSweepsUpDeletePendingParent` verify both halves.
One thing I held off on: `RebaseBackupCommand` +
`LibvirtRebaseBackupCommandWrapper` are still in tree but no longer referenced
by the provider. Happy to delete them in a follow-up commit if you'd prefer
that — left as-is to keep this diff focused on the chain-delete change.
Happy to iterate if the approach differs from what you had in mind.
--
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]
