Re: [libvirt] [PATCH] [TCK] network: create networks and check for exected results

On 11/01/2010 04:56 AM, Stefan Berger wrote: Actually, since you have already got the test setup ready, would you mind just squashing this in? If it works for you, then you have my ACK to push the squashed version, such that libvirt-tck.git only has a single commit with all the problems already fixed. This incremental diff to your patch converts `` to $(), fixes a few useless uses of cat, changes temporary file creation to instead use a secure temporary directory, ensures grep -E will either work or has a fallback of egrep, and adds "" around more variables that might be user-provided. diff --git i/scripts/networks/networkApplyTest.sh w/scripts/networks/networkApplyTest.sh index 547f00f..6627dd3 100644 --- i/scripts/networks/networkApplyTest.sh +++ w/scripts/networks/networkApplyTest.sh @@ -2,18 +2,19 @@ VIRSH=virsh -uri= -if [ "x${LIBVIRT_TCK_CONFIG}x" != "xx" ]; then - uri_exp=`cat ${LIBVIRT_TCK_CONFIG} | grep "^uri\s*=" | sed -e 's/uri\s*=\s*//' | tail -n 1` - if [ "x${uri_exp}x" != "xx" ]; then - eval "uri=${uri_exp}" - fi - if [ "x${uri}x" == "xx" ]; then - uri="qemu:///system" - fi -else - uri="qemu:///system" -fi +# For each line starting with uri=, remove the prefix and set the hold +# space to the rest of the line. Then at file end, print the hold +# space, which is effectively the last uri= line encountered. +uri=$(sed -n '/^uri[ ]*=[ ]*/ { + s/// + h +} +$ { + x + p +}' < "$LIBVIRT_TCK_CONFIG") +: "${uri:=qemu:///system}" + LIBVIRT_URI=${uri} @@ -72,29 +73,23 @@ function tap_final() { [ -n "${TAP_FAIL_LIST}" ] && echo "FAILED tests ${TAP_FAIL_LIST}" - okay=`echo "($TAP_TOT_CTR-$TAP_FAIL_CTR)*100/$TAP_TOT_CTR" | bc -l` + okay=$(echo "($TAP_TOT_CTR-$TAP_FAIL_CTR)*100/$TAP_TOT_CTR" | bc -l) echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${okay:0:5}% okay" } # A wrapper for mktemp in case it does not exist -# Echos the name of a temporary file. -function mktmpfile() { +# Echos the name of a secure temporary directory. +function mktmpdir() { local tmp - type -P mktemp > /dev/null - if [ $? -eq 0 ]; then - tmp=$(mktemp -t nwfvmtest.XXXXXX) - echo ${tmp} - else - while :; do - tmp="/tmp/nwfvmtest.${RANDOM}" - if [ ! -f ${tmp} ]; then - touch ${tmp} - chmod 666 ${tmp} - echo ${tmp} - break - fi - done - fi + { + tmp=$( (umask 077 && mktemp -d ./nwfvmtest.XXXXXX) 2>/dev/null) && + test -n "$tmp" && test -d "$tmp" + } || + { + tmp=./nwfvmtest$$-$RANDOM + (umask 077 && mkdir "$tmp") + } || { echo "failed to create secure temporary directory" >&2; exit 1; } + echo "${tmp}" return 0 } @@ -104,51 +99,56 @@ function checkExpectedOutput() { local datafile="$2" local flags="$3" local skipregex="$4" - local cmd line tmpfile tmpfile2 skip + local cmd line tmpdir tmpfile tmpfile2 skip - tmpfile=`mktmpfile` - tmpfile2=`mktmpfile` + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/file + tmpfile2=$tmpdir/file2 + + if echo a | grep -E '(a|b)' >/dev/null 2>&1 + then EGREP='grep -E' + else EGREP=egrep + fi - exec 4<${datafile} + exec 4<"${datafile}" read <&4 line="${REPLY}" while [ "x${line}x" != "xx" ]; do - cmd=`echo ${line##\#}` + cmd=$(echo "${line##\#}") skip=0 if [ "x${skipregex}x" != "xx" ]; then - skip=`echo ${cmd} | grep -c -E ${skipregex}` + skip=$(echo "${cmd}" | ${EGREP} -c ${skipregex}) fi - eval ${cmd} 2>&1 | tee ${tmpfile} 1>/dev/null + eval "${cmd}" 2>&1 | tee "${tmpfile}" 1>/dev/null - rm ${tmpfile2} 2>/dev/null - touch ${tmpfile2} + : >"{tmpfile2}" - while [ 1 ]; do + while :; do read <&4 line="${REPLY}" if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx" ]; then - if [ ${skip} -ne 0 ]; then - break - fi + if [ ${skip} -ne 0 ]; then + break + fi - diff ${tmpfile} ${tmpfile2} >/dev/null + diff "${tmpfile}" "${tmpfile2}" >/dev/null if [ $? -ne 0 ]; then if [ $((flags & FLAG_VERBOSE)) -ne 0 ]; then echo "FAIL ${xmlfile} : ${cmd}" - diff ${tmpfile} ${tmpfile2} + diff "${tmpfile}" "${tmpfile2}" fi ((failctr++)) if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "tmp files: $tmpfile, $tmpfile2" - echo "Press enter" - read + echo "Press enter" + read fi [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ test_result $((passctr+failctr)) "" 1 @@ -167,13 +167,13 @@ function checkExpectedOutput() { break fi - echo "${line}" >> ${tmpfile2} + echo "${line}" >> "${tmpfile2}" done done exec 4>&- - rm -rf "${tmpfile}" "${tmpfile2}" 2>/dev/null + rm -rf "${tmpdir}" } @@ -189,7 +189,7 @@ function doTest() { return 1 fi - netname=`cat "${xmlfile}" | sed -n 's/.*<name>\([[:print:]]*\)<.*/\1/p'` + netname=$(sed -n 's/.*<name>\([^<]*\)<.*/\1/p' "${xmlfile}") ${VIRSH} net-create "${xmlfile}" > /dev/null @@ -197,7 +197,7 @@ function doTest() { ${VIRSH} net-destroy "${netname}" > /dev/null - if [ -r ${postdatafile} ]; then + if [ -r "${postdatafile}" ]; then checkExpectedOutput "${xmlfile}" "${postdatafile}" "${flags}" "" fi @@ -212,22 +212,25 @@ function runTests() { local datafiles f c local tap_total=0 ctr=0 - pushd ${PWD} > /dev/null - cd ${hostoutdir} - datafiles=`ls *.dat` + pushd "${PWD}" > /dev/null + cd "${hostoutdir}" + datafiles=$(ls *.dat) popd > /dev/null if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then # Need to count the number of total tests for fil in ${datafiles}; do - c=$(grep -c "^#" ${hostoutdir}/${fil}) + c=$(grep -c "^#" "${hostoutdir}/${fil}") ((tap_total+=c)) ((ctr++)) done echo "1..${tap_total}" fi - for fil in `echo "${datafiles}" | grep -v "\.post\.dat$"`; do + for fil in $datafiles; do + case $fil in + *.post.dat) continue;; + esac f=${fil%%.dat} doTest "${xmldir}/${f}.xml" "${hostoutdir}/${fil}" \ "${hostoutdir}/${f}.post.dat" "${flags}" @@ -268,7 +271,7 @@ function main() { shift 1 done - if [ `uname` != "Linux" ]; then + if [ $(uname) != "Linux" ]; then if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then echo "1..0 # Skipped: Only valid on Linux hosts" else @@ -283,7 +286,7 @@ function main() { exit 0 fi - for name in `virsh list | awk '{print $2}'` + for name in $(virsh list | awk '{print $2}') do case ${name} in tck*) @@ -302,7 +305,7 @@ function main() { esac done - for name in `virsh net-list | sed -n '3,$p'` + for name in $(virsh net-list | sed -n '3,$p') do case ${name} in tck*) @@ -324,8 +327,8 @@ function main() { fi if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd ${PWD} > /dev/null - . test-lib.sh + pushd "${PWD}" > /dev/null + . ./test-lib.sh if [ $? -ne 0 ]; then exit 1 fi -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org