Double quote variables to prevent globbing and word splitting

This change goes through and quotes variables where appropriate to
avoid issues with incorrect splitting. The performance tests ran into
an issue with $SUDO_COMMAND splitting incorrectly because it was not
quoted. This change fixes that issue and hopefully gets ahead of any
other similar problems.

Reviewed by: John Wren Kennedy <jwk404@gmail.com>
Reviewed-by: Tony Nguyen <tony.nguyen@delphix.com>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Aleksandr Liber <aleksandr.liber@perforce.com>
Closes #17235
This commit is contained in:
Aleksandr Liber 2025-04-30 18:15:19 -07:00 committed by GitHub
parent 81dec433b0
commit aa46cc9812
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -15,7 +15,7 @@
# Copyright (c) 2016, Intel Corporation. # Copyright (c) 2016, Intel Corporation.
# #
. $STF_SUITE/include/libtest.shlib . "$STF_SUITE"/include/libtest.shlib
# Defaults common to all the tests in the regression group # Defaults common to all the tests in the regression group
export PERF_RUNTIME=${PERF_RUNTIME:-'180'} export PERF_RUNTIME=${PERF_RUNTIME:-'180'}
@ -46,12 +46,12 @@ function get_suffix
typeset sync=$2 typeset sync=$2
typeset iosize=$3 typeset iosize=$3
typeset sync_str=$(get_sync_str $sync) typeset sync_str=$(get_sync_str "$sync")
typeset filesystems=$(get_nfilesystems) typeset filesystems=$(get_nfilesystems)
typeset suffix="$sync_str.$iosize-ios" typeset suffix="$sync_str.$iosize-ios"
suffix="$suffix.$threads-threads.$filesystems-filesystems" suffix="$suffix.$threads-threads.$filesystems-filesystems"
echo $suffix echo "$suffix"
} }
function do_fio_run_impl function do_fio_run_impl
@ -65,12 +65,12 @@ function do_fio_run_impl
typeset sync=$6 typeset sync=$6
typeset iosize=$7 typeset iosize=$7
typeset sync_str=$(get_sync_str $sync) typeset sync_str=$(get_sync_str "$sync")
log_note "Running with $threads $sync_str threads, $iosize ios" log_note "Running with $threads $sync_str threads, $iosize ios"
if [[ -n $threads_per_fs && $threads_per_fs -ne 0 ]]; then if [[ -n $threads_per_fs && $threads_per_fs -ne 0 ]]; then
log_must test $do_recreate log_must test "$do_recreate"
verify_threads_per_fs $threads $threads_per_fs verify_threads_per_fs "$threads" "$threads_per_fs"
fi fi
if $do_recreate; then if $do_recreate; then
@ -105,7 +105,7 @@ function do_fio_run_impl
# the default filesystem (e.g. against a clone). # the default filesystem (e.g. against a clone).
# #
export DIRECTORY=$(get_directory) export DIRECTORY=$(get_directory)
log_note "DIRECTORY: " $DIRECTORY log_note "DIRECTORY: $DIRECTORY"
export RUNTIME=$PERF_RUNTIME export RUNTIME=$PERF_RUNTIME
export RANDSEED=$PERF_RANDSEED export RANDSEED=$PERF_RANDSEED
@ -122,33 +122,33 @@ function do_fio_run_impl
# disable client cache for reads. # disable client cache for reads.
if [[ $NFS -eq 1 ]]; then if [[ $NFS -eq 1 ]]; then
export DIRECT=1 export DIRECT=1
do_setup_nfs $script do_setup_nfs "$script"
else else
export DIRECT=0 export DIRECT=0
fi fi
# This will be part of the output filename. # This will be part of the output filename.
typeset suffix=$(get_suffix $threads $sync $iosize) typeset suffix=$(get_suffix "$threads" "$sync" "$iosize")
# Start the data collection # Start the data collection
do_collect_scripts $suffix do_collect_scripts "$suffix"
# Define output file # Define output file
typeset logbase="$(get_perf_output_dir)/$(basename \ typeset logbase="$(get_perf_output_dir)/$(basename \
$SUDO_COMMAND)" "$SUDO_COMMAND")"
typeset outfile="$logbase.fio.$suffix" typeset outfile="$logbase.fio.$suffix"
# Start the load # Start the load
if [[ $NFS -eq 1 ]]; then if [[ $NFS -eq 1 ]]; then
log_must ssh -t $NFS_USER@$NFS_CLIENT " log_must ssh -t "$NFS_USER@$NFS_CLIENT" "
fio --output-format=${PERF_FIO_FORMAT} \ fio --output-format=${PERF_FIO_FORMAT} \
--output /tmp/fio.out /tmp/test.fio --output /tmp/fio.out /tmp/test.fio
" "
log_must scp $NFS_USER@$NFS_CLIENT:/tmp/fio.out $outfile log_must scp "$NFS_USER@$NFS_CLIENT":/tmp/fio.out "$outfile"
log_must ssh -t $NFS_USER@$NFS_CLIENT "sudo -S umount $NFS_MOUNT" log_must ssh -t "$NFS_USER@$NFS_CLIENT" "sudo -S umount $NFS_MOUNT"
else else
log_must fio --output-format=${PERF_FIO_FORMAT} \ log_must fio --output-format="${PERF_FIO_FORMAT}" \
--output $outfile $FIO_SCRIPTS/$script --output "$outfile" "$FIO_SCRIPTS/$script"
fi fi
} }
@ -176,13 +176,13 @@ function do_fio_run
for sync in $PERF_SYNC_TYPES; do for sync in $PERF_SYNC_TYPES; do
for iosize in $PERF_IOSIZES; do for iosize in $PERF_IOSIZES; do
do_fio_run_impl \ do_fio_run_impl \
$script \ "$script" \
$do_recreate \ "$do_recreate" \
$clear_cache \ "$clear_cache" \
$threads \ "$threads" \
$threads_per_fs \ "$threads_per_fs" \
$sync \ "$sync" \
$iosize "$iosize"
done done
done done
done done
@ -195,12 +195,12 @@ function do_fio_run
function do_setup_nfs function do_setup_nfs
{ {
typeset script=$1 typeset script=$1
zfs set sharenfs=on $TESTFS zfs set sharenfs=on "$TESTFS"
log_must chmod -R 777 /$TESTFS log_must chmod -R 777 /"$TESTFS"
ssh -t $NFS_USER@$NFS_CLIENT "mkdir -m 777 -p $NFS_MOUNT" ssh -t "$NFS_USER@$NFS_CLIENT" "mkdir -m 777 -p $NFS_MOUNT"
ssh -t $NFS_USER@$NFS_CLIENT "sudo -S umount $NFS_MOUNT" ssh -t "$NFS_USER@$NFS_CLIENT" "sudo -S umount $NFS_MOUNT"
log_must ssh -t $NFS_USER@$NFS_CLIENT " log_must ssh -t "$NFS_USER@$NFS_CLIENT" "
sudo -S mount $NFS_OPTIONS $NFS_SERVER:/$TESTFS $NFS_MOUNT sudo -S mount $NFS_OPTIONS $NFS_SERVER:/$TESTFS $NFS_MOUNT
" "
# #
@ -211,9 +211,9 @@ function do_setup_nfs
export jobnum='$jobnum' export jobnum='$jobnum'
while read line; do while read line; do
eval echo "$line" eval echo "$line"
done < $FIO_SCRIPTS/$script > /tmp/test.fio done < "$FIO_SCRIPTS/$script" > /tmp/test.fio
log_must sed -i -e "s%directory.*%directory=$NFS_MOUNT%" /tmp/test.fio log_must sed -i -e "s%directory.*%directory=$NFS_MOUNT%" /tmp/test.fio
log_must scp /tmp/test.fio $NFS_USER@$NFS_CLIENT:/tmp log_must scp /tmp/test.fio "$NFS_USER@$NFS_CLIENT":/tmp
log_must rm /tmp/test.fio log_must rm /tmp/test.fio
} }
@ -233,17 +233,17 @@ function do_collect_scripts
typeset oIFS=$IFS typeset oIFS=$IFS
IFS=',' IFS=','
for item in $PERF_COLLECT_SCRIPTS; do for item in $PERF_COLLECT_SCRIPTS; do
collect_scripts+=($(echo $item | sed 's/^ *//g')) collect_scripts+=($(echo "$item" | sed 's/^ *//g'))
done done
IFS=$oIFS IFS=$oIFS
typeset idx=0 typeset idx=0
while [[ $idx -lt "${#collect_scripts[@]}" ]]; do while [[ $idx -lt "${#collect_scripts[@]}" ]]; do
typeset logbase="$(get_perf_output_dir)/$(basename \ typeset logbase="$(get_perf_output_dir)/$(basename \
$SUDO_COMMAND)" "$SUDO_COMMAND")"
typeset outfile="$logbase.${collect_scripts[$idx + 1]}.$suffix" typeset outfile="$logbase.${collect_scripts[$idx + 1]}.$suffix"
timeout $PERF_RUNTIME ${collect_scripts[$idx]} >$outfile 2>&1 & timeout "$PERF_RUNTIME" "${collect_scripts[$idx]}" >"$outfile" 2>&1 &
((idx += 2)) ((idx += 2))
done done
@ -256,9 +256,9 @@ function do_collect_scripts
function get_perf_output_dir function get_perf_output_dir
{ {
typeset dir="$PWD/perf_data" typeset dir="$PWD/perf_data"
[[ -d $dir ]] || mkdir -p $dir [[ -d $dir ]] || mkdir -p "$dir"
echo $dir echo "$dir"
} }
function apply_zinject_delays function apply_zinject_delays
@ -270,7 +270,7 @@ function apply_zinject_delays
for disk in $DISKS; do for disk in $DISKS; do
log_must zinject \ log_must zinject \
-d $disk -D ${ZINJECT_DELAYS[$idx]} $PERFPOOL -d "$disk" -D "${ZINJECT_DELAYS[$idx]}" "$PERFPOOL"
done done
((idx += 1)) ((idx += 1))
@ -302,7 +302,7 @@ function recreate_perf_pool
# This function handles the case where the pool already exists, # This function handles the case where the pool already exists,
# and will destroy the previous pool and recreate a new pool. # and will destroy the previous pool and recreate a new pool.
# #
create_pool $PERFPOOL $DISKS create_pool "$PERFPOOL" "$DISKS"
} }
function verify_threads_per_fs function verify_threads_per_fs
@ -310,8 +310,8 @@ function verify_threads_per_fs
typeset threads=$1 typeset threads=$1
typeset threads_per_fs=$2 typeset threads_per_fs=$2
log_must test -n $threads log_must test -n "$threads"
log_must test -n $threads_per_fs log_must test -n "$threads_per_fs"
# #
# A value of "0" is treated as a "special value", and it is # A value of "0" is treated as a "special value", and it is
@ -325,7 +325,7 @@ function verify_threads_per_fs
# than or equal to zero; since we just verified the value isn't # than or equal to zero; since we just verified the value isn't
# 0 above, then it must be greater than zero here. # 0 above, then it must be greater than zero here.
# #
log_must test $threads_per_fs -ge 0 log_must test "$threads_per_fs" -ge 0
# #
# This restriction can be lifted later if needed, but for now, # This restriction can be lifted later if needed, but for now,
@ -341,9 +341,9 @@ function populate_perf_filesystems
typeset nfilesystems=${1:-1} typeset nfilesystems=${1:-1}
export TESTFS="" export TESTFS=""
for i in $(seq 1 $nfilesystems); do for i in $(seq 1 "$nfilesystems"); do
typeset dataset="$PERFPOOL/fs$i" typeset dataset="$PERFPOOL/fs$i"
create_dataset $dataset $PERF_FS_OPTS create_dataset "$dataset" "$PERF_FS_OPTS"
if [[ -z "$TESTFS" ]]; then if [[ -z "$TESTFS" ]]; then
TESTFS="$dataset" TESTFS="$dataset"
else else
@ -354,13 +354,13 @@ function populate_perf_filesystems
function get_nfilesystems function get_nfilesystems
{ {
typeset filesystems=( $TESTFS ) typeset filesystems=($TESTFS)
echo ${#filesystems[@]} echo ${#filesystems[@]}
} }
function get_directory function get_directory
{ {
typeset filesystems=( $TESTFS ) typeset filesystems=($TESTFS)
typeset directory= typeset directory=
typeset idx=0 typeset idx=0
@ -376,7 +376,7 @@ function get_directory
((idx += 1)) ((idx += 1))
done done
echo $directory echo "$directory"
} }
function get_min_arc_size function get_min_arc_size
@ -447,7 +447,7 @@ function get_dbuf_cache_size
dbuf_cache_size=$(($(get_arc_target) / 2**dbuf_cache_shift)) dbuf_cache_size=$(($(get_arc_target) / 2**dbuf_cache_shift))
fi || log_fail "get_dbuf_cache_size failed" fi || log_fail "get_dbuf_cache_size failed"
echo $dbuf_cache_size echo "$dbuf_cache_size"
} }
# Create a file with some information about how this system is configured. # Create a file with some information about how this system is configured.
@ -455,23 +455,23 @@ function get_system_config
{ {
typeset config=$PERF_DATA_DIR/$1 typeset config=$PERF_DATA_DIR/$1
echo "{" >>$config echo "{" >>"$config"
if is_linux; then if is_linux; then
echo " \"ncpus\": \"$(lscpu | awk '/^CPU\(s\)/ {print $2; exit}')\"," >>$config echo " \"ncpus\": \"$(lscpu | awk '/^CPU\(s\)/ {print $2; exit}')\"," >>"$config"
echo " \"physmem\": \"$(free -b | \ echo " \"physmem\": \"$(free -b | \
awk '$1 == "Mem:" { print $2 }')\"," >>$config awk '$1 == "Mem:" { print $2 }')\"," >>"$config"
echo " \"c_max\": \"$(get_max_arc_size)\"," >>$config echo " \"c_max\": \"$(get_max_arc_size)\"," >>"$config"
echo " \"hostname\": \"$(uname -n)\"," >>$config echo " \"hostname\": \"$(uname -n)\"," >>"$config"
echo " \"kernel version\": \"$(uname -sr)\"," >>$config echo " \"kernel version\": \"$(uname -sr)\"," >>"$config"
else else
dtrace -qn 'BEGIN{ dtrace -qn 'BEGIN{
printf(" \"ncpus\": %d,\n", `ncpus); printf(" \"ncpus\": %d,\n", `ncpus);
printf(" \"physmem\": %u,\n", `physmem * `_pagesize); printf(" \"physmem\": %u,\n", `physmem * `_pagesize);
printf(" \"c_max\": %u,\n", `arc_stats.arcstat_c_max.value.ui64); printf(" \"c_max\": %u,\n", `arc_stats.arcstat_c_max.value.ui64);
printf(" \"kmem_flags\": \"0x%x\",", `kmem_flags); printf(" \"kmem_flags\": \"0x%x\",", `kmem_flags);
exit(0)}' >>$config exit(0)}' >>"$config"
echo " \"hostname\": \"$(uname -n)\"," >>$config echo " \"hostname\": \"$(uname -n)\"," >>"$config"
echo " \"kernel version\": \"$(uname -v)\"," >>$config echo " \"kernel version\": \"$(uname -v)\"," >>"$config"
fi fi
if is_linux; then if is_linux; then
lsblk -dino NAME,SIZE | awk 'BEGIN { lsblk -dino NAME,SIZE | awk 'BEGIN {
@ -479,11 +479,11 @@ function get_system_config
{disk = $1} {size = $2; {disk = $1} {size = $2;
if (first != 1) {printf(",\n")} else {first = 0} if (first != 1) {printf(",\n")} else {first = 0}
printf(" \"%s\": \"%s\"", disk, size)} printf(" \"%s\": \"%s\"", disk, size)}
END {printf("\n },\n")}' >>$config END {printf("\n },\n")}' >>"$config"
zfs_tunables="/sys/module/zfs/parameters" zfs_tunables="/sys/module/zfs/parameters"
printf " \"tunables\": {\n" >>$config printf " \"tunables\": {\n" >>"$config"
for tunable in \ for tunable in \
zfs_arc_max \ zfs_arc_max \
zfs_arc_sys_free \ zfs_arc_sys_free \
@ -500,12 +500,12 @@ function get_system_config
do do
if [ "$tunable" != "zfs_arc_max" ] if [ "$tunable" != "zfs_arc_max" ]
then then
printf ",\n" >>$config printf ",\n" >>"$config"
fi fi
printf " \"$tunable\": \"$(<$zfs_tunables/$tunable)\"" \ printf " \"$tunable\": \"$(<$zfs_tunables/$tunable)\"" \
>>$config >>"$config"
done done
printf "\n }\n" >>$config printf "\n }\n" >>"$config"
else else
iostat -En | awk 'BEGIN { iostat -En | awk 'BEGIN {
printf(" \"disks\": {\n"); first = 1} printf(" \"disks\": {\n"); first = 1}
@ -513,15 +513,15 @@ function get_system_config
/^Size: [^0]/ {size = $2; /^Size: [^0]/ {size = $2;
if (first != 1) {printf(",\n")} else {first = 0} if (first != 1) {printf(",\n")} else {first = 0}
printf(" \"%s\": \"%s\"", disk, size)} printf(" \"%s\": \"%s\"", disk, size)}
END {printf("\n },\n")}' >>$config END {printf("\n },\n")}' >>"$config"
sed -n 's/^set \(.*\)[ ]=[ ]\(.*\)/\1=\2/p' /etc/system | \ sed -n 's/^set \(.*\)[ ]=[ ]\(.*\)/\1=\2/p' /etc/system | \
awk -F= 'BEGIN {printf(" \"system\": {\n"); first = 1} awk -F= 'BEGIN {printf(" \"system\": {\n"); first = 1}
{if (first != 1) {printf(",\n")} else {first = 0}; {if (first != 1) {printf(",\n")} else {first = 0};
printf(" \"%s\": %s", $1, $2)} printf(" \"%s\": %s", $1, $2)}
END {printf("\n }\n")}' >>$config END {printf("\n }\n")}' >>"$config"
fi fi
echo "}" >>$config echo "}" >>"$config"
} }
# #
@ -535,7 +535,7 @@ function pool_to_lun_list
case "$UNAME" in case "$UNAME" in
Linux) Linux)
ctds=$(zpool list -HLv $pool | \ ctds=$(zpool list -HLv "$pool" | \
awk '/sd[a-z]*|loop[0-9]*|dm-[0-9]*/ {print $1}') awk '/sd[a-z]*|loop[0-9]*|dm-[0-9]*/ {print $1}')
for ctd in $ctds; do for ctd in $ctds; do
@ -543,17 +543,17 @@ function pool_to_lun_list
done done
;; ;;
FreeBSD) FreeBSD)
lun_list+=$(zpool list -HLv $pool | \ lun_list+=$(zpool list -HLv "$pool" | \
awk '/a?da[0-9]+|md[0-9]+|mfid[0-9]+|nda[0-9]+|nvd[0-9]+|vtbd[0-9]+/ awk '/a?da[0-9]+|md[0-9]+|mfid[0-9]+|nda[0-9]+|nvd[0-9]+|vtbd[0-9]+/
{ printf "%s:", $1 }') { printf "%s:", $1 }')
;; ;;
*) *)
ctds=$(zpool list -v $pool | ctds=$(zpool list -v "$pool" |
awk '/c[0-9]*t[0-9a-fA-F]*d[0-9]*/ {print $1}') awk '/c[0-9]*t[0-9a-fA-F]*d[0-9]*/ {print $1}')
for ctd in $ctds; do for ctd in $ctds; do
# Get the device name as it appears in /etc/path_to_inst # Get the device name as it appears in /etc/path_to_inst
devname=$(readlink -f /dev/dsk/${ctd}s0 | sed -n 's/\/devices\([^:]*\):.*/\1/p') devname=$(readlink -f /dev/dsk/"${ctd}"s0 | sed -n 's/\/devices\([^:]*\):.*/\1/p')
# Add a string composed of the driver name and instance # Add a string composed of the driver name and instance
# number to the list for comparison with dev_statname. # number to the list for comparison with dev_statname.
lun=$(sed 's/"//g' /etc/path_to_inst | awk -v dn="$devname" '$0 ~ dn {print $3$2}') lun=$(sed 's/"//g' /etc/path_to_inst | awk -v dn="$devname" '$0 ~ dn {print $3$2}')
@ -561,7 +561,7 @@ function pool_to_lun_list
done done
;; ;;
esac esac
echo $lun_list echo "$lun_list"
} }
function print_perf_settings function print_perf_settings