Skip to content

Commit edae295

Browse files
AleksandrLibertonyhutter
authored andcommitted
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 <[email protected]> Reviewed-by: Tony Nguyen <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Aleksandr Liber <[email protected]> Closes #17235 (cherry picked from commit aa46cc9)
1 parent c85f2fd commit edae295

File tree

1 file changed

+68
-68
lines changed

1 file changed

+68
-68
lines changed

tests/zfs-tests/tests/perf/perf.shlib

Lines changed: 68 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
# Copyright (c) 2016, Intel Corporation.
1616
#
1717

18-
. $STF_SUITE/include/libtest.shlib
18+
. "$STF_SUITE"/include/libtest.shlib
1919

2020
# Defaults common to all the tests in the regression group
2121
export PERF_RUNTIME=${PERF_RUNTIME:-'180'}
@@ -46,12 +46,12 @@ function get_suffix
4646
typeset sync=$2
4747
typeset iosize=$3
4848

49-
typeset sync_str=$(get_sync_str $sync)
49+
typeset sync_str=$(get_sync_str "$sync")
5050
typeset filesystems=$(get_nfilesystems)
5151

5252
typeset suffix="$sync_str.$iosize-ios"
5353
suffix="$suffix.$threads-threads.$filesystems-filesystems"
54-
echo $suffix
54+
echo "$suffix"
5555
}
5656

5757
function do_fio_run_impl
@@ -65,12 +65,12 @@ function do_fio_run_impl
6565
typeset sync=$6
6666
typeset iosize=$7
6767

68-
typeset sync_str=$(get_sync_str $sync)
68+
typeset sync_str=$(get_sync_str "$sync")
6969
log_note "Running with $threads $sync_str threads, $iosize ios"
7070

7171
if [[ -n $threads_per_fs && $threads_per_fs -ne 0 ]]; then
72-
log_must test $do_recreate
73-
verify_threads_per_fs $threads $threads_per_fs
72+
log_must test "$do_recreate"
73+
verify_threads_per_fs "$threads" "$threads_per_fs"
7474
fi
7575

7676
if $do_recreate; then
@@ -105,7 +105,7 @@ function do_fio_run_impl
105105
# the default filesystem (e.g. against a clone).
106106
#
107107
export DIRECTORY=$(get_directory)
108-
log_note "DIRECTORY: " $DIRECTORY
108+
log_note "DIRECTORY: $DIRECTORY"
109109

110110
export RUNTIME=$PERF_RUNTIME
111111
export RANDSEED=$PERF_RANDSEED
@@ -122,33 +122,33 @@ function do_fio_run_impl
122122
# disable client cache for reads.
123123
if [[ $NFS -eq 1 ]]; then
124124
export DIRECT=1
125-
do_setup_nfs $script
125+
do_setup_nfs "$script"
126126
else
127127
export DIRECT=0
128128
fi
129129

130130
# This will be part of the output filename.
131-
typeset suffix=$(get_suffix $threads $sync $iosize)
131+
typeset suffix=$(get_suffix "$threads" "$sync" "$iosize")
132132

133133
# Start the data collection
134-
do_collect_scripts $suffix
134+
do_collect_scripts "$suffix"
135135

136136
# Define output file
137137
typeset logbase="$(get_perf_output_dir)/$(basename \
138-
$SUDO_COMMAND)"
138+
"$SUDO_COMMAND")"
139139
typeset outfile="$logbase.fio.$suffix"
140140

141141
# Start the load
142142
if [[ $NFS -eq 1 ]]; then
143-
log_must ssh -t $NFS_USER@$NFS_CLIENT "
143+
log_must ssh -t "$NFS_USER@$NFS_CLIENT" "
144144
fio --output-format=${PERF_FIO_FORMAT} \
145145
--output /tmp/fio.out /tmp/test.fio
146146
"
147-
log_must scp $NFS_USER@$NFS_CLIENT:/tmp/fio.out $outfile
148-
log_must ssh -t $NFS_USER@$NFS_CLIENT "sudo -S umount $NFS_MOUNT"
147+
log_must scp "$NFS_USER@$NFS_CLIENT":/tmp/fio.out "$outfile"
148+
log_must ssh -t "$NFS_USER@$NFS_CLIENT" "sudo -S umount $NFS_MOUNT"
149149
else
150-
log_must fio --output-format=${PERF_FIO_FORMAT} \
151-
--output $outfile $FIO_SCRIPTS/$script
150+
log_must fio --output-format="${PERF_FIO_FORMAT}" \
151+
--output "$outfile" "$FIO_SCRIPTS/$script"
152152
fi
153153
}
154154

