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.
   
   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]

Reply via email to