abh1sar commented on code in PR #12898:
URL: https://github.com/apache/cloudstack/pull/12898#discussion_r3988082204


##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -87,6 +91,75 @@ sanity_checks() {
   log -ne "Environment Sanity Checks successfully passed"
 }
 
+encrypt_backup() {
+  local backup_dir="$1"
+  if [[ -z "$ENCRYPT_PASSFILE" ]]; then
+    return
+  fi
+  if [[ ! -f "$ENCRYPT_PASSFILE" ]]; then
+    echo "Encryption passphrase file not found: $ENCRYPT_PASSFILE"
+    return 1
+  fi
+  log -ne "Encrypting backup files with LUKS"
+  # Preserve compression if it was requested upstream — otherwise the
+  # encrypt-step re-convert produces an uncompressed (but encrypted) qcow2,
+  # silently discarding the compression work done earlier.
+  local compress_flag=""
+  if [[ "$COMPRESS" == "true" ]]; then
+    compress_flag="-c"
+  fi
+  for img in "$backup_dir"/*.qcow2; do
+    [[ -f "$img" ]] || continue
+    local tmp_img="${img}.luks"
+    if qemu-img convert $compress_flag -O qcow2 \
+        --object "secret,id=sec0,file=$ENCRYPT_PASSFILE" \
+        -o "encrypt.format=luks,encrypt.key-secret=sec0" \
+        "$img" "$tmp_img" >> "$logFile" 2>&1; then
+      mv "$tmp_img" "$img"
+      log -ne "Encrypted: $img"
+    else
+      echo "Encryption failed for $img"
+      rm -f "$tmp_img"
+      return 1
+    fi
+  done
+}
+
+verify_backup() {
+  local backup_dir="$1"
+  local failed=0
+  # If encryption was applied to this backup, qemu-img check has to open the
+  # qcow2 with the same LUKS secret — otherwise every verification call fails
+  # with a "Could not open" error and --verify is unusable on encrypted
+  # backups.
+  local check_secret=()
+  if [[ -n "$ENCRYPT_PASSFILE" && -f "$ENCRYPT_PASSFILE" ]]; then
+    check_secret=(--object "secret,id=sec0,file=$ENCRYPT_PASSFILE")
+  fi
+  for img in "$backup_dir"/*.qcow2; do
+    [[ -f "$img" ]] || continue
+    local check_ok=0
+    if [[ ${#check_secret[@]} -gt 0 ]]; then
+      qemu-img check "${check_secret[@]}" --image-opts \
+        "driver=qcow2,file.filename=$img,encrypt.key-secret=sec0" \
+        > /dev/null 2>&1 && check_ok=1
+    else
+      qemu-img check "$img" > /dev/null 2>&1 && check_ok=1
+    fi
+    if [[ $check_ok -eq 1 ]]; then
+      log -ne "Backup verification passed: $img"
+    else
+      echo "Backup verification failed for $img"
+      log -ne "Backup verification FAILED: $img"

Review Comment:
   Only exit code 0 is accepted, and a non-zero result makes the caller run 
`cleanup`, deleting the entire backup.
   
   `qemu-img check` uses 2 for a corrupt image and 3 for leaked clusters. Leaks 
are benign, so as written a backup with leaked clusters is declared failed and 
destroyed. Could you confirm the exit code semantics on your side and treat 3 
as a warning rather than a failure?
   



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