@@ -176,13 +176,13 @@ function do_fio_run
176176
for sync in $PERF_SYNC_TYPES; do
177177
for iosize in $PERF_IOSIZES; do
178178
do_fio_run_impl \
179-
$script \
180-
$do_recreate \
181-
$clear_cache \
182-
$threads \
183-
$threads_per_fs \
184-
$sync \
185-
$iosize
179+
"$script" \
180+
"$do_recreate" \
181+
"$clear_cache" \
182+
"$threads" \
183+
"$threads_per_fs" \
184+
"$sync" \
185+
"$iosize"
186186
done
187187
done
188188
done
@@ -195,12 +195,12 @@ function do_fio_run
195195
function do_setup_nfs
196196
{
197197
typeset script=$1
198-
zfs set sharenfs=on $TESTFS
199-
log_must chmod -R 777 /$TESTFS
198+
zfs set sharenfs=on "$TESTFS"
199+
log_must chmod -R 777 /"$TESTFS"
200200

201-
ssh -t $NFS_USER@$NFS_CLIENT "mkdir -m 777 -p $NFS_MOUNT"
202-
ssh -t $NFS_USER@$NFS_CLIENT "sudo -S umount $NFS_MOUNT"
203-
log_must ssh -t $NFS_USER@$NFS_CLIENT "
201+
ssh -t "$NFS_USER@$NFS_CLIENT" "mkdir -m 777 -p $NFS_MOUNT"
202+
ssh -t "$NFS_USER@$NFS_CLIENT" "sudo -S umount $NFS_MOUNT"
203+
log_must ssh -t "$NFS_USER@$NFS_CLIENT" "
204204
sudo -S mount $NFS_OPTIONS $NFS_SERVER:/$TESTFS $NFS_MOUNT
205205
"
206206
#
@@ -211,9 +211,9 @@ function do_setup_nfs
211211
export jobnum='$jobnum'
212212
while read line; do
213213
eval echo "$line"
214-
done < $FIO_SCRIPTS/$script > /tmp/test.fio
214+
done < "$FIO_SCRIPTS/$script" > /tmp/test.fio
215215
log_must sed -i -e "s%directory.*%directory=$NFS_MOUNT%" /tmp/test.fio
216-
log_must scp /tmp/test.fio $NFS_USER@$NFS_CLIENT:/tmp
216+
log_must scp /tmp/test.fio "$NFS_USER@$NFS_CLIENT":/tmp
217217
log_must rm /tmp/test.fio
218218
}
219219

@@ -233,17 +233,17 @@ function do_collect_scripts
233233
typeset oIFS=$IFS
234234
IFS=','
235235
for item in $PERF_COLLECT_SCRIPTS; do
236-
collect_scripts+=($(echo $item | sed 's/^ *//g'))
236+
collect_scripts+=($(echo "$item" | sed 's/^ *//g'))
237237
done
238238
IFS=$oIFS
239239

240240
typeset idx=0
241241
while [[ $idx -lt "${#collect_scripts[@]}" ]]; do
242242
typeset logbase="$(get_perf_output_dir)/$(basename \
243-
$SUDO_COMMAND)"
243+
"$SUDO_COMMAND")"
244244
typeset outfile="$logbase.${collect_scripts[$idx + 1]}.$suffix"
245245

246-
timeout $PERF_RUNTIME ${collect_scripts[$idx]} >$outfile 2>&1 &
246+
timeout "$PERF_RUNTIME" "${collect_scripts[$idx]}" >"$outfile" 2>&1 &
247247
((idx += 2))
248248
done
249249

@@ -256,9 +256,9 @@ function do_collect_scripts
256256
function get_perf_output_dir
257257
{
258258
typeset dir="$PWD/perf_data"
259-
[[ -d $dir ]] || mkdir -p $dir
259+
[[ -d $dir ]] || mkdir -p "$dir"
260260

261-
echo $dir
261+
echo "$dir"
262262
}
263263

264264
function apply_zinject_delays
@@ -270,7 +270,7 @@ function apply_zinject_delays
270270

271271
for disk in $DISKS; do
272272
log_must zinject \
273-
-d $disk -D ${ZINJECT_DELAYS[$idx]} $PERFPOOL
273+
-d "$disk" -D "${ZINJECT_DELAYS[$idx]}" "$PERFPOOL"
274274
done
275275

