MitchDrage commented on code in PR #14305:
URL: https://github.com/apache/cloudstack/pull/14305#discussion_r4194768371
##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -503,9 +530,33 @@ mount_operation() {
fi
}
+abort_backup_job() {
+ [[ $BACKUP_JOB_ACTIVE -eq 1 ]] || return 0
+
+ # The push backup job writes into $dest until it ends, so it must be gone
before the
+ # destination is removed or unmounted.
+ virsh -c qemu:///system domjobabort "$VM" > /dev/null 2>>"$logFile" || true
+ local i job
+ for ((i = 0; i < 60; i++)); do
+ job=$(virsh -c qemu:///system domjobinfo "$VM" 2>/dev/null | awk '/Job
type:/ {print $3}')
+ if [[ -z "$job" || "$job" == "None" ]]; then
+ BACKUP_JOB_ACTIVE=0
+ return 0
+ fi
+ sleep 1
+ done
+ return 1
+}
+
cleanup() {
local status=0
+ if ! abort_backup_job; then
+ echo "Backup job for vm $VM is still running after abort, leaving $dest
mounted at $mount_point"
Review Comment:
Yep, that's where the old code ended up too: `cleanup()` deleted the
destination under the live job, then umount failed busy and left the mount
behind (we had a significant number of those in production).
With this PR the thaw path aborts the job first, and in testing the abort
completes in about 1.4s, so the 60s timeout should only be reached if the NAS
itself has hung.
That's why I kept the old behaviour of leaving the mount in place when this
timeout is reached; I've just made it explicit, with a message naming the mount
point.
If you'd like something else here, what would you recommend for that case? I
was thinking a `umount -f` when the abort times out. The issue is that
concurrent backups to the same repository on that host share the NFS
superblock, so their I/O would fail too. Again though, that should only happen
if the NAS has hung, so an alert would be a good thing to add here regardless.
--
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]
