Hanarion commented on code in PR #12133:
URL: https://github.com/apache/cloudstack/pull/12133#discussion_r3052924491


##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -218,6 +219,34 @@ mount_operation() {
   fi
 }
 
+umount_operation() {
+  elapsed=0
+  while fuser -m "$mount_point" >/dev/null 2>&1 && (( elapsed < 10 )); do
+      sleep 1
+      elapsed=$((elapsed + 1))
+  done
+
+  # Check if timeout was reached
+  if (( elapsed >= 10 )); then
+      echo "Timeout for unmounting reached: still busy"
+  fi
+
+  # Attempt to unmount safely and capture output
+  set +e
+  umount_output=$(umount "$mount_point" 2>&1)
+  umount_exit=$?
+  set -e
+
+  if [ "$umount_exit" -eq 0 ]; then
+    # Only remove directory if unmount succeeded
+    rmdir "$mount_point"
+  else
+    echo "Warning: failed to unmount $mount_point, skipping rmdir"

Review Comment:
   My bad i missed that, indeed we can remove the echo and only keep it in log. 
   Regarding the failure state, why I made it not fail is that the backup is 
not usable if in failed state, and not deletable, on the DB side, the backup 
files were not created, but on the disk side, it was indeed created and takes 
storage space. Maybe the better solution would be to detect if the backup 
fails, if so, clean the files ?
   My thoughts were that we prefer a semi-failed backup than a inexistant 
backup if we need to restore a backup.
   
   Hardcoding the timeout was a quick and easy solution indeed, I can add a 
constant on the top of the file to set it.



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