276276
((idx += 1))
@@ -302,16 +302,16 @@ function recreate_perf_pool
302302
# This function handles the case where the pool already exists,
303303
# and will destroy the previous pool and recreate a new pool.
304304
#
305-
create_pool $PERFPOOL $DISKS
305+
create_pool "$PERFPOOL" "$DISKS"
306306
}
307307

308308
function verify_threads_per_fs
309309
{
310310
typeset threads=$1
311311
typeset threads_per_fs=$2
312312

313-
log_must test -n $threads
314-
log_must test -n $threads_per_fs
313+
log_must test -n "$threads"
314+
log_must test -n "$threads_per_fs"
315315

316316
#
317317
# A value of "0" is treated as a "special value", and it is
@@ -325,7 +325,7 @@ function verify_threads_per_fs
325325
# than or equal to zero; since we just verified the value isn't
326326
# 0 above, then it must be greater than zero here.
327327
#
328-
log_must test $threads_per_fs -ge 0
328+
log_must test "$threads_per_fs" -ge 0
329329

330330
#
331331
# This restriction can be lifted later if needed, but for now,
@@ -341,9 +341,9 @@ function populate_perf_filesystems
341341
typeset nfilesystems=${1:-1}
342342

343343
export TESTFS=""
344-
for i in $(seq 1 $nfilesystems); do
344+
for i in $(seq 1 "$nfilesystems"); do
345345
typeset dataset="$PERFPOOL/fs$i"
346-
create_dataset $dataset $PERF_FS_OPTS
346+
create_dataset "$dataset" "$PERF_FS_OPTS"
347347
if [[ -z "$TESTFS" ]]; then
348348
TESTFS="$dataset"
349349
else
@@ -354,13 +354,13 @@ function populate_perf_filesystems
354354

355355
function get_nfilesystems
356356
{
357-
typeset filesystems=( $TESTFS )
357+
typeset filesystems=($TESTFS)
358358
echo ${#filesystems[@]}
359359
}
360360

361361
function get_directory
362362
{
363-
typeset filesystems=( $TESTFS )
363+
typeset filesystems=($TESTFS)
364364
typeset directory=
365365

366366
typeset idx=0
@@ -376,7 +376,7 @@ function get_directory
376376
((idx += 1))
377377
done
378378

379-
echo $directory
379+
echo "$directory"
380380
}
381381

382382
function get_min_arc_size
@@ -447,43 +447,43 @@ function get_dbuf_cache_size
447447
dbuf_cache_size=$(($(get_arc_target) / 2**dbuf_cache_shift))
448448
fi || log_fail "get_dbuf_cache_size failed"
449449

450-
echo $dbuf_cache_size
450+
echo "$dbuf_cache_size"
451451
}
452452

453453
# Create a file with some information about how this system is configured.
454454
function get_system_config
455455
{
456456
typeset config=$PERF_DATA_DIR/$1
457457

458-
echo "{" >>$config
458+
echo "{" >>"$config"
459459
if is_linux; then
460-
echo " \"ncpus\": \"$(lscpu | awk '/^CPU\(s\)/ {print $2; exit}')\"," >>$config
460+
echo " \"ncpus\": \"$(lscpu | awk '/^CPU\(s\)/ {print $2; exit}')\"," >>"$config"
461461
echo " \"physmem\": \"$(free -b | \
462-
awk '$1 == "Mem:" { print $2 }')\"," >>$config
463-
echo " \"c_max\": \"$(get_max_arc_size)\"," >>$config
464-
echo " \"hostname\": \"$(uname -n)\"," >>$config
465-
echo " \"kernel version\": \"$(uname -sr)\"," >>$config
462+
awk '$1 == "Mem:" { print $2 }')\"," >>"$config"
463+
echo " \"c_max\": \"$(get_max_arc_size)\"," >>"$config"
464+
echo " \"hostname\": \"$(uname -n)\"," >>"$config"
465+
echo " \"kernel version\": \"$(uname -sr)\"," >>"$config"
466466
else
467467
dtrace -qn 'BEGIN{
468468
printf(" \"ncpus\": %d,\n", `ncpus);
469469
printf(" \"physmem\": %u,\n", `physmem * `_pagesize);
470470
printf(" \"c_max\": %u,\n", `arc_stats.arcstat_c_max.value.ui64);
471471
printf(" \"kmem_flags\": \"0x%x\",", `kmem_flags);
472-
exit(0)}' >>$config
473-
echo " \"hostname\": \"$(uname -n)\"," >>$config
474-
echo " \"kernel version\": \"$(uname -v)\"," >>$config
472+
exit(0)}' >>"$config"
473+
echo " \"hostname\": \"$(uname -n)\"," >>"$config"
474+
echo " \"kernel version\": \"$(uname -v)\"," >>"$config"
475475
fi
476476
if is_linux; then
477477
lsblk -dino NAME,SIZE | awk 'BEGIN {
478478
printf(" \"disks\": {\n"); first = 1}
479479
{disk = $1} {size = $2;
480480
if (first != 1) {printf(",\n")} else {first = 0}
481481
printf(" \"%s\": \"%s\"", disk, size)}
482-
END {printf("\n },\n")}' >>$config
482+
END {printf("\n },\n")}' >>"$config"
483483

484484
zfs_tunables="/sys/module/zfs/parameters"
485485

486-
printf " \"tunables\": {\n" >>$config
486+
printf " \"tunables\": {\n" >>"$config"
487487
for tunable in \
488488
zfs_arc_max \
489489
zfs_arc_sys_free \
@@ -500,28 +500,28 @@ function get_system_config
500500
do
501501
if [ "$tunable" != "zfs_arc_max" ]
502502
then
503-
printf ",\n" >>$config
503+
printf ",\n" >>"$config"
504504
fi
505505
printf " \"$tunable\": \"$(<$zfs_tunables/$tunable)\"" \
506-
>>$config
506+
>>"$config"
507507
done
508-
printf "\n }\n" >>$config
508+
printf "\n }\n" >>"$config"
509509
else
510510
iostat -En | awk 'BEGIN {
511511
printf(" \"disks\": {\n"); first = 1}
512512
/^c/ {disk = $1}
513513
/^Size: [^0]/ {size = $2;
514514
if (first != 1) {printf(",\n")} else {first = 0}
515515
printf(" \"%s\": \"%s\"", disk, size)}
516-
END {printf("\n },\n")}' >>$config
516+
END {printf("\n },\n")}' >>"$config"
517517

518518
sed -n 's/^set \(.*\)[ ]=[ ]\(.*\)/\1=\2/p' /etc/system | \
519519
awk -F= 'BEGIN {printf(" \"system\": {\n"); first = 1}
520520
{if (first != 1) {printf(",\n")} else {first = 0};
521521
printf(" \"%s\": %s", $1, $2)}
522-
END {printf("\n }\n")}' >>$config
522+
END {printf("\n }\n")}' >>"$config"
523523
fi
524-
echo "}" >>$config
524+
echo "}" >>"$config"
525525
}
526526

527527
#
@@ -535,33 +535,33 @@ function pool_to_lun_list
535535

536536
case "$UNAME" in
537537
Linux)
538-
ctds=$(zpool list -HLv $pool | \
538+
ctds=$(zpool list -HLv "$pool" | \
539539
awk '/sd[a-z]*|loop[0-9]*|dm-[0-9]*/ {print $1}')
540540

541541
for ctd in $ctds; do
542542
lun_list="$lun_list$ctd:"
543543
done
544544
;;
545545
FreeBSD)
546-
lun_list+=$(zpool list -HLv $pool | \
546+
lun_list+=$(zpool list -HLv "$pool" | \
547547
awk '/a?da[0-9]+|md[0-9]+|mfid[0-9]+|nda[0-9]+|nvd[0-9]+|vtbd[0-9]+/
548548
{ printf "%s:", $1 }')
549549
;;
550550
*)
551-
ctds=$(zpool list -v $pool |
551+
ctds=$(zpool list -v "$pool" |
552552
awk '/c[0-9]*t[0-9a-fA-F]*d[0-9]*/ {print $1}')
553553

554554
for ctd in $ctds; do
555555
# Get the device name as it appears in /etc/path_to_inst
556-
devname=$(readlink -f /dev/dsk/${ctd}s0 | sed -n 's/\/devices\([^:]*\):.*/\1/p')
556+
devname=$(readlink -f /dev/dsk/"${ctd}"s0 | sed -n 's/\/devices\([^:]*\):.*/\1/p')
557557
# Add a string composed of the driver name and instance
558558
# number to the list for comparison with dev_statname.
559559
lun=$(sed 's/"//g' /etc/path_to_inst | awk -v dn="$devname" '$0 ~ dn {print $3$2}')
560560
lun_list="$lun_list$lun:"
561561
done
562562
;;
563563
esac
564-
echo $lun_list
564+
echo "$lun_list"
565565
}
566566

567567
function print_perf_settings

0 commit comments

Comments
 (0)