2:59 p.m.
On 04/07/2010 12:53 PM, Stefan Berger wrote:
+++ libvirt-acl/tests/nwfiltervmtest.sh
@@ -0,0 +1,186 @@
+#!/bin/sh
Not all sh understand...
+function doTest() {
+ local xmlfile="$1"
...local. If this is a bash test, you need to update the #!.
Otherwise, you have to give up on local variables.
+ local fwallfile="$2"
+ local ifname="$3"
+ local cmd line tmpfile tmpfile2
+ local linenums ctr=0
+ local regex="s/${ORIG_IFNAME}/${ifname}/g"
+
+ if [ ! -r "${xmlfile}" ]; then
+ echo "FAIL : Cannot access filter XML file ${xmlfile}."
+ return 1
+ fi
+
+ tmpfile=`mktemp`
+ tmpfile2=`mktemp`
Not all systems have mktemp; is it worth adding fallback code in that case?
+
+ ${VIRSH} nwfilter-define "${xmlfile}" > /dev/null
+
+ exec 4<&0
+
+ exec < ${fwallfile}
+
+ read line
+ while [ ! -z "${line}" ]; do
Depending on the content of line, this is unsafe in some implementations
of test(1). Better would be:
while [ "x$line" != x ]; do
+ cmd=`echo ${line##\#} | sed ${regex}`
Another bash-ism, that would need a rewrite if you want to be portable
to generic sh. Or we decide to change the #!, and be done with the issue.
+
+ exec ${cmd} | grep -v "^Bridge" | grep -v "^$" > ${tmpfile}
+
+ rm ${tmpfile2}
+ touch ${tmpfile2}
+
+ while [ 1 ]; do
I think that looks better as:
while :; do
+ read
+
+ line="${REPLY}"
+
+ if [ "${line:0:1}" == "#" -o -z "${line}" ]; then
More bash-isms. And [ cond1 -o cond2 ] ought to fail 'make
syntax-check' (if it didn't, then our syntax check regex is a bit
suspect). Of course, if you assume bash, then it is portable, but if
you are portable to any sh, this needs to use [ cond1 ] || [ cond2 ]
+
+ diff ${tmpfile} ${tmpfile2} > /dev/null
+
+ if [ $? -ne 0 ]; then
+ echo "FAIL ${xmlfile} : ${cmd}"
+ diff ${tmpfile} ${tmpfile2}
+ else
+ echo "PASS ${xmlfile} : ${cmd}"
+ fi
+
+ break;
+
+ fi
+ echo "${line}" | sed ${regex} >> ${tmpfile2}
+ done
+ done
+
+ exec 0<&4
+ exec 4<&-
+
+ rm -rf "${tmpfile}" "${tmpfile2}"
+}
+
+
+function runTests() {
+ local ifname="$1"
+ local xmldir="$2"
+ local fwalldir="$3"
+ local fwallfiles f
+
+ pushd ${PWD} > /dev/null
+ cd ${fwalldir}
+ fwallfiles=`ls *.fwall`
+ popd > /dev/null
pushd/popd is another bash-ism.
+
+ for fil in ${fwallfiles}; do
+ f=${fil%%.fwall}
Another bash-ism.
+ doTest "${xmldir}/${f}.xml"
"${fwalldir}/${fil}" "${ifname}"
+ done
+}
+
+
+function checkVM() {
+ local vmname="$1"
+ local ifname="$2"
+ local nwfilter="$3"
+ local f i c
+
+ c=`virsh dumpxml ${vmname} | grep -c "<interface"`
+ if [ ${c} -ne 1 ]; then
+ echo "VM '${vmname}' has multiple interfaces. I cannot tell for sure
"
+ echo "whether this VM has the correct interface name '${ifname}' and
"
+ echo "reference the filter '${nwfilter}'. Cowardly skipping this
VM..."
+ return 1
+ fi
+
+ f=`${VIRSH} dumpxml ${vmname} | \
+ tr -d "\n" | \
+ sed "s/.*filterref filter='\([a-zA-Z0-9_]\+\)'.*/\1/"`
That would fail on Solaris sed, where sed requires that a trailing
newline be present in the input. Plus, "\n" is not necessarily portable
shell (it is unclear whether the \ would be literally preserved). I
would have used:
{ $VIRSH dumpxml $vmname | tr -d '\n'; echo; } | \
sed ...
+ i=`${VIRSH} dumpxml ${vmname} | \
+ tr -d "\n" | \
+ sed "s/.*\<interface.*target
dev='\([a-zA-Z0-9_]\+\)'.*<\/interface>.*/\1/"`
+
+ if [ "${i}" == "${ifname}" -a "${f}" ==
"${nwfilter}" ]; then
Another abuse of [ -a ] that 'make syntax-check' should have caught.
+ return 0
+ fi
+
+ return 1
+}
+
+
+function main() {
+ local prgname="$0"
Some shells corrupt $0 in the context of shell functions.
+ local ifname="${ORIG_IFNAME}"
+ local xmldir="nwfilterxml2xmlin"
+ local fwalldir="nwfilterxml2fwallout"
+ local found=0 vms
+ local filtername="testcase"
+
+ while [ $# -ne 0 ]; do
+ case "$1" in
+ --help|-h|-\?) usage ${prgname}; exit 0;;
+ --ifname|-i) shift 1; ifname="$1";;
+ *) usage ${prgname}; exit 1;;
+ esac
+ shift 1
+ done
+
+ vms=`${VIRSH} list | grep running | gawk '{print $2}'`
+ if [ -z ${vms} ]; then
+ echo "Error: Need a running VM."
+ exit 1;
+ fi
+
+ for vm in ${vms}; do
+ checkVM "${vm}" "${ifname}" "${filtername}"
+ if [ $? -eq 0 ]; then
+ found=1;
+ break;
+ fi
+ done
+
+ if [ ${found} -eq 0 ]; then
+ echo "Error: Suitable VM seems not to be running. Check the help screen";
+ echo "to (--help) to read about requirements to the running VM.";
+ exit 1;
+ fi
+
+ runTests "${ifname}" "${xmldir}" "${fwalldir}"
+}
+
+main $*
Would that be safer as:
main "$@"
My review stops here - shell is my area of expertise, but my Linux
network filtering knowledge is sparse.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org