Damans227 commented on code in PR #14009:
URL: https://github.com/apache/cloudstack/pull/14009#discussion_r4231421158
##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -154,7 +154,9 @@ mount_operation() {
if [ ${NAS_TYPE} == "cifs" ]; then
MOUNT_OPTS="${MOUNT_OPTS},nobrl"
fi
- mount -t ${NAS_TYPE} ${NAS_ADDRESS} ${mount_point} $([[ ! -z "${MOUNT_OPTS}"
]] && echo -o ${MOUNT_OPTS}) | tee -a "$logFile"
+ local mount_args=(-t "${NAS_TYPE}" "${NAS_ADDRESS}" "${mount_point}")
+ [[ -n "${MOUNT_OPTS}" ]] && mount_args+=(-o "${MOUNT_OPTS}")
+ mount "${mount_args[@]}" 2>&1 | tee -a "$logFile"
if [ $? -eq 0 ]; then
Review Comment:
can we check ${PIPESTATUS[0]} here? $? is the result of tee, so a failed
mount still counts as mounted
##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -154,7 +154,9 @@ mount_operation() {
if [ ${NAS_TYPE} == "cifs" ]; then
MOUNT_OPTS="${MOUNT_OPTS},nobrl"
fi
- mount -t ${NAS_TYPE} ${NAS_ADDRESS} ${mount_point} $([[ ! -z "${MOUNT_OPTS}"
]] && echo -o ${MOUNT_OPTS}) | tee -a "$logFile"
+ local mount_args=(-t "${NAS_TYPE}" "${NAS_ADDRESS}" "${mount_point}")
+ [[ -n "${MOUNT_OPTS}" ]] && mount_args+=(-o "${MOUNT_OPTS}")
+ mount "${mount_args[@]}" 2>&1 | tee -a "$logFile"
Review Comment:
any reason for 2>&1 here? a mount warning would now end up in the output the
agent reads the backup size from
--
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]
