Copilot commented on code in PR #13424:
URL: https://github.com/apache/cloudstack/pull/13424#discussion_r3470255127
##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -196,10 +196,12 @@ backup_running_vm() {
# Print statistics
virsh -c qemu:///system domjobinfo $VM --completed
- du -sb $dest | cut -f1
-
- umount $mount_point
- rmdir $mount_point
+ backup_size=$(du -sb "$dest" 2>$logFile | cut -f1) || backup_size=0
Review Comment:
Redirection `2>$logFile` truncates `/var/log/cloudstack/agent/agent.log`
every time this runs (even when `du` succeeds, because the file is
opened/truncated). This can destroy agent logs and make diagnosing backup
issues harder. Use append redirection (and quote the path) instead.
##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -196,10 +196,12 @@ backup_running_vm() {
# Print statistics
virsh -c qemu:///system domjobinfo $VM --completed
- du -sb $dest | cut -f1
-
- umount $mount_point
- rmdir $mount_point
+ backup_size=$(du -sb "$dest" 2>$logFile | cut -f1) || backup_size=0
+
+ timeout $TIMEOUT umount "$mount_point" 2>$logFile || true
+ rmdir "$mount_point" 2>$logFile || true
Review Comment:
Both `umount` and `rmdir` redirect stderr with `2>$logFile`, which truncates
the CloudStack agent log on every backup attempt. Use append redirection (and
quote variables) so cleanup errors are captured without wiping existing logs.
--
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]