[libvirt] [PATCH] [TCK] Convert scripts to also run with dash
I am converting the shell-scripted tap tests to also run using the dash shell, fixing some other problems on the way as well. Two of the tests' data files contained '\\1' in an sed command. This created char(1) with the dash, '\1' with the bash. I found it easier to replace the sed command with a different one than figuring out how to solve this escape problem so it works for both shells. Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- scripts/networks/100-apply-verify-host.t | 9 scripts/networks/networkApplyTest.sh | 133 +++----- scripts/networks/networkxml2hostout/tck-testnet-1.dat | 2 scripts/networks/networkxml2hostout/tck-testnet-2.dat | 2 scripts/nwfilter/050-apply-verify-host.t | 9 scripts/nwfilter/nwfilter2vmtest.sh | 298 ++++++++---------- 6 files changed, 215 insertions(+), 238 deletions(-) Index: libvirt-tck/scripts/networks/networkApplyTest.sh =================================================================== --- libvirt-tck.orig/scripts/networks/networkApplyTest.sh +++ libvirt-tck/scripts/networks/networkApplyTest.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh VIRSH=virsh @@ -33,8 +33,8 @@ TAP_FAIL_LIST="" TAP_FAIL_CTR=0 TAP_TOT_CTR=0 -function usage() { - local cmd="$0" +usage() { + cmd="$0" cat <<EOF Usage: ${cmd} [--help|-h|-?] [--noattach] [--wait] [--verbose] [--libvirt-test] [--tap-test] @@ -56,37 +56,37 @@ EOF } -function tap_fail() { - echo "not ok $1 - ${2:0:66}" - TAP_FAIL_LIST+="$1 " - ((TAP_FAIL_CTR++)) - ((TAP_TOT_CTR++)) +tap_fail() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "not ok $1 - ${txt}" + TAP_FAIL_LIST="$TAP_FAIL_LIST $1 " + TAP_FAIL_CTR=$((TAP_FAIL_CTR+1)) + TAP_TOT_CTR=$((TAP_TOT_CTR+1)) } -function tap_pass() { - echo "ok $1 - ${2:0:70}" - ((TAP_TOT_CTR++)) +tap_pass() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "ok $1 - ${txt}" + TAP_TOT_CTR=$((TAP_TOT_CTR+1)) } -function tap_final() { - local okay - +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) - echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${okay:0:5}% okay" + txt=$(echo $okay | gawk '{print substr($0,1,5)}') + echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${txt}% okay" } # A wrapper for mktemp in case it does not exist # Echos the name of a secure temporary directory. -function mktmpdir() { - local tmp +mktmpdir() { { - tmp=$( (umask 077 && mktemp -d ./nwfvmtest.XXXXXX) 2>/dev/null) && + tmp=$( (umask 077 && mktemp -d ./nwtst.XXXXXX) 2>/dev/null) && test -n "$tmp" && test -d "$tmp" } || { - tmp=./nwfvmtest$$-$RANDOM + tmp=./nwtst$$-$RANDOM (umask 077 && mkdir "$tmp") } || { echo "failed to create secure temporary directory" >&2; exit 1; } echo "${tmp}" @@ -94,12 +94,11 @@ function mktmpdir() { } -function checkExpectedOutput() { - local xmlfile="$1" - local datafile="$2" - local flags="$3" - local skipregex="$4" - local cmd line tmpdir tmpfile tmpfile2 skip +checkExpectedOutput() { + xmlfile="$1" + datafile="$2" + flags="$3" + skipregex="$4" tmpdir=$(mktmpdir) tmpfile=$tmpdir/file @@ -112,8 +111,8 @@ function checkExpectedOutput() { exec 4<"${datafile}" - read <&4 - line="${REPLY}" + IFS="" + read line <&4 while [ "x${line}x" != "xx" ]; do cmd=$(echo "${line##\#}") @@ -128,10 +127,11 @@ function checkExpectedOutput() { : >"${tmpfile2}" while :; do - read <&4 - line="${REPLY}" + read line <&4 + + letter=$(echo ${line} | gawk '{print substr($1,1,1)}') - if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx" ]; then + if [ "x${letter}x" = "x#x" ] || [ "x${line}x" = "xx" ]; then if [ ${skip} -ne 0 ]; then break @@ -144,18 +144,18 @@ function checkExpectedOutput() { echo "FAIL ${xmlfile} : ${cmd}" diff "${tmpfile}" "${tmpfile2}" fi - ((failctr++)) + failctr=$((failctr+1)) if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "tmp files: $tmpfile, $tmpfile2" echo "Press enter" - read + read enter fi [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ test_result $((passctr+failctr)) "" 1 [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \ tap_fail $((passctr+failctr)) "${xmlfile} : ${cmd}" else - ((passctr++)) + passctr=$((passctr+1)) [ $((flags & FLAG_VERBOSE)) -ne 0 ] && \ echo "PASS ${xmlfile} : ${cmd}" [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ @@ -177,12 +177,11 @@ function checkExpectedOutput() { } -function doTest() { - local xmlfile="$1" - local datafile="$2" - local postdatafile="$3" - local flags="$4" - local netname +doTest() { + xmlfile="$1" + datafile="$2" + postdatafile="$3" + flags="$4" if [ ! -r "${xmlfile}" ]; then echo "FAIL : Cannot access filter XML file ${xmlfile}." @@ -205,24 +204,20 @@ function doTest() { } -function runTests() { - local xmldir="$1" - local hostoutdir="$2" - local flags="$3" - local datafiles f c - local tap_total=0 ctr=0 - - pushd "${PWD}" > /dev/null - cd "${hostoutdir}" - datafiles=$(ls *.dat) - popd > /dev/null +runTests() { + xmldir="$1" + hostoutdir="$2" + flags="$3" + tap_total=0 ctr=0 + + datafiles=$(cd "${hostoutdir}";ls *.dat) 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}") - ((tap_total+=c)) - ((ctr++)) + tap_total=$((tap_total+c)) + ctr=$((ctr+1)) done echo "1..${tap_total}" fi @@ -250,22 +245,20 @@ function runTests() { } -function main() { - local prgname="$0" - local vm1 vm2 - local xmldir="networkxml2xmlin" - local hostoutdir="networkxml2hostout" - local res rc - local flags +main() { + prgname="$0" + xmldir="networkxml2xmlin" + hostoutdir="networkxml2hostout" + flags=0 while [ $# -ne 0 ]; do case "$1" in --help|-h|-\?) usage ${prgname}; exit 0;; - --wait) ((flags |= FLAG_WAIT ));; - --verbose) ((flags |= FLAG_VERBOSE ));; - --libvirt-test) ((flags |= FLAG_LIBVIRT_TEST ));; - --tap-test) ((flags |= FLAG_TAP_TEST ));; - --force) ((flags |= FLAG_FORCE_CLEAN ));; + --wait) flags=$((flags | FLAG_WAIT ));; + --verbose) flags=$((flags | FLAG_VERBOSE ));; + --libvirt-test) flags=$((flags | FLAG_LIBVIRT_TEST ));; + --tap-test) flags=$((flags | FLAG_TAP_TEST ));; + --force) flags=$((flags | FLAG_FORCE_CLEAN ));; *) usage ${prgname}; exit 1;; esac shift 1 @@ -290,8 +283,8 @@ function main() { do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" ] || \ + [ $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh destroy ${name} 2>&1) res=$(virsh undefine ${name} 2>&1) if [ $? -ne 0 ]; then @@ -309,8 +302,8 @@ function main() { do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" ] || \ + [ $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh net-destroy ${name} 2>&1) rc=$? res=$(virsh net-undefine ${name} 2>&1) @@ -327,13 +320,13 @@ function main() { fi if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd "${PWD}" > /dev/null + curdir="${PWD}" . ./test-lib.sh if [ $? -ne 0 ]; then exit 1 fi test_intro $this_test - popd > /dev/null + cd "${curdir}" fi res=$(${VIRSH} capabilities 2>&1) Index: libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh =================================================================== --- libvirt-tck.orig/scripts/nwfilter/nwfilter2vmtest.sh +++ libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh ORIG_IFNAME="vnet0" ATTACH_IFNAME="attach0" @@ -6,17 +6,20 @@ TESTFILTERNAME="nwfiltertestfilter" TESTVM2FWALLDATA="nwfilterxml2fwallout/testvm.fwall.dat" 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 -else - uri="qemu:///system" -fi -LIBVIRT_URI=${uri} +# 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} FLAG_WAIT="$((1<<0))" FLAG_ATTACH="$((1<<1))" @@ -34,8 +37,8 @@ TAP_FAIL_LIST="" TAP_FAIL_CTR=0 TAP_TOT_CTR=0 -function usage() { - local cmd="$0" +usage() { + cmd="$0" cat <<EOF Usage: ${cmd} [--help|-h|-?] [--noattach] [--wait] [--verbose] [--libvirt-test] [--tap-test] @@ -60,66 +63,61 @@ EOF } -function tap_fail() { - echo "not ok $1 - ${2:0:66}" - TAP_FAIL_LIST+="$1 " - ((TAP_FAIL_CTR++)) - ((TAP_TOT_CTR++)) +tap_fail() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "not ok $1 - ${txt}" + TAP_FAIL_LIST="$TAP_FAIL_LIST $1 " + TAP_FAIL_CTR=$((TAP_FAIL_CTR+1)) + TAP_TOT_CTR=$((TAP_TOT_CTR+1)) } -function tap_pass() { - echo "ok $1 - ${2:0:70}" - ((TAP_TOT_CTR++)) +tap_pass() { + txt=$(echo "$2" | gawk '{print substr($0,1,70)}') + echo "ok $1 - ${txt}" + TAP_TOT_CTR=$((TAP_TOT_CTR+1)) } -function tap_final() { - local okay - +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` - echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${okay:0:5}% okay" + txt=$(echo $okay | gawk '{print substr($0,1,5)}') + echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${txt}% okay" } # A wrapper for mktemp in case it does not exist # Echos the name of a temporary file. -function mktmpfile() { - 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 +mktmpdir() { + { + 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 } -function checkExpectedOutput() { - local xmlfile="$1" - local fwallfile="$2" - local ifname="$3" - local flags="$4" - local skipregex="$5" - local regex="s/${ORIG_IFNAME}/${ifname}/g" - local cmd line tmpfile tmpfile2 skip - - tmpfile=`mktmpfile` - tmpfile2=`mktmpfile` +checkExpectedOutput() { + xmlfile="$1" + fwallfile="$2" + ifname="$3" + flags="$4" + skipregex="$5" + regex="s/${ORIG_IFNAME}/${ifname}/g" + + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/file + tmpfile2=$tmpdir/file2 exec 4<${fwallfile} - read <&4 - line="${REPLY}" + IFS="" + + read line <&4 while [ "x${line}x" != "xx" ]; do cmd=`echo ${line##\#} | sed ${regex}` @@ -135,10 +133,11 @@ function checkExpectedOutput() { touch ${tmpfile2} while [ 1 ]; do - read <&4 - line="${REPLY}" + read line <&4 - if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx" ]; then + letter=$(echo $line | gawk '{print substr($1,1,1)}') + + if [ "x${letter}x" = "x#x" ] || [ "x${line}x" = "xx" ]; then if [ ${skip} -ne 0 ]; then break @@ -151,18 +150,18 @@ function checkExpectedOutput() { echo "FAIL ${xmlfile} : ${cmd}" diff ${tmpfile} ${tmpfile2} fi - ((failctr++)) + failctr=$((failctr+1)) if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "tmp files: $tmpfile, $tmpfile2" echo "Press enter" - read + read enter fi [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ test_result $((passctr+failctr)) "" 1 [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \ tap_fail $((passctr+failctr)) "${xmlfile} : ${cmd}" else - ((passctr++)) + passctr=$((passctr+1)) [ $((flags & FLAG_VERBOSE)) -ne 0 ] && \ echo "PASS ${xmlfile} : ${cmd}" [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ @@ -180,19 +179,18 @@ function checkExpectedOutput() { exec 4>&- - rm -rf "${tmpfile}" "${tmpfile2}" 2>/dev/null + rm -rf "${tmpdir}" } -function doTest() { - local xmlfile="$1" - local fwallfile="$2" - local vm1name="$3" - local vm2name="$4" - local flags="$5" - local testnum="$6" - local linenums ctr=0 - local tmpfile b msg rc +doTest() { + xmlfile="$1" + fwallfile="$2" + vm1name="$3" + vm2name="$4" + flags="$5" + testnum="$6" + ctr=0 if [ ! -r "${xmlfile}" ]; then echo "FAIL : Cannot access filter XML file ${xmlfile}." @@ -209,7 +207,8 @@ function doTest() { if [ $((flags & FLAG_ATTACH)) -ne 0 ]; then - tmpfile=`mktmpfile` + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/tmpfile b=`{ ${VIRSH} dumpxml ${vm1name} | tr -d "\n"; echo; } | \ sed "s/.*\<interface.*source bridge='\([a-zA-Z0-9_]\+\)'.*<\/interface>.*/\1/"` @@ -225,7 +224,7 @@ EOF msg=`${VIRSH} attach-device "${vm1name}" "${tmpfile}" > /dev/null` rc=$? - ((attachctr++)) + attachctr=$((attachctr+1)) if [ $rc -eq 0 ]; then checkExpectedOutput "${xmlfile}" "${fwallfile}" "${ATTACH_IFNAME}" \ @@ -245,48 +244,45 @@ EOF checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \ "${vm2name}" "${flags}" #"(PRE|POST)ROUTING" fi - - ((attachfailctr++)) + + attachfailctr=$((attachfailctr+1)) if [ $((flags & FLAG_VERBOSE)) -ne 0 ]; then echo "FAIL: Could not attach interface to vm ${vm1name}." if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter" - read + read enter fi fi fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} fi return 0 } -function runTests() { - local vm1name="$1" - local vm2name="$2" - local xmldir="$3" - local fwalldir="$4" - local flags="$5" - local fwallfiles f c - local tap_total=0 ctr=0 - - pushd ${PWD} > /dev/null - cd ${fwalldir} - fwallfiles=`ls *.fwall` - popd > /dev/null +runTests() { + vm1name="$1" + vm2name="$2" + xmldir="$3" + fwalldir="$4" + flags="$5" + tap_total=0 + ctr=0 + + fwallfiles=$(cd ${fwalldir}; ls *.fwall) if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then # Need to count the number of total tests for fil in ${fwallfiles}; do c=$(grep -c "^#" ${fwalldir}/${fil}) - ((tap_total+=c)) - ((ctr++)) + tap_total=$((tap_total+c)) + ctr=$((ctr+1)) done c=$(grep -c "^#" "${TESTVM2FWALLDATA}") - ((tap_total+=c*ctr)) - [ $((flags & FLAG_ATTACH)) -ne 0 ] && ((tap_total*=2)) + tap_total=$((tap_total+c*ctr)) + [ $((flags & FLAG_ATTACH)) -ne 0 ] && tap_total=$((tap_total*2)) echo "1..${tap_total}" fi @@ -310,14 +306,14 @@ function runTests() { } -function createVM() { - local vmname="$1" - local filtername="$2" - local ipaddr="$3" - local macaddr="$4" - local flags="$5" - local res - local tmpfile='mktmpfile' +createVM() { + vmname="$1" + filtername="$2" + ipaddr="$3" + macaddr="$4" + flags="$5" + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/tmpfile cat > ${tmpfile} << EOF <domain type='kvm'> @@ -365,7 +361,7 @@ EOF echo "Could not start VM ${vmname} : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi $(${VIRSH} undefine ${vmname}) return 1 @@ -373,23 +369,22 @@ EOF [ $((flags & FLAG_VERBOSE)) -ne 0 ] && echo "Created VM ${vmname}." - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 0 } -function destroyVM() { - local vmname="$1" - local flags="$2" - local res +destroyVM() { + vmname="$1" + flags="$2" res=$(${VIRSH} destroy ${vmname}) if [ $? -ne 0 ]; then echo "Could not destroy VM ${vmname} : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -399,7 +394,7 @@ function destroyVM() { echo "Could not undefine VM ${vmname} : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -410,10 +405,10 @@ function destroyVM() { } -function createTestFilters() { - local flags="$1" - local tmpfile=`mktmpfile` - local res +createTestFilters() { + flags="$1" + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/tmpfile cat >${tmpfile} << EOF <filter name="${TESTFILTERNAME}"> @@ -433,9 +428,9 @@ EOF echo "Could not define filter : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 1 fi @@ -448,28 +443,27 @@ EOF echo "Could not define filter : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 1 fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 0 } -function deleteTestFilter() { - local flags="$1" - local res +deleteTestFilter() { + flags="$1" res=$(${VIRSH} nwfilter-undefine ${TESTFILTERNAME} 2>&1) if [ $? -ne 0 ]; then echo "Could not undefine filter : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -478,7 +472,7 @@ function deleteTestFilter() { echo "Could not undefine filter : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -486,27 +480,25 @@ function deleteTestFilter() { } -function main() { - local prgname="$0" - local vm1 vm2 - local xmldir="nwfilterxml2xmlin" - local fwalldir="nwfilterxml2fwallout" - local found=0 vms res - local filtername="tck-testcase" - local libvirtdpid=-1 - local flags OPWD +main() { + prgname="$0" + xmldir="nwfilterxml2xmlin" + fwalldir="nwfilterxml2fwallout" + found=0 + filtername="tck-testcase" + libvirtdpid=-1 - ((flags=${FLAG_ATTACH})) + flags=${FLAG_ATTACH} while [ $# -ne 0 ]; do case "$1" in --help|-h|-\?) usage ${prgname}; exit 0;; - --noattach) ((flags &= ~FLAG_ATTACH ));; - --wait) ((flags |= FLAG_WAIT ));; - --verbose) ((flags |= FLAG_VERBOSE ));; - --libvirt-test) ((flags |= FLAG_LIBVIRT_TEST ));; - --tap-test) ((flags |= FLAG_TAP_TEST ));; - --force) ((flags |= FLAG_FORCE_CLEAN ));; + --noattach) flags=$((flags & ~FLAG_ATTACH));; + --wait) flags=$((flags | FLAG_WAIT ));; + --verbose) flags=$((flags | FLAG_VERBOSE ));; + --libvirt-test) flags=$((flags | FLAG_LIBVIRT_TEST ));; + --tap-test) flags=$((flags | FLAG_TAP_TEST ));; + --force) flags=$((flags | FLAG_FORCE_CLEAN ));; *) usage ${prgname}; exit 1;; esac shift 1 @@ -527,15 +519,17 @@ function main() { exit 0 fi - for name in `virsh list | awk '{print $2}'` + for name in `virsh list --all | awk '{print $2}'` do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" = "x1" ] || \ + [ $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh destroy ${name} 2>&1) + rc1=$? res=$(virsh undefine ${name} 2>&1) - if [ $? -ne 0 ]; then + rc2=$? + if [ $rc1 -ne 0 ] && [ $rc2 -ne 0 ]; then echo "Bail out! Could not undefine nwfiler ${name}: ${res}" exit 0 fi @@ -550,15 +544,15 @@ function main() { do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" = "x1" ] || \ + [ $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh nwfilter-undefine ${name} 2>&1) if [ $? -ne 0 ]; then - echo "Bail out! Could not undefine domain ${name}: ${res}" + echo "Bail out! Could not undefine filter ${name}: ${res}" exit 1 fi else - echo "Bail out! Domain ${name} already exists, use --force to clean" + echo "Bail out! Filter ${name} already exists, use --force to clean" exit 1 fi esac @@ -566,19 +560,19 @@ function main() { fi if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd ${PWD} > /dev/null + curdir="${PWD}" . test-lib.sh if [ $? -ne 0 ]; then exit 1 fi test_intro $this_test - popd > /dev/null + cd "${curdir}" fi res=$(${VIRSH} capabilities 2>&1) - vm1="tck-testvm${RANDOM}" - vm2="tck-testvm${RANDOM}" + vm1="tck-test$$1" + vm2="tck-test$$2" createTestFilters "${flags}" if [ $? -ne 0 ]; then Index: libvirt-tck/scripts/networks/100-apply-verify-host.t =================================================================== --- libvirt-tck.orig/scripts/networks/100-apply-verify-host.t +++ libvirt-tck/scripts/networks/100-apply-verify-host.t @@ -1,10 +1,5 @@ -#!/bin/bash +#!/bin/sh pwd=$(dirname $0) -pushd ${PWD} > /dev/null - -cd ${pwd} -bash ./networkApplyTest.sh --tap-test - -popd > /dev/null +(cd ${pwd}; sh ./networkApplyTest.sh --tap-test) Index: libvirt-tck/scripts/nwfilter/050-apply-verify-host.t =================================================================== --- libvirt-tck.orig/scripts/nwfilter/050-apply-verify-host.t +++ libvirt-tck/scripts/nwfilter/050-apply-verify-host.t @@ -1,10 +1,5 @@ -#!/bin/bash +#!/bin/sh pwd=$(dirname $0) -pushd ${PWD} > /dev/null - -cd ${pwd} -bash ./nwfilter2vmtest.sh --tap-test --noattach - -popd > /dev/null \ No newline at end of file +(cd ${pwd}; sh ./nwfilter2vmtest.sh --tap-test --noattach) Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-1.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat @@ -5,7 +5,7 @@ MASQUERADE all -- 10.1.2.0/24 #iptables -n -L FORWARD | grep ' 10\\.1\\.2\\.' ACCEPT all -- 0.0.0.0/0 10.1.2.0/24 state RELATED,ESTABLISHED ACCEPT all -- 10.1.2.0/24 0.0.0.0/0 -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\\(dnsmasq[[:print:]*]\\)|\\1|p' +#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*dnsmasq|dnsmasq|p' dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/tck-testnet.pid --conf-file= --listen-address 10.1.2.1 --except-interface lo --dhcp-range 10.1.2.2,10.1.2.254 --dhcp-lease-max=253 --dhcp-no-override #route -n | grep '10\\.1\\.2\\.' 10.1.2.0 0.0.0.0 255.255.255.0 U 0 0 0 tck-testbr Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-2.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-2.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-2.dat @@ -2,7 +2,7 @@ ACCEPT all -- 0.0.0.0/0 10.1.2.0/24 ACCEPT all -- 10.1.2.0/24 0.0.0.0/0 #iptables -t nat -L -n | grep ' 10\\.1\\.2\\.' -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\\(dnsmasq[[:print:]*]\\)|\\1|p' +#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*dnsmasq|dnsmasq|p' dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/tck-testnet.pid --conf-file= --listen-address 10.1.2.1 --except-interface lo --dhcp-range 10.1.2.2,10.1.2.254 --dhcp-lease-max=253 --dhcp-no-override #route -n | grep '10\\.1\\.2\\.' 10.1.2.0 0.0.0.0 255.255.255.0 U 0 0 0 tck-testbr
On 11/15/2010 08:33 AM, Stefan Berger wrote:
I am converting the shell-scripted tap tests to also run using the dash shell, fixing some other problems on the way as well.
Two of the tests' data files contained '\\1' in an sed command. This created char(1) with the dash, '\1' with the bash. I found it easier to replace the sed command with a different one than figuring out how to solve this escape problem so it works for both shells.
-function tap_fail() { - echo "not ok $1 - ${2:0:66}" - TAP_FAIL_LIST+="$1 " - ((TAP_FAIL_CTR++)) - ((TAP_TOT_CTR++)) +tap_fail() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "not ok $1 - ${txt}" + TAP_FAIL_LIST="$TAP_FAIL_LIST $1 " + TAP_FAIL_CTR=$((TAP_FAIL_CTR+1))
Older versions of dash have a bug where you have to pre-expand a variable to get this to work: TAP_FAIL_CTR=$(($TAP_FAIL_CTR + 1)) (throughout the patch)
@@ -112,8 +111,8 @@ function checkExpectedOutput() {
exec 4<"${datafile}"
- read <&4 - line="${REPLY}" + IFS=""
Where do you restore IFS back to its original space-tab-newline setting?
while :; do - read <&4 - line="${REPLY}" + read line <&4 + + letter=$(echo ${line} | gawk '{print substr($1,1,1)}')
- if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx" ]; then + if [ "x${letter}x" = "x#x" ] || [ "x${line}x" = "xx" ]; then
Rather than wasting a $() and gawk subprocess to compute $letter, you can use the much more efficient: case $line in '#'*) # line started with # ;; *) # line did not start with # ;; esac
@@ -327,13 +320,13 @@ function main() { fi
if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd "${PWD}" > /dev/null + curdir="${PWD}" . ./test-lib.sh if [ $? -ne 0 ]; then exit 1 fi test_intro $this_test - popd > /dev/null + cd "${curdir}"
You really should check whether cd succeeds; as failure to return to the parent directory deserves aborting the script to avoid corrupting unintended files in a different directory.
Index: libvirt-tck/scripts/networks/100-apply-verify-host.t =================================================================== --- libvirt-tck.orig/scripts/networks/100-apply-verify-host.t +++ libvirt-tck/scripts/networks/100-apply-verify-host.t @@ -1,10 +1,5 @@ -#!/bin/bash +#!/bin/sh
pwd=$(dirname $0)
-pushd ${PWD} > /dev/null - -cd ${pwd} -bash ./networkApplyTest.sh --tap-test - -popd > /dev/null +(cd ${pwd}; sh ./networkApplyTest.sh --tap-test)
To be robust to starting the script via a relative pathname that starts with -, I'd write this as: pwd=$(dirname -- "$0") (cd -- "${pwd}" && sh ./networkApplyTest.sh --tap-test
Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-1.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat @@ -5,7 +5,7 @@ MASQUERADE all -- 10.1.2.0/24 #iptables -n -L FORWARD | grep ' 10\\.1\\.2\\.' ACCEPT all -- 0.0.0.0/0 10.1.2.0/24 state RELATED,ESTABLISHED ACCEPT all -- 10.1.2.0/24 0.0.0.0/0 -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\\(dnsmasq[[:print:]*]\\)|\\1|p' +#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*dnsmasq|dnsmasq|p'
Hmm; you used 'read' to read in these lines, but unless you use 'read -r', that means that backslash interpolation is taking place in the shell. 'read -r' is portable to POSIX (and therefore dash), even if it is not portable to generic /bin/sh (but we already have lots of other things that are not portable to generic /bin/sh that aren't worth fretting about, since we can assume that the nwfilter tests only make sense on Linux where /bin/sh will mostly conform to POSIX). Maybe it's better to fix the driver scripts to use 'read -r', at which point these lines could use single \, while at the same time working around the problem you were seeing where dash interpolated \1 inside "" into char(1). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 11/15/2010 12:35 PM, Eric Blake wrote:
On 11/15/2010 08:33 AM, Stefan Berger wrote:
I am converting the shell-scripted tap tests to also run using the dash shell, fixing some other problems on the way as well.
Two of the tests' data files contained '\\1' in an sed command. This created char(1) with the dash, '\1' with the bash. I found it easier to replace the sed command with a different one than figuring out how to solve this escape problem so it works for both shells.
-function tap_fail() { - echo "not ok $1 - ${2:0:66}" - TAP_FAIL_LIST+="$1 " - ((TAP_FAIL_CTR++)) - ((TAP_TOT_CTR++)) +tap_fail() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "not ok $1 - ${txt}" + TAP_FAIL_LIST="$TAP_FAIL_LIST $1 " + TAP_FAIL_CTR=$((TAP_FAIL_CTR+1)) Older versions of dash have a bug where you have to pre-expand a variable to get this to work:
TAP_FAIL_CTR=$(($TAP_FAIL_CTR + 1))
(throughout the patch)
Fixed.
@@ -112,8 +111,8 @@ function checkExpectedOutput() {
exec 4<"${datafile}"
- read<&4 - line="${REPLY}" + IFS="" Where do you restore IFS back to its original space-tab-newline setting?
Fixed.
while :; do - read<&4 - line="${REPLY}" + read line<&4 + + letter=$(echo ${line} | gawk '{print substr($1,1,1)}')
- if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx" ]; then + if [ "x${letter}x" = "x#x" ] || [ "x${line}x" = "xx" ]; then
Rather than wasting a $() and gawk subprocess to compute $letter, you can use the much more efficient:
case $line in '#'*) # line started with # ;; *) # line did not start with # ;; esac
Ok, since it's faster.
@@ -327,13 +320,13 @@ function main() { fi
if [ $((flags& FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd "${PWD}"> /dev/null + curdir="${PWD}" . ./test-lib.sh if [ $? -ne 0 ]; then exit 1 fi test_intro $this_test - popd> /dev/null + cd "${curdir}" You really should check whether cd succeeds; as failure to return to the parent directory deserves aborting the script to avoid corrupting unintended files in a different directory.
Fixed.
Index: libvirt-tck/scripts/networks/100-apply-verify-host.t =================================================================== --- libvirt-tck.orig/scripts/networks/100-apply-verify-host.t +++ libvirt-tck/scripts/networks/100-apply-verify-host.t @@ -1,10 +1,5 @@ -#!/bin/bash +#!/bin/sh
pwd=$(dirname $0)
-pushd ${PWD}> /dev/null - -cd ${pwd} -bash ./networkApplyTest.sh --tap-test - -popd> /dev/null +(cd ${pwd}; sh ./networkApplyTest.sh --tap-test) To be robust to starting the script via a relative pathname that starts with -, I'd write this as:
pwd=$(dirname -- "$0") (cd -- "${pwd}"&& sh ./networkApplyTest.sh --tap-test
Fixed.
Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-1.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat @@ -5,7 +5,7 @@ MASQUERADE all -- 10.1.2.0/24 #iptables -n -L FORWARD | grep ' 10\\.1\\.2\\.' ACCEPT all -- 0.0.0.0/0 10.1.2.0/24 state RELATED,ESTABLISHED ACCEPT all -- 10.1.2.0/24 0.0.0.0/0 -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\\(dnsmasq[[:print:]*]\\)|\\1|p' +#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*dnsmasq|dnsmasq|p' Hmm; you used 'read' to read in these lines, but unless you use 'read -r', that means that backslash interpolation is taking place in the shell. 'read -r' is portable to POSIX (and therefore dash), even if it is not portable to generic /bin/sh (but we already have lots of other things that are not portable to generic /bin/sh that aren't worth fretting about, since we can assume that the nwfilter tests only make sense on Linux where /bin/sh will mostly conform to POSIX). Maybe it's better to fix the driver scripts to use 'read -r', at which point these lines could use single \, while at the same time working around the problem you were seeing where dash interpolated \1 inside "" into char(1).
I tried this now and converted all \\ to \. I still get the same problem here with dash using 'read -r line' now. The same problem exists with echo "\1" or echo '\1' run via bash or dash. Bash prints out '\1' and dash (0.5.6-2.fc14) prints char(1) in both cases. Stefan
On 11/15/2010 11:24 AM, Stefan Berger wrote:
I tried this now and converted all \\ to \. I still get the same problem here with dash using 'read -r line' now. The same problem exists with
echo "\1"
or
echo '\1'
Ah - the plot thickens, and the light bulb goes on for me. It's not just a problem with read, but also with your use of 'echo' to try and replay a just-read string. 'echo' and '\' don't mix. You have to use printf for any chance of portability. printf %s\\n "\1" printf %s\\n '\1'
run via bash or dash. Bash prints out '\1' and dash (0.5.6-2.fc14) prints char(1) in both cases.
Yes, because dash's echo is POSIX-compliant by default, and bash's is (intentionally) not. 'shopt -s xpg_echo' in bash to get bash behaving more like POSIX in automatically interpolating \, although you'll still get a difference in whether \1 gets interpolated to char(1), since POSIX only requires interpolation of \0nnn (dash's interpolation of \1 is an extension): $ dash -c "echo '"'\1'\' | od -tx1 0000000 01 0a 0000002 $ shopt -s xpg_echo $ echo '\1' \1 $ echo '\041' ! $ shopt -u xpg_echo $ echo '\041' \041 ke@office (0) ~/libvirt $ dash -c "echo '"'\041'\' ! $ dash -c 'printf %s\\n "\\041"' \041 -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 11/15/2010 01:50 PM, Eric Blake wrote:
I tried this now and converted all \\ to \. I still get the same problem here with dash using 'read -r line' now. The same problem exists with
echo "\1"
or
echo '\1' Ah - the plot thickens, and the light bulb goes on for me. It's not just a problem with read, but also with your use of 'echo' to try and replay a just-read string. 'echo' and '\' don't mix. You have to use
On 11/15/2010 11:24 AM, Stefan Berger wrote: printf for any chance of portability.
printf %s\\n "\1" printf %s\\n '\1' I am surprised that dash doesn't take the opportunity to already extend "\1" or '\1' to char(1) in these cases. A mystery... :-)
I'll fix it. Thanks. Stefan
On 11/15/2010 12:08 PM, Stefan Berger wrote:
On 11/15/2010 01:50 PM, Eric Blake wrote:
I tried this now and converted all \\ to \. I still get the same problem here with dash using 'read -r line' now. The same problem exists with
echo "\1"
or
echo '\1' Ah - the plot thickens, and the light bulb goes on for me. It's not just a problem with read, but also with your use of 'echo' to try and replay a just-read string. 'echo' and '\' don't mix. You have to use
On 11/15/2010 11:24 AM, Stefan Berger wrote: printf for any chance of portability.
printf %s\\n "\1" printf %s\\n '\1' I am surprised that dash doesn't take the opportunity to already extend "\1" or '\1' to char(1) in these cases. A mystery... :-)
As a shell argument in isolation, "\1" is undefined in POSIX, but "\\1" and '\1' are both the two-character strings for \ and 1. Then, add printf or echo into the mix. echo is required to do further interpolation of some \ sequences (although not all shells enforce this by default), but not all sequences. For echo, dash does some non-required interpolations that bash in xpg_echo mode does not do; but for printf, both bash and dash provide non-required interpolations. Printf is only required to do \ interpolation of its first argument, and of any subsequent arguments that line up with %b in the first argument (and what's worse, the interpolations required of the two positions of printf arguments are different). Therefore: echo '\1' => undefined by POSIX (bash with xpg_echo outputs \1, bash in normal mode outputs \1, dash outputs char(1)) echo '\\1' => defined by POSIX, but not portable (bash with xpg_echo outputs \1, bash in normal mode outputs \\1, dash outputs \1) echo "\1" => doubly undefined by POSIX echo "\\1" => same problem as echo '\1' echo '\01' => defined by POSIX, but not portable (bash with xpg_echo outputs char(1), bash in normal mode outputs \01, dash outputs char(1)) printf %s\\n '\1' => well-defined as \1 everywhere printf %s\\n "\1" => undefined by POSIX, although it tends to portably output \1 printf %s\\n "\\1" => well-defined as \1 everywhere printf '\1' => well-defined as char(1) everywhere printf %b '\1' => undefined by POSIX, although it tends to portably output char(1) printf %b '\01' => well-defined as char(1) everywhere printf %b "\1" => undefined by POSIX, although it tends to portably output char(1) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 11/15/2010 02:24 PM, Eric Blake wrote:
On 11/15/2010 12:08 PM, Stefan Berger wrote:
On 11/15/2010 01:50 PM, Eric Blake wrote:
I tried this now and converted all \\ to \. I still get the same problem here with dash using 'read -r line' now. The same problem exists with
echo "\1"
or
echo '\1' Ah - the plot thickens, and the light bulb goes on for me. It's not just a problem with read, but also with your use of 'echo' to try and replay a just-read string. 'echo' and '\' don't mix. You have to use
On 11/15/2010 11:24 AM, Stefan Berger wrote: printf for any chance of portability.
printf %s\\n "\1" printf %s\\n '\1' I am surprised that dash doesn't take the opportunity to already extend "\1" or '\1' to char(1) in these cases. A mystery... :-) As a shell argument in isolation, "\1" is undefined in POSIX, but "\\1" and '\1' are both the two-character strings for \ and 1.
Then, add printf or echo into the mix. echo is required to do further interpolation of some \ sequences (although not all shells enforce this by default), but not all sequences. For echo, dash does some non-required interpolations that bash in xpg_echo mode does not do; but for printf, both bash and dash provide non-required interpolations. Printf is only required to do \ interpolation of its first argument, and of any subsequent arguments that line up with %b in the first argument (and what's worse, the interpolations required of the two positions of printf arguments are different).
Therefore:
echo '\1' => undefined by POSIX (bash with xpg_echo outputs \1, bash in normal mode outputs \1, dash outputs char(1)) echo '\\1' => defined by POSIX, but not portable (bash with xpg_echo outputs \1, bash in normal mode outputs \\1, dash outputs \1) echo "\1" => doubly undefined by POSIX echo "\\1" => same problem as echo '\1' echo '\01' => defined by POSIX, but not portable (bash with xpg_echo outputs char(1), bash in normal mode outputs \01, dash outputs char(1)) printf %s\\n '\1' => well-defined as \1 everywhere printf %s\\n "\1" => undefined by POSIX, although it tends to portably output \1 printf %s\\n "\\1" => well-defined as \1 everywhere printf '\1' => well-defined as char(1) everywhere printf %b '\1' => undefined by POSIX, although it tends to portably output char(1) printf %b '\01' => well-defined as char(1) everywhere printf %b "\1" => undefined by POSIX, although it tends to portably output char(1)
I knew I was operating right on the edge of the POSIX standard when I started this script cleanup... :-) Stefan
On 16/11/2010, at 6:08 AM, Stefan Berger wrote: <snip>
Ah - the plot thickens, and the light bulb goes on for me. It's not just a problem with read, but also with your use of 'echo' to try and replay a just-read string. 'echo' and '\' don't mix. You have to use printf for any chance of portability.
printf %s\\n "\1" printf %s\\n '\1' I am surprised that dash doesn't take the opportunity to already extend "\1" or '\1' to char(1) in these cases. A mystery... :-)
I'll fix it. Thanks.
We seem to have recurring problems with finding portable commands, that work across everything. Would use an alternative approach, such as having our own wrapper functions, be more reliable? (conceptually like this) function replace_stuff() { if "$SHELL" = "dash" dash specific sed here elsif "$SHELL = bash" bash specific sed here else sh fallback sed here fi } # main code replace_stuff ("foo" "bar") ?
On 11/15/2010 12:29 PM, Justin Clift wrote:
On 16/11/2010, at 6:08 AM, Stefan Berger wrote: <snip>
Ah - the plot thickens, and the light bulb goes on for me. It's not just a problem with read, but also with your use of 'echo' to try and replay a just-read string. 'echo' and '\' don't mix. You have to use printf for any chance of portability.
printf %s\\n "\1" printf %s\\n '\1' I am surprised that dash doesn't take the opportunity to already extend "\1" or '\1' to char(1) in these cases. A mystery... :-)
I'll fix it. Thanks.
We seem to have recurring problems with finding portable commands, that work across everything.
Actually, it seems more like the problem is that the portability issues are known and even documented in places like autoconf's manual, but there are so many portability rules that not everyone knows them off the top of their head (myself included), so we end up relearning the issues every time they bite us. POSIX has helped tremendously; just ask anyone about the hoops they had to jump through for portability 15 years ago. But then you remember that not everyone implements POSIX correctly yet, and it's easy to see why it's a perpetual battle.
Would use an alternative approach, such as having our own wrapper functions, be more reliable?
(conceptually like this)
function replace_stuff() { if "$SHELL" = "dash" dash specific sed here elsif "$SHELL = bash" bash specific sed here else sh fallback sed here
Here, it's not the sed that was the problem, but the use of 'echo' mixed with backslashes. Fix the shell code to output unambiguous data by using printf instead, and a single sed script suffices. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake -
Justin Clift -
Stefan Berger