[libvirt] [PATCH] [TCK] nwfilter: add a test case using concurrency

Now that the existing scripts are (hopefully) cleaned up and my POSIX compliancy shell-scripting skills have temporarily :-) improved, I am now adding a test case that exercises concurrency. The test case creates and destroys 2 VMs concurrently as well as changes their referenced filters in a tight loop. This kind of test previously uncovered - deadlocks in libvirt due to lock-ordering in the nwfilter subsystem - libvirt termination bug in libaugeas due to doubly closed file descriptors and the side effects All of these have been fixed recently. The test script is known to run in bash, dash and ksh shells. Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- scripts/nwfilter/060-concurrency.t | 5 scripts/nwfilter/concurrency/chg-vm-filter.sh | 19 + scripts/nwfilter/concurrency/start-destroy-vm.sh | 19 + scripts/nwfilter/concurrency/tck-vm1-filter1.xml | 26 + scripts/nwfilter/concurrency/tck-vm1-filter2.xml | 26 + scripts/nwfilter/concurrency/tck-vm1.xml | 30 + scripts/nwfilter/concurrency/tck-vm2-filter1.xml | 26 + scripts/nwfilter/concurrency/tck-vm2-filter2.xml | 26 + scripts/nwfilter/concurrency/tck-vm2.xml | 30 + scripts/nwfilter/nwfilter_concurrent.sh | 371 +++++++++++++++++++++++ 10 files changed, 578 insertions(+) Index: libvirt-tck/scripts/nwfilter/060-concurrency.t =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/060-concurrency.t @@ -0,0 +1,5 @@ +#!/bin/sh + +pwd=$(dirname -- "$0") + +(cd -- "${pwd}"; sh ./nwfilter_concurrent.sh --tap-test) Index: libvirt-tck/scripts/nwfilter/nwfilter_concurrent.sh =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/nwfilter_concurrent.sh @@ -0,0 +1,371 @@ +#!/bin/sh + +VIRSH=virsh + +# 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))" +FLAG_VERBOSE="$((1<<2))" +FLAG_LIBVIRT_TEST="$((1<<3))" +FLAG_TAP_TEST="$((1<<4))" +FLAG_FORCE_CLEAN="$((1<<5))" + +failctr=0 +passctr=0 +attachfailctr=0 +attachctr=0 + +TAP_FAIL_LIST="" +TAP_FAIL_CTR=0 +TAP_TOT_CTR=0 + +usage() { + cmd="$0" +cat <<EOF +Usage: ${cmd} [--help|-h|-?] [--noattach] [--wait] [--verbose] + [--libvirt-test] [--tap-test] + +Options: + --help,-h,-? : Display this help screen. + --noattach : Skip tests that attach and detach a network interface + --wait : Wait for the user to press the enter key once an error + was detected + --verbose : Verbose output + --libvirt-test : Use the libvirt test output format + --tap-test : TAP format output + --force : Allow the automatic cleaning of VMs and nwfilters + previously created by the TCK test suite + +This test will create two virtual machines. The one virtual machine +will use a filter called '${TESTFILTERNAME}', and reference the filter +'clean-traffic' which should be available by default with every install. +The other virtual machine will reference the filter 'tck-testcase' and will +have its filter permanently updated. +EOF +} + + +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)) +} + +tap_pass() { + txt=$(echo "$2" | gawk '{print substr($0,1,70)}') + echo "ok $1 - ${txt}" + TAP_TOT_CTR=$(($TAP_TOT_CTR + 1)) +} + +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` + 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. +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 +} + + +PRGDIR="./concurrency" +CREATE_DES_VM1="${PRGDIR}/start-destroy-vm.sh 1" +CREATE_DES_VM2="${PRGDIR}/start-destroy-vm.sh 2" +CHG_FILTER_VM1="${PRGDIR}/chg-vm-filter.sh 1" +CHG_FILTER_VM2="${PRGDIR}/chg-vm-filter.sh 2" + + +startPrgs() +{ + flags="$1" + + sh ${CHG_FILTER_VM1} "$4" 2>&1 >/dev/null & + [ $? -ne 0 ] && \ + ( killPrgs "${flags}" "Could not start program 3" ; return 1; ) + CHG_FILTER_VM1_THR=$! + + sh ${CHG_FILTER_VM2} "$5" 2>&1 >/dev/null & + [ $? -ne 0 ] && \ + ( killPrgs "${flags}" "Could not start program 4" ; return 1; ) + CHG_FILTER_VM2_THR=$! + + # Give some time for the filters to be created + sleep 2 + + sh ${CREATE_DES_VM1} "$2" 2>&1 >/dev/null & + [ $? -ne 0 ] && \ + ( killPrgs "${flags}" "Could not start program 1" ; return 1; ) + CREATE_DES_VM1_THR=$! + + sh ${CREATE_DES_VM2} "$3" 2>&1 >/dev/null & + [ $? -ne 0 ] && \ + ( killPrgs "${flags}" "Could not start program 2" ; return 1; ) + CREATE_DES_VM2_THR=$! + +} + + +killPrgs() +{ + msg="$1" + + # terminate all process + [ "x${CREATE_DES_VM1_THR}x" != "xx" ] && \ + kill -9 ${CREATE_DES_VM1_THR} + [ "x${CREATE_DES_VM2_THR}x" != "xx" ] && \ + kill -9 ${CREATE_DES_VM2_THR} + [ "x${CHG_FILTER_VM1_THR}x" != "xx" ] && \ + kill -9 ${CHG_FILTER_VM1_THR} + [ "x${CHG_FILTER_VM2_THR}x" != "xx" ] && \ + kill -9 ${CHG_FILTER_VM2_THR} +} + + +testFail() +{ + flags="$1" + msg="$2" + + failctr=$(($failctr + 1)) + if [ $(($flags & $FLAG_VERBOSE)) -ne 0 ]; then + echo "FAIL : ${msg}" + fi + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then + echo "Press enter" + read enter + fi + [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \ + test_result $(($passctr + $failctr)) "" 1 + [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \ + tap_fail $(($passctr + $failctr)) "${msg}" +} + + +testPass() +{ + flags="$1" + msg="$2" + + passctr=$(($passctr + 1)) + if [ $(($flags & $FLAG_VERBOSE)) -ne 0 ]; then + echo "PASS : ${msg}" + fi + [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \ + test_result $(($passctr + $failctr)) "" 0 + [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \ + tap_pass $(($passctr + $failctr)) "${msg}" +} + +cleanup() +{ + rm -rf "$1" + killPrgs + exit $2 +} + +runTest() +{ + flags="$1" + + passctr=0 + failctr=0 + + tmpdir=`mktmpdir` + failctr=0 + passctr=0 + logvm1="${PWD}/${tmpdir}/logvm1" + logvm2="${PWD}/${tmpdir}/logvm2" + logfivm1="${PWD}/${tmpdir}/logfivm1" + logfivm2="${PWD}/${tmpdir}/logfivm2" + + loops=15 + + trap "cleanup ${tmpdir} 1" INT + + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then + # Need to display the number of total tests + tap_total=$((4 * $loops)) + echo "1..${tap_total}" + fi + + startPrgs "${flags}" "${logvm1}" "${logvm2}" \ + "${logfivm1}" "${logfivm2}" + + [ $? -ne 0 ] && rm -rf "${tmpdir}" && return 1; + + sleep 8 + + expect=1 + + while :; + do + # VM start/destroy cycles expected to be slow + tmp=$(($expect / 3)) + # The logs give us the number of cycles the VMs were created + # and destroyed. + val=$(cat "${logvm1}" 2>/dev/null | tail -n 1) + ( [ "x${val}x" = "xx" ] || [ ${val} -lt ${tmp} ] ) \ + && testFail "${flags}" \ + "VM1 log - step ${expect} ($val < $tmp)" \ + || testPass "${flags}" \ + "VM1 log - step ${expect} ($val >= $tmp)" + + val=$(cat "${logvm2}" 2>/dev/null | tail -n 1) + ( [ "x${val}x" = "xx" ] || [ ${val} -lt ${tmp} ] ) \ + && testFail "${flags}" \ + "VM2 log - step ${expect} ($val < $tmp)" \ + || testPass "${flags}" \ + "VM2 log - step ${expect} ($val >= $tmp)" + + # The changing of the filters is expected to work much faster + tmp=$(($expect * 5)) + # Also here the logs give us the number of filter change cycles + val=$(cat "${logfivm1}" 2>/dev/null | tail -n 1) + ( [ "x${val}x" = "xx" ] || [ ${val} -lt ${tmp} ] ) \ + && testFail "${flags}" \ + "VM1 filter log - step ${expect} ($val < $tmp)" \ + || testPass "${flags}" \ + "VM1 filter log - step ${expect} ($val >= $tmp)" + + val=$(cat "${logfivm2}" 2>/dev/null | tail -n 1) + ( [ "x${val}x" = "xx" ] || [ ${val} -lt ${tmp} ] ) \ + && testFail "${flags}" \ + "VM2 filter log - step ${expect} ($val < $tmp)" \ + || testPass "${flags}" \ + "VM2 filter log - step ${expect} ($val >= $tmp)" + + expect=$(($expect + 1)) + [ ${expect} -gt ${loops} ] && break; + + sleep 4 + done + + cleanup "${tmpdir}" 0 +} + + +main() { + prgname="$0" + xmldir="nwfilterxml2xmlin" + fwalldir="nwfilterxml2fwallout" + found=0 + filtername="tck-testcase" + libvirtdpid=-1 + + flags=${FLAG_ATTACH} + + while [ $# -ne 0 ]; do + case "$1" in + --help|-h|-\?) usage ${prgname}; exit 0;; + --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 + done + + if [ `uname` != "Linux" ]; then + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then + echo "1..0 # Skipped: Only valid on Linux hosts" + else + echo "This script will only run on Linux." + fi + exit 1; + fi + + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then + if [ "${LIBVIRT_URI}" != "qemu:///system" ]; then + echo "1..0 # Skipped: Only valid for Qemu system driver" + exit 0 + fi + + for name in `virsh list --all | awk '{print $2}'` + do + case ${name} in + tck*) + 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) + rc2=$? + if [ $rc1 -ne 0 ] && [ $rc2 -ne 0 ]; then + echo "Bail out! Could not destroy VM ${name}: ${res}" + exit 0 + fi + else + echo "Bail out! VM ${name} already exists, use --force to clean" + exit 1 + fi + esac + done + + for name in `virsh nwfilter-list | awk '{print $2}'` + do + case ${name} in + tck*) + 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 filter ${name}: ${res}" + exit 1 + fi + else + echo "Bail out! Filter ${name} already exists, use --force to clean" + exit 1 + fi + esac + done + fi + + if [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ]; then + curdir="${PWD}" + . test-lib.sh + if [ $? -ne 0 ]; then + exit 1 + fi + test_intro $this_test + cd "${curdir}" + [ $? -ne 0 ] && echo "cd failed" && exit 1 + fi + + runTest "${flags}" + + return 0 +} + +main "$@" Index: libvirt-tck/scripts/nwfilter/concurrency/chg-vm-filter.sh =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/chg-vm-filter.sh @@ -0,0 +1,19 @@ +#!/bin/sh +cd $(dirname "$0") +ctr=0 +[ -z "$2" ] && exit 1 +idx="$1" +logfile="$2" +rm -f "${logfile}" +touch "${logfile}" +while :; +do + virsh nwfilter-define tck-vm${idx}-filter1.xml + [ $? -ne 0 ] && break + [ ! -w "${logfile}" ] && break + virsh nwfilter-define tck-vm${idx}-filter2.xml + [ $? -ne 0 ] && break + ctr=$(($ctr + 1)) + [ ! -w "${logfile}" ] && break + echo "${ctr}" >> "${logfile}" +done Index: libvirt-tck/scripts/nwfilter/concurrency/start-destroy-vm.sh =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/start-destroy-vm.sh @@ -0,0 +1,19 @@ +#!/bin/sh +cd $(dirname "$0") +ctr=0 +[ -z "$2" ] && exit 1 +idx="$1" +logfile="$2" +rm -f "${logfile}" +touch "${logfile}" +while :; +do + virsh create tck-vm${idx}.xml + [ $? -ne 0 ] && break + sleep 2 + virsh destroy tck-vm${idx} + [ $? -ne 0 ] && break + ctr=$(($ctr + 1)) + [ ! -w "${logfile}" ] && break + echo "${ctr}" >> "${logfile}" +done Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm1-filter1.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm1-filter1.xml @@ -0,0 +1,26 @@ +<filter name='tck-vm1-filter' chain='root'> + <uuid>464d2617-43d0-7694-b479-320b72dac187</uuid> + <filterref filter='clean-traffic'/> + <rule action='accept' direction='in' priority='500'> + <all comment='test'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <all/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='21' dstportend='22'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='80'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <icmp/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp dstportstart='53'/> + </rule> + <rule action='drop' direction='inout' priority='500'> + <all/> + </rule> +</filter> + Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm1-filter2.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm1-filter2.xml @@ -0,0 +1,26 @@ +<filter name='tck-vm1-filter' chain='root'> + <uuid>464d2617-43d0-7694-b479-320b72dac187</uuid> + <filterref filter='clean-traffic'/> + <rule action='accept' direction='in' priority='500'> + <all comment='test again'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <all/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='21' dstportend='22'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='80'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <icmp/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp dstportstart='53'/> + </rule> + <rule action='drop' direction='inout' priority='500'> + <all/> + </rule> +</filter> + Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm1.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm1.xml @@ -0,0 +1,30 @@ + <domain type='kvm'> + <name>tck-vm1</name> + <memory>32768</memory> + <currentMemory>32768</currentMemory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <interface type='bridge'> + <source bridge='virbr0'/> + <filterref filter='tck-vm1-filter'> + </filterref> + <target dev='tck-vm1-if0'/> + </interface> + <console type='pty'> + </console> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'/> + </devices> + </domain> Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm2-filter1.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm2-filter1.xml @@ -0,0 +1,26 @@ +<filter name='tck-vm2-filter' chain='root'> + <uuid>364d2617-43d0-7694-b479-320b72dac187</uuid> + <filterref filter='clean-traffic'/> + <rule action='accept' direction='in' priority='500'> + <all comment='another test'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <all/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='21' dstportend='22'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='80'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <icmp/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp dstportstart='53'/> + </rule> + <rule action='drop' direction='inout' priority='500'> + <all/> + </rule> +</filter> + Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm2-filter2.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm2-filter2.xml @@ -0,0 +1,26 @@ +<filter name='tck-vm2-filter' chain='root'> + <uuid>364d2617-43d0-7694-b479-320b72dac187</uuid> + <filterref filter='clean-traffic'/> + <rule action='accept' direction='in' priority='500'> + <all comment='another test again'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <all/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='21' dstportend='22'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='80'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <icmp/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp dstportstart='53'/> + </rule> + <rule action='drop' direction='inout' priority='500'> + <all/> + </rule> +</filter> + Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm2.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm2.xml @@ -0,0 +1,30 @@ + <domain type='kvm'> + <name>tck-vm2</name> + <memory>32768</memory> + <currentMemory>32768</currentMemory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <interface type='bridge'> + <source bridge='virbr0'/> + <filterref filter='tck-vm2-filter'> + </filterref> + <target dev='tck-vm2-if0'/> + </interface> + <console type='pty'> + </console> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'/> + </devices> + </domain>

On 11/15/2010 01:22 PM, Stefan Berger wrote:
Now that the existing scripts are (hopefully) cleaned up and my POSIX compliancy shell-scripting skills have temporarily :-) improved, I am now adding a test case that exercises concurrency. The test case creates and destroys 2 VMs concurrently as well as changes their referenced filters in a tight loop. This kind of test previously uncovered
- deadlocks in libvirt due to lock-ordering in the nwfilter subsystem - libvirt termination bug in libaugeas due to doubly closed file descriptors and the side effects
All of these have been fixed recently.
The test script is known to run in bash, dash and ksh shells.
Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
Index: libvirt-tck/scripts/nwfilter/060-concurrency.t =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/060-concurrency.t @@ -0,0 +1,5 @@ +#!/bin/sh + +pwd=$(dirname -- "$0") + +(cd -- "${pwd}"; sh ./nwfilter_concurrent.sh --tap-test)
I'd use && instead of ; so that if the cd fails we don't try to run a random file in the wrong directory.
Index: libvirt-tck/scripts/nwfilter/nwfilter_concurrent.sh =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/nwfilter_concurrent.sh @@ -0,0 +1,371 @@ +#!/bin/sh + +VIRSH=virsh + +# 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))" +FLAG_VERBOSE="$((1<<2))" +FLAG_LIBVIRT_TEST="$((1<<3))" +FLAG_TAP_TEST="$((1<<4))" +FLAG_FORCE_CLEAN="$((1<<5))"
I see some common patterns here with your other tck shell scripts :) Would it be better to factor out some of this common initialization and common script routines (such as tap_fail) into a single .sh file and source that file up frong, rather than copying them into each driver? Especially since if we fix a bug in one, we have to copy that fix to multiple files at the moment?
+ +killPrgs() +{ + msg="$1" + + # terminate all process + [ "x${CREATE_DES_VM1_THR}x" != "xx" ] && \ + kill -9 ${CREATE_DES_VM1_THR}
Should we try 'kill -2' rather than 'kill -9' to send a SIGINT and try and allow the subprocesses a chance to gracefully clean up after themselves rather than forcefully quitting?
+runTest() +{ + flags="$1" + + passctr=0 + failctr=0 + + tmpdir=`mktmpdir` + failctr=0 + passctr=0 + logvm1="${PWD}/${tmpdir}/logvm1" + logvm2="${PWD}/${tmpdir}/logvm2" + logfivm1="${PWD}/${tmpdir}/logfivm1" + logfivm2="${PWD}/${tmpdir}/logfivm2" + + loops=15 + ...
+ val=$(cat "${logfivm2}" 2>/dev/null | tail -n 1) + ( [ "x${val}x" = "xx" ] || [ ${val} -lt ${tmp} ] ) \ + && testFail "${flags}" \ + "VM2 filter log - step ${expect} ($val < $tmp)" \ + || testPass "${flags}" \ + "VM2 filter log - step ${expect} ($val >= $tmp)" + + expect=$(($expect + 1)) + [ ${expect} -gt ${loops} ] && break; + + sleep 4 + done
This seems very sensitive to timing measured on your machine. Is there any way to make it more robust, and less likely to fail on a much faster or much slower machine? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 11/15/2010 07:01 PM, Eric Blake wrote:
On 11/15/2010 01:22 PM, Stefan Berger wrote:
Now that the existing scripts are (hopefully) cleaned up and my POSIX compliancy shell-scripting skills have temporarily :-) improved, I am now adding a test case that exercises concurrency. The test case creates and destroys 2 VMs concurrently as well as changes their referenced filters in a tight loop. This kind of test previously uncovered
- deadlocks in libvirt due to lock-ordering in the nwfilter subsystem - libvirt termination bug in libaugeas due to doubly closed file descriptors and the side effects
All of these have been fixed recently.
The test script is known to run in bash, dash and ksh shells.
Signed-off-by: Stefan Berger<stefanb@us.ibm.com> Index: libvirt-tck/scripts/nwfilter/060-concurrency.t =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/060-concurrency.t @@ -0,0 +1,5 @@ +#!/bin/sh + +pwd=$(dirname -- "$0") + +(cd -- "${pwd}"; sh ./nwfilter_concurrent.sh --tap-test) I'd use&& instead of ; so that if the cd fails we don't try to run a random file in the wrong directory.
Index: libvirt-tck/scripts/nwfilter/nwfilter_concurrent.sh =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/nwfilter_concurrent.sh @@ -0,0 +1,371 @@ +#!/bin/sh + +VIRSH=virsh + +# 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))" +FLAG_VERBOSE="$((1<<2))" +FLAG_LIBVIRT_TEST="$((1<<3))" +FLAG_TAP_TEST="$((1<<4))" +FLAG_FORCE_CLEAN="$((1<<5))" I see some common patterns here with your other tck shell scripts :)
Would it be better to factor out some of this common initialization and common script routines (such as tap_fail) into a single .sh file and source that file up frong, rather than copying them into each driver? Especially since if we fix a bug in one, we have to copy that fix to multiple files at the moment?
At some point this could be done. Mostly the tap functions could be moved into a common file.
+ +killPrgs() +{ + msg="$1" + + # terminate all process + [ "x${CREATE_DES_VM1_THR}x" != "xx" ]&& \ + kill -9 ${CREATE_DES_VM1_THR} Should we try 'kill -2' rather than 'kill -9' to send a SIGINT and try and allow the subprocesses a chance to gracefully clean up after themselves rather than forcefully quitting?
Fine by me.
+runTest() +{ + flags="$1" + + passctr=0 + failctr=0 + + tmpdir=`mktmpdir` + failctr=0 + passctr=0 + logvm1="${PWD}/${tmpdir}/logvm1" + logvm2="${PWD}/${tmpdir}/logvm2" + logfivm1="${PWD}/${tmpdir}/logfivm1" + logfivm2="${PWD}/${tmpdir}/logfivm2" + + loops=15 + ...
+ val=$(cat "${logfivm2}" 2>/dev/null | tail -n 1) + ( [ "x${val}x" = "xx" ] || [ ${val} -lt ${tmp} ] ) \ +&& testFail "${flags}" \ + "VM2 filter log - step ${expect} ($val< $tmp)" \ + || testPass "${flags}" \ + "VM2 filter log - step ${expect} ($val>= $tmp)" + + expect=$(($expect + 1)) + [ ${expect} -gt ${loops} ]&& break; + + sleep 4 + done This seems very sensitive to timing measured on your machine. Is there any way to make it more robust, and less likely to fail on a much faster or much slower machine?
Much slower machines would be a concern, or one that's extremely busy. Faster ones should not be a problem. I also wasn't sure how to go about it and how to determine when a process really is stuck and let the test case fail. Theoretically one of the start-destroy processes could only go through a single 'cycle' and it's difficult to say then whether the test succeeded. Another possibility would be to require that each one of the start-destroy processes makes 5 cycles and then the test ends, independent of how long it takes. Stefan
participants (2)
-
Eric Blake
-
Stefan Berger