DaanHoogland commented on code in PR #12843:
URL: https://github.com/apache/cloudstack/pull/12843#discussion_r3309967146


##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -169,7 +187,13 @@ backup_running_vm() {
         echo "Virsh backup job failed"
         cleanup ;;

Review Comment:
   this remark makes sense; either catch and evaluate the return of the 
`cleanup()` call or add a break statement.



##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -262,19 +287,62 @@ mount_operation() {
   fi
 }
 
+check_free_space() {
+  local free_bytes
+  free_bytes=$(df -P "$mount_point" 2>/dev/null | awk 'NR==2 {print $4}')
+  if [[ -n "$free_bytes" ]]; then
+    # df -P reports 1K blocks; convert to bytes.
+    free_bytes=$((free_bytes * 1024))
+    if [[ $free_bytes -lt $MIN_FREE_SPACE ]]; then
+      echo "Insufficient free space on backup target: $((free_bytes / 
1048576)) MB available, $((MIN_FREE_SPACE / 1048576)) MB required"
+      exit 1
+    fi
+    log -ne "Backup target has $((free_bytes / 1073741824)) GB free space"
+  fi
+}
+
 cleanup() {
+  # Idempotent: skip if a prior explicit call already ran. Without this guard,
+  # the EXIT trap would re-run cleanup and fail on the already-unmounted point.
+  [[ $CLEANUP_DONE -eq 1 ]] && return 0
+  CLEANUP_DONE=1

Review Comment:
   should this line move to the end of the method, when cleanup is actually 
done?



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