November 2010 - Devel - libvirt List Archives

[libvirt] [PATCH] daemon: fix indention problem in daemon/libvirt.c
by Osier Yang 16 Nov '10

16 Nov '10

* daemon/libvirtd.c --- daemon/libvirtd.c | 40 ++++++++++++++++++++-------------------- 1 files changed, 20 insertions(+), 20 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index ab2b579..9446638 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1077,33 +1077,33 @@ static int qemudNetworkEnable(struct qemud_server *server) { static gnutls_session_t remoteInitializeTLSSession (void) { - gnutls_session_t session; - int err; + gnutls_session_t session; + int err; - err = gnutls_init (&session, GNUTLS_SERVER); - if (err != 0) goto failed; + err = gnutls_init (&session, GNUTLS_SERVER); + if (err != 0) goto failed; - /* avoid calling all the priority functions, since the defaults - * are adequate. - */ - err = gnutls_set_default_priority (session); - if (err != 0) goto failed; + /* avoid calling all the priority functions, since the defaults + * are adequate. + */ + err = gnutls_set_default_priority (session); + if (err != 0) goto failed; - err = gnutls_credentials_set (session, GNUTLS_CRD_CERTIFICATE, x509_cred); - if (err != 0) goto failed; + err = gnutls_credentials_set (session, GNUTLS_CRD_CERTIFICATE, x509_cred); + if (err != 0) goto failed; - /* request client certificate if any. - */ - gnutls_certificate_server_set_request (session, GNUTLS_CERT_REQUEST); + /* request client certificate if any. + */ + gnutls_certificate_server_set_request (session, GNUTLS_CERT_REQUEST); - gnutls_dh_set_prime_bits (session, DH_BITS); + gnutls_dh_set_prime_bits (session, DH_BITS); - return session; + return session; - failed: - VIR_ERROR(_("remoteInitializeTLSSession: %s"), - gnutls_strerror (err)); - return NULL; +failed: + VIR_ERROR(_("remoteInitializeTLSSession: %s"), + gnutls_strerror (err)); + return NULL; } /* Check DN is on tls_allowed_dn_list. */ -- 1.7.3.2

2 1

[libvirt] [PATCH] qemu: fix typos in qemu_monitor_text.c
by Osier Yang 16 Nov '10

16 Nov '10

From: root <root(a)Osier.(none)> * src/qemu/qemu_monitor_text.c (qemuMonitorTextChangeMedia) --- src/qemu/qemu_monitor_text.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 2552111..64ec57b 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -906,7 +906,7 @@ int qemuMonitorTextChangeMedia(qemuMonitorPtr mon, if (qemuMonitorCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not eject media on %s"), devname); + _("could not change media on %s"), devname); goto cleanup; } @@ -915,7 +915,7 @@ int qemuMonitorTextChangeMedia(qemuMonitorPtr mon, * No message is printed on success it seems */ if (strstr(reply, "device ")) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not eject media on %s: %s"), devname, reply); + _("could not change media on %s: %s"), devname, reply); goto cleanup; } -- 1.7.3.2

2 2

[libvirt] Schedule for next libvirt releases
by Daniel Veillard 16 Nov '10

16 Nov '10

Let's try to keep with end of month based release cycles, so assuming 0.8.6 gets pushed on the 30th, that means 2 weeks from today, and hence we would enter the freeze on Tuesday 23 next week. This means one more week to push patches for 0.8.6. For the following release, I will try to get something out at the end of December, I guess it won't be too hard to do the freeze in the last week, and I will try to make a release for the new year (0.8.7 unless something crazy lands in git in the meantime :-) Happy hacking ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

1 0

[libvirt] [PATCH] [RESEND] qemu: record timestamp in qemu domain log
by Osier Yang 16 Nov '10

16 Nov '10

Currently only support domain start and shutdown, for domain start, record timestamp before the qemu command line, and for domain shutdown, just say it's shutting down with timestamp. * src/qemu/qemu_driver.c --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fd61864..423befb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3877,6 +3877,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, char ebuf[1024]; char *pidfile = NULL; int logfile = -1; + char *timestamp; qemuDomainObjPrivatePtr priv = vm->privateData; struct qemudHookData hookData; @@ -4084,7 +4085,17 @@ static int qemudStartVMDaemon(virConnectPtr conn, goto cleanup; } + if ((timestamp = virTimestamp()) == NULL) { + virReportOOMError(); + goto cleanup; + } else if (safewrite(logfile, timestamp, strlen(timestamp)) < 0) { + VIR_WARN("Unable to write timestamp to logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + VIR_FREE(timestamp); + } + tmp = progenv; + while (*tmp) { if (safewrite(logfile, *tmp, strlen(*tmp)) < 0) VIR_WARN("Unable to write envv to logfile: %s", @@ -4233,7 +4244,6 @@ cleanup: return -1; } - static void qemudShutdownVMDaemon(struct qemud_driver *driver, virDomainObjPtr vm, int migrated) { @@ -4243,10 +4253,44 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver, virErrorPtr orig_err; virDomainDefPtr def; int i; + int logfile = -1; + char *timestamp; + char ebuf[1024]; VIR_DEBUG("Shutting down VM '%s' pid=%d migrated=%d", vm->def->name, vm->pid, migrated); + if ((logfile = qemudLogFD(driver, vm->def->name)) < 0) { + /* To not break the normal domain shutdown process, skip the + * timestamp log writing if failed on opening log file. */ + VIR_WARN("Unable to open logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + } else { + if ((timestamp = virTimestamp()) == NULL) { + virReportOOMError(); + } else { + char *shutdownMsg; + + if ((virAsprintf(&shutdownMsg, "%s: shutting down\n", + timestamp)) < 0) { + virReportOOMError(); + } else { + if (safewrite(logfile, shutdownMsg, strlen(shutdownMsg)) < 0) { + VIR_WARN("Unable to write shutdownMsg to logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + } + + VIR_FREE(shutdownMsg); + } + + VIR_FREE(timestamp); + } + + if (VIR_CLOSE(logfile) < 0) + VIR_WARN("Unable to close logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + } + /* This method is routinely used in clean up paths. Disable error * reporting so we don't squash a legit error. */ orig_err = virSaveLastError(); @@ -4382,7 +4426,6 @@ retry: } } - static virDrvOpenStatus qemudOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { -- 1.7.3.2

3 7

[libvirt] [PATCH] replace last instances of close()
by Stefan Berger 16 Nov '10

16 Nov '10

I am replacing the last instances of close() I found with VIR_CLOSE() / VIR_FORCE_CLOSE respectively. The first patch of virsh I missed out on previously. The 2nd patch I had left out intentionally to look at it more carefully: The 'closed' variable could be easily removed since it wasn't used anywhere else. The possible race condition that could result from the filedescriptor being closed and not set to -1 (and possibly let us write into 'something' totally different if the fd was allocated by another thread) seems to be prevented by the qemuMonitorLock() already placed around the code that reads from or writes to the fd. So the change of this code as shown in the patch should not have any side-effects. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/qemu/qemu_monitor.c | 14 +++----------- tools/virsh.c | 4 ++-- 2 files changed, 5 insertions(+), 13 deletions(-) Index: libvirt-acl/tools/virsh.c =================================================================== --- libvirt-acl.orig/tools/virsh.c +++ libvirt-acl/tools/virsh.c @@ -8977,12 +8977,12 @@ editWriteToTempFile (vshControl *ctl, co if (safewrite (fd, doc, strlen (doc)) == -1) { vshError(ctl, _("write: %s: failed to write to temporary file: %s"), ret, strerror(errno)); - close (fd); + VIR_FORCE_CLOSE(fd); unlink (ret); VIR_FREE(ret); return NULL; } - if (close (fd) == -1) { + if (VIR_CLOSE(fd) < 0) { vshError(ctl, _("close: %s: failed to write or close temporary file: %s"), ret, strerror(errno)); unlink (ret); Index: libvirt-acl/src/qemu/qemu_monitor.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_monitor.c +++ libvirt-acl/src/qemu/qemu_monitor.c @@ -73,8 +73,6 @@ struct _qemuMonitor { /* If the monitor EOF callback is currently active (stops more commands being run) */ unsigned eofcb: 1; - /* If the monitor is in process of shutting down */ - unsigned closed: 1; unsigned json: 1; }; @@ -692,17 +690,11 @@ void qemuMonitorClose(qemuMonitorPtr mon VIR_DEBUG("mon=%p", mon); qemuMonitorLock(mon); - if (!mon->closed) { + + if (mon->fd >= 0) { if (mon->watch) virEventRemoveHandle(mon->watch); - if (mon->fd != -1) - close(mon->fd); - /* NB: ordinarily one might immediately set mon->watch to -1 - * and mon->fd to -1, but there may be a callback active - * that is still relying on these fields being valid. So - * we merely close them, but not clear their values and - * use this explicit 'closed' flag to track this state */ - mon->closed = 1; + VIR_FORCE_CLOSE(mon->fd); } if (qemuMonitorUnref(mon) > 0)

3 6

[libvirt] process= support for 'qemu-kvm -name' [Bug 576950]
by John Morrissey 16 Nov '10

16 Nov '10

I wrote (attached here, and to the bug) a quick patch that sets the process name to the same value as the window title. I'm unsure where to go from here. Should I add support for converting "native" QEMU command lines to libvirt XML? What would that look like, since I'm not modifying the libvirt format? Should it just drop any ,process= from the QEMU command line it's parsing? I also imagine the test cases will need updating. If someone could give me some high-level guidance, I'd be happy to keep going on this. john -- John Morrissey _o /\ ---- __o jwm(a)horde.net _-< \_ / \ ---- < \, www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__

4 11

[libvirt] [PATCH] nwfilter: use /bin/sh rather than requiring bash
by Eric Blake 16 Nov '10

16 Nov '10

* src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesWriteToTempFile): Use /bin/sh. (bash_cmd_path): Delete. (ebiptablesDriverInit, ebiptablesDriverShutdown): No need to search for bash. (CMD_EXEC): Prefer $() over ``, since we can assume POSIX. (iptablesSetupVirtInPost): Use portable 'test' syntax. (iptablesLinkIPTablesBaseChain): Use POSIX $(()) syntax. --- It turns out that the current use of test x == x in nwfilter was technically safe after all, since we were requiring the existence of bash (and that was also relatively reasonable, since nwfilter is only compiled for Linux). But that still feels awkward, when it is easier to just fix things to work with /bin/sh. Here, I went the easy route, and assumed that since things are Linux, we can use shell constructs that aren't strictly portable to all /bin/sh implementations, so long as they are portable to POSIX (and will thus work with dash or busybox sh). src/nwfilter/nwfilter_ebiptables_driver.c | 25 +++++++++++++------------ 1 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 20d1ba3..c3a0d3e 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -52,12 +52,16 @@ #define CHAINPREFIX_HOST_IN_TEMP 'J' #define CHAINPREFIX_HOST_OUT_TEMP 'P' +/* This file generates a temporary shell script. Since ebiptables is + Linux-specific, we can be reasonably certain that /bin/sh is more + or less POSIX-compliant, so we can use $() and $(()). However, we + cannot assume that /bin/sh is bash, so stick to POSIX syntax. */ #define CMD_SEPARATOR "\n" #define CMD_DEF_PRE "cmd='" #define CMD_DEF_POST "'" #define CMD_DEF(X) CMD_DEF_PRE X CMD_DEF_POST -#define CMD_EXEC "eval res=\\`\"${cmd}\"\\`" CMD_SEPARATOR +#define CMD_EXEC "eval res=\\$(\"${cmd}\")" CMD_SEPARATOR #define CMD_STOPONERR(X) \ X ? "if [ $? -ne 0 ]; then" \ " echo \"Failure to execute command '${cmd}'.\";" \ @@ -76,7 +80,6 @@ static char *ebtables_cmd_path; static char *iptables_cmd_path; static char *ip6tables_cmd_path; -static char *bash_cmd_path; static char *grep_cmd_path; static char *gawk_cmd_path; @@ -427,7 +430,7 @@ static int iptablesLinkIPTablesBaseChain(const char *iptables_cmd, " " CMD_DEF("%s -I %s %d -j %s") CMD_SEPARATOR " " CMD_EXEC " %s" - " let r=r+1\n" + " r=$(( $r + 1 ))\n" " " CMD_DEF("%s -D %s ${r}") CMD_SEPARATOR " " CMD_EXEC " %s" @@ -650,7 +653,7 @@ iptablesSetupVirtInPost(const char *iptables_cmd, virBufferVSprintf(buf, "res=$(%s -n -L " VIRT_IN_POST_CHAIN " | grep \"\\%s %s\")\n" - "if [ \"${res}\" == \"\" ]; then " + "if [ \"${res}\" = \"\" ]; then " CMD_DEF("%s" " -A " VIRT_IN_POST_CHAIN " %s %s -j ACCEPT") CMD_SEPARATOR @@ -2431,7 +2434,7 @@ ebiptablesDisplayRuleInstance(virConnectPtr conn ATTRIBUTE_UNUSED, * * Write the string into a temporary file and return the name of * the temporary file. The string is assumed to contain executable - * commands. A line '#!/bin/bash' will automatically be written + * commands. A line '#!/bin/sh' will automatically be written * as the first line in the file. The permissions of the file are * set so that the file can be run as an executable script. */ @@ -2444,7 +2447,7 @@ ebiptablesWriteToTempFile(const char *string) { char *header; size_t written; - virBufferVSprintf(&buf, "#!%s\n", bash_cmd_path); + virBufferAddLit(&buf, "#!/bin/sh\n"); if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); @@ -2513,10 +2516,10 @@ err_exit: * commands executed via the script the was run. * * Returns 0 in case of success, != 0 in case of an error. The returned - * value is NOT the result of running the commands inside the bash + * value is NOT the result of running the commands inside the shell * script. * - * Execute a sequence of commands (held in the given buffer) as a bash + * Execute a sequence of commands (held in the given buffer) as a /bin/sh * script and return the status of the execution. */ static int @@ -3657,7 +3660,6 @@ ebiptablesDriverInit(void) if (virMutexInit(&execCLIMutex)) return EINVAL; - bash_cmd_path = virFindFileInPath("bash"); gawk_cmd_path = virFindFileInPath("gawk"); grep_cmd_path = virFindFileInPath("grep"); @@ -3701,9 +3703,9 @@ ebiptablesDriverInit(void) VIR_FREE(ip6tables_cmd_path); } - /* ip(6)tables support needs bash, gawk & grep, ebtables doesn't */ + /* ip(6)tables support needs gawk & grep, ebtables doesn't */ if ((iptables_cmd_path != NULL || ip6tables_cmd_path != NULL) && - (!grep_cmd_path || !bash_cmd_path || !gawk_cmd_path)) { + (!grep_cmd_path || !gawk_cmd_path)) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("essential tools to support ip(6)tables " "firewalls could not be located")); @@ -3730,7 +3732,6 @@ static void ebiptablesDriverShutdown() { VIR_FREE(gawk_cmd_path); - VIR_FREE(bash_cmd_path); VIR_FREE(grep_cmd_path); VIR_FREE(ebtables_cmd_path); VIR_FREE(iptables_cmd_path); -- 1.7.3.2

2 7

[libvirt] [PATCH] [TCK] nwfilter: add a test case using concurrency
by Stefan Berger 16 Nov '10

16 Nov '10

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(a)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>

2 2

[libvirt] [PATCH v2] [TCK] Convert scripts to also run with dash
by Stefan Berger 16 Nov '10

16 Nov '10

v2: - following Eric's suggestions from review of v1 - runs with ksh shell as well I am converting the shell-scripted tap tests to also run using the dash and ksh 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(a)us.ibm.com> --- scripts/networks/100-apply-verify-host.t | 11 scripts/networks/networkApplyTest.sh | 189 +++--- scripts/networks/networkxml2hostout/tck-testnet-1.dat | 10 scripts/networks/networkxml2hostout/tck-testnet-1.post.dat | 10 scripts/networks/networkxml2hostout/tck-testnet-2.dat | 10 scripts/networks/networkxml2hostout/tck-testnet-2.post.dat | 10 scripts/nwfilter/050-apply-verify-host.t | 11 scripts/nwfilter/nwfilter2vmtest.sh | 375 ++++++------- scripts/nwfilter/nwfilterxml2fwallout/comment-test.fwall | 6 9 files changed, 310 insertions(+), 322 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,16 +94,16 @@ 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 tmpfile2=$tmpdir/file2 + OIFS="${IFS}" if echo a | grep -E '(a|b)' >/dev/null 2>&1 then EGREP='grep -E' @@ -112,11 +112,12 @@ function checkExpectedOutput() { exec 4<"${datafile}" - read <&4 - line="${REPLY}" + IFS="" + + read -r line <&4 while [ "x${line}x" != "xx" ]; do - cmd=$(echo "${line##\#}") + cmd=$(printf %s\\n "${line##\#}") skip=0 if [ "x${skipregex}x" != "xx" ]; then @@ -128,10 +129,14 @@ function checkExpectedOutput() { : >"${tmpfile2}" while :; do - read <&4 - line="${REPLY}" + read -r line <&4 + + case "${line}" in + '#'*) letter="#";; + *) letter="";; + esac - 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 @@ -140,49 +145,50 @@ function checkExpectedOutput() { diff "${tmpfile}" "${tmpfile2}" >/dev/null if [ $? -ne 0 ]; then - if [ $((flags & FLAG_VERBOSE)) -ne 0 ]; then + if [ $(($flags & $FLAG_VERBOSE)) -ne 0 ]; then echo "FAIL ${xmlfile} : ${cmd}" diff "${tmpfile}" "${tmpfile2}" fi - ((failctr++)) - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then - echo "tmp files: $tmpfile, $tmpfile2" - echo "Press enter" - read + failctr=$(($failctr + 1)) + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then + echo "tmp files: $tmpfile, $tmpfile2" + 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)) "${xmlfile} : ${cmd}" + [ $(($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++)) - [ $((flags & FLAG_VERBOSE)) -ne 0 ] && \ - echo "PASS ${xmlfile} : ${cmd}" - [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ - test_result $((passctr+failctr)) "" 0 - [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \ - tap_pass $((passctr+failctr)) "${xmlfile} : ${cmd}" + passctr=$(($passctr + 1)) + [ $(($flags & $FLAG_VERBOSE)) -ne 0 ] && \ + echo "PASS ${xmlfile} : ${cmd}" + [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \ + test_result $(($passctr + $failctr)) "" 0 + [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \ + tap_pass $(($passctr + $failctr)) "${xmlfile} : ${cmd}" fi break fi - echo "${line}" >> "${tmpfile2}" + printf %s\\n "${line}" >> "${tmpfile2}" done done exec 4>&- rm -rf "${tmpdir}" + + IFS="${OIFS}" } -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 +211,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 + 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 @@ -236,9 +238,9 @@ function runTests() { "${hostoutdir}/${f}.post.dat" "${flags}" done - if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - test_final $((passctr+failctr)) $failctr - elif [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ]; then + test_final $(($passctr + $failctr)) $failctr + elif [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then tap_final else echo "" @@ -250,29 +252,27 @@ 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 done if [ $(uname) != "Linux" ]; then - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; 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." @@ -280,7 +280,7 @@ function main() { exit 1; fi - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + 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 @@ -290,8 +290,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 +309,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) @@ -326,18 +326,17 @@ function main() { done fi - if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd "${PWD}" > /dev/null + if [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ]; then + curdir="${PWD}" . ./test-lib.sh if [ $? -ne 0 ]; then exit 1 fi test_intro $this_test - popd > /dev/null + cd "${curdir}" + [ $? -ne 0 ] && echo "cd failed" && exit 1 fi - res=$(${VIRSH} capabilities 2>&1) - runTests "${xmldir}" "${hostoutdir}" "${flags}" return 0 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,73 +63,69 @@ 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 + OIFS="${IFS}" exec 4<${fwallfile} - read <&4 - line="${REPLY}" + IFS="" + + read -r line <&4 while [ "x${line}x" != "xx" ]; do - cmd=`echo ${line##\#} | sed ${regex}` + cmd=$(printf %s\\n ${line##\#} | sed ${regex}) skip=0 if [ "x${skipregex}x" != "xx" ]; then - skip=`echo ${cmd} | grep -c -E ${skipregex}` + skip=`printf %s\\n ${cmd} | grep -c -E ${skipregex}` fi eval ${cmd} 2>&1 | tee ${tmpfile} 1>/dev/null @@ -135,10 +134,14 @@ function checkExpectedOutput() { touch ${tmpfile2} while [ 1 ]; do - read <&4 - line="${REPLY}" + read -r line <&4 - if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx" ]; then + case "${line}" in + '#'*) letter="#";; + *) letter="";; + esac + + if [ "x${letter}x" = "x#x" ] || [ "x${line}x" = "xx" ]; then if [ ${skip} -ne 0 ]; then break @@ -147,52 +150,53 @@ function checkExpectedOutput() { diff ${tmpfile} ${tmpfile2} >/dev/null if [ $? -ne 0 ]; then - if [ $((flags & FLAG_VERBOSE)) -ne 0 ]; then + if [ $(($flags & $FLAG_VERBOSE)) -ne 0 ]; then echo "FAIL ${xmlfile} : ${cmd}" diff ${tmpfile} ${tmpfile2} fi - ((failctr++)) - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + 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}" + [ $(($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++)) - [ $((flags & FLAG_VERBOSE)) -ne 0 ] && \ + passctr=$(($passctr + 1)) + [ $(($flags & $FLAG_VERBOSE)) -ne 0 ] && \ echo "PASS ${xmlfile} : ${cmd}" - [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ - test_result $((passctr+failctr)) "" 0 - [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \ - tap_pass $((passctr+failctr)) "${xmlfile} : ${cmd}" + [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \ + test_result $(($passctr + $failctr)) "" 0 + [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \ + tap_pass $(($passctr + $failctr)) "${xmlfile} : ${cmd}" fi break fi - echo "${line}" | sed ${regex} >> ${tmpfile2} + printf %s\\n "${line}" | sed ${regex} >> ${tmpfile2} done done exec 4>&- - rm -rf "${tmpfile}" "${tmpfile2}" 2>/dev/null + rm -rf "${tmpdir}" + + IFS="${OIFS}" } -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}." @@ -207,9 +211,10 @@ function doTest() { checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \ "${vm2name}" "${flags}" "" - if [ $((flags & FLAG_ATTACH)) -ne 0 ]; then + 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 +230,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}" \ @@ -237,7 +242,7 @@ EOF echo "FAIL: Detach of interface failed." fi else - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then # In case of TAP, run the test anyway so we get to the full number # of tests checkExpectedOutput "${xmlfile}" "${fwallfile}" "${ATTACH_IFNAME}" \ @@ -245,48 +250,45 @@ EOF checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \ "${vm2name}" "${flags}" #"(PRE|POST)ROUTING" fi - - ((attachfailctr++)) - if [ $((flags & FLAG_VERBOSE)) -ne 0 ]; then + + 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 + 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 - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + 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 @@ -296,9 +298,9 @@ function runTests() { "${vm2name}" "${flags}" done - if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - test_final $((passctr+failctr)) $failctr - elif [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ]; then + test_final $(($passctr + $failctr)) $failctr + elif [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then tap_final else echo "" @@ -310,14 +312,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'> @@ -363,33 +365,32 @@ EOF res=$(${VIRSH} start ${vmname}) if [ $? -ne 0 ]; then echo "Could not start VM ${vmname} : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi $(${VIRSH} undefine ${vmname}) return 1 fi - [ $((flags & FLAG_VERBOSE)) -ne 0 ] && echo "Created VM ${vmname}." + [ $(($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 + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -397,23 +398,23 @@ function destroyVM() { res=$(${VIRSH} undefine ${vmname}) if [ $? -ne 0 ]; then echo "Could not undefine VM ${vmname} : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi - [ $((flags & FLAG_VERBOSE)) -ne 0 ] && echo "Destroyed VM ${vmname}." + [ $(($flags & $FLAG_VERBOSE)) -ne 0 ] && echo "Destroyed VM ${vmname}." return 0 } -function createTestFilters() { - local flags="$1" - local tmpfile=`mktmpfile` - local res +createTestFilters() { + flags="$1" + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/tmpfile cat >${tmpfile} << EOF <filter name="${TESTFILTERNAME}"> @@ -431,11 +432,11 @@ EOF res=$(${VIRSH} nwfilter-define ${tmpfile}) if [ $? -ne 0 ]; then echo "Could not define filter : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 1 fi @@ -446,39 +447,38 @@ EOF res=$(${VIRSH} nwfilter-define ${tmpfile}) if [ $? -ne 0 ]; then echo "Could not define filter : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + 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 + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi res=$(${VIRSH} nwfilter-undefine tck-testcase 2>&1) if [ $? -ne 0 ]; then echo "Could not undefine filter : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -486,34 +486,32 @@ 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 done if [ `uname` != "Linux" ]; then - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; 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." @@ -521,21 +519,23 @@ function main() { exit 1; fi - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + 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 | 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,35 +550,34 @@ 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 done fi - if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd ${PWD} > /dev/null + if [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ]; then + curdir="${PWD}" . test-lib.sh if [ $? -ne 0 ]; then exit 1 fi test_intro $this_test - popd > /dev/null + cd "${curdir}" + [ $? -ne 0 ] && echo "cd failed" && exit 1 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) +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) +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 @@ -1,17 +1,17 @@ -#iptables -t nat -L -n | grep ' 10\\.1\\.2\\.' +#iptables -t nat -L -n | grep ' 10\.1\.2\.' MASQUERADE tcp -- 10.1.2.0/24 !10.1.2.0/24 masq ports: 1024-65535 MASQUERADE udp -- 10.1.2.0/24 !10.1.2.0/24 masq ports: 1024-65535 MASQUERADE all -- 10.1.2.0/24 !10.1.2.0/24 -#iptables -n -L FORWARD | grep ' 10\\.1\\.2\\.' +#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[[:print:]*]$|\1|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\\.' +#route -n | grep '10\.1\.2\.' 10.1.2.0 0.0.0.0 255.255.255.0 U 0 0 0 tck-testbr #brctl show | grep tck-testbr tck-testbr 8000.000000000000 yes -#ifconfig tck-testbr | grep ':10\\.1\\.2\\.' +#ifconfig tck-testbr | grep ':10\.1\.2\.' inet addr:10.1.2.1 Bcast:10.1.2.255 Mask:255.255.255.0 #virsh net-list | grep tck-testnet tck-testnet active no 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 @@ -1,14 +1,14 @@ -#iptables -L FORWARD -n | grep ' 10\\.1\\.2\\.' +#iptables -L FORWARD -n | grep ' 10\.1\.2\.' 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' +#iptables -t nat -L -n | grep ' 10\.1\.2\.' +#ps aux | sed -n '/dnsmasq .*10\.1\.2\./ s|.*$dnsmasq[[:print:]*]$|\1|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\\.' +#route -n | grep '10\.1\.2\.' 10.1.2.0 0.0.0.0 255.255.255.0 U 0 0 0 tck-testbr #brctl show | grep tck-testbr tck-testbr 8000.000000000000 yes -#ifconfig tck-testbr | grep ':10\\.1\\.2\\.' +#ifconfig tck-testbr | grep ':10\.1\.2\.' inet addr:10.1.2.1 Bcast:10.1.2.255 Mask:255.255.255.0 #virsh net-list | grep tck-testnet tck-testnet active no Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.post.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-1.post.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.post.dat @@ -1,7 +1,7 @@ -#iptables -t nat -L -n | grep ' 10\\.1\\.2\\.' -#iptables -n -L FORWARD | grep ' 10\\.1\\.2\\.' -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\$dnsmasq[[:print:]*]\$|\\1|p' -#route -n | grep '10\\.1\\.2\\.' +#iptables -t nat -L -n | grep ' 10\.1\.2\.' +#iptables -n -L FORWARD | grep ' 10\.1\.2\.' +#ps aux | sed -n '/dnsmasq .*10\.1\.2\./ s|.*$dnsmasq[[:print:]*]$|\1|p' +#route -n | grep '10\.1\.2\.' #brctl show | grep tck-testbr -#ifconfig tck-testbr 2>/dev/null| grep ':10\\.1\\.2\\.' +#ifconfig tck-testbr 2>/dev/null| grep ':10\.1\.2\.' #virsh net-list | grep tck-testnet Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-2.post.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-2.post.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-2.post.dat @@ -1,7 +1,7 @@ -#iptables -t nat -L -n | grep ' 10\\.1\\.2' -#iptables -n -L FORWARD | grep ' 10\\.1\\.2' -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\$dnsmasq[[:print:]*]\$|\\1|p' -#route -n | grep '10\\.1\\.2\\.' +#iptables -t nat -L -n | grep ' 10\.1\.2' +#iptables -n -L FORWARD | grep ' 10\.1\.2' +#ps aux | sed -n '/dnsmasq .*10\.1\.2\./ s|.*$dnsmasq[[:print:]*]$|\1|p' +#route -n | grep '10\.1\.2\.' #brctl show | grep tck-testbr -#ifconfig tck-testbr 2>/dev/null | grep ':10\\.1\\.2\\.' +#ifconfig tck-testbr 2>/dev/null | grep ':10\.1\.2\.' #virsh net-list | grep tck-testnet Index: libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/comment-test.fwall =================================================================== --- libvirt-tck.orig/scripts/nwfilter/nwfilterxml2fwallout/comment-test.fwall +++ libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/comment-test.fwall @@ -33,21 +33,21 @@ Chain FI-vnet0 (1 references) target prot opt source destination RETURN tcp ::/0 a:b:c::/128 /* tcp/ipv6 rule */ tcp spts:256:4369 dpts:32:33 state ESTABLISHED ctdir ORIGINAL RETURN udp ::/0 ::/0 /* `ls`;${COLUMNS};$(ls);"test";&'3 spaces' */ state ESTABLISHED ctdir ORIGINAL -RETURN sctp ::/0 ::/0 /* comment with lone ', `, ", `, \\, $x, and two spaces */ state ESTABLISHED ctdir ORIGINAL +RETURN sctp ::/0 ::/0 /* comment with lone ', `, ", `, \, $x, and two spaces */ state ESTABLISHED ctdir ORIGINAL RETURN ah ::/0 ::/0 /* tmp=`mktemp`; echo ${RANDOM} > ${tmp} ; cat < ${tmp}; rm -f ${tmp} */ state ESTABLISHED ctdir ORIGINAL #ip6tables -L FO-vnet0 -n Chain FO-vnet0 (1 references) target prot opt source destination ACCEPT tcp a:b:c::/128 ::/0 MAC 01:02:03:04:05:06 /* tcp/ipv6 rule */ tcp spts:32:33 dpts:256:4369 state NEW,ESTABLISHED ctdir REPLY ACCEPT udp ::/0 ::/0 /* `ls`;${COLUMNS};$(ls);"test";&'3 spaces' */ state NEW,ESTABLISHED ctdir REPLY -ACCEPT sctp ::/0 ::/0 /* comment with lone ', `, ", `, \\, $x, and two spaces */ state NEW,ESTABLISHED ctdir REPLY +ACCEPT sctp ::/0 ::/0 /* comment with lone ', `, ", `, \, $x, and two spaces */ state NEW,ESTABLISHED ctdir REPLY ACCEPT ah ::/0 ::/0 /* tmp=`mktemp`; echo ${RANDOM} > ${tmp} ; cat < ${tmp}; rm -f ${tmp} */ state NEW,ESTABLISHED ctdir REPLY #ip6tables -L HI-vnet0 -n Chain HI-vnet0 (1 references) target prot opt source destination RETURN tcp ::/0 a:b:c::/128 /* tcp/ipv6 rule */ tcp spts:256:4369 dpts:32:33 state ESTABLISHED ctdir ORIGINAL RETURN udp ::/0 ::/0 /* `ls`;${COLUMNS};$(ls);"test";&'3 spaces' */ state ESTABLISHED ctdir ORIGINAL -RETURN sctp ::/0 ::/0 /* comment with lone ', `, ", `, \\, $x, and two spaces */ state ESTABLISHED ctdir ORIGINAL +RETURN sctp ::/0 ::/0 /* comment with lone ', `, ", `, \, $x, and two spaces */ state ESTABLISHED ctdir ORIGINAL RETURN ah ::/0 ::/0 /* tmp=`mktemp`; echo ${RANDOM} > ${tmp} ; cat < ${tmp}; rm -f ${tmp} */ state ESTABLISHED ctdir ORIGINAL #ip6tables -L libvirt-host-in -n | grep vnet0 | tr -s " " HI-vnet0 all ::/0 ::/0 [goto] PHYSDEV match --physdev-in vnet0

2 2

[libvirt] [PATCH v2] deprecate fclose() and introduce VIR_{FORCE_}FCLOSE()
by Stefan Berger 15 Nov '10

15 Nov '10

V2: - following Eric's suggestion, deprecating fdclose(), introducing VIR_FDCLOSE() - following some other nits that Eric pointed out - replaced some more fclose () I previously had missed (last 2 files in patch) Similarly to deprecating close(), I am now deprecating fclose() and introduce VIR_FORCE_FCLOSE() and VIR_FCLOSE(). Also, fdopen() is replaced with VIR_FDOPEN(). Most of the files are opened in read-only mode, so usage of VIR_FORCE_CLOSE() seemed appropriate. Others that are opened in write mode already had the fclose() < 0 check and I converted those to VIR_FCLOSE() < 0. I did not find occurrences of possible double-closed files on the way. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- HACKING | 33 +++++++++++++++++++++++++++------ daemon/libvirtd.c | 6 +++--- docs/hacking.html.in | 36 ++++++++++++++++++++++++++++-------- src/libvirt_private.syms | 2 ++ src/nodeinfo.c | 8 ++++---- src/openvz/openvz_conf.c | 12 ++++++------ src/openvz/openvz_driver.c | 4 ++-- src/qemu/qemu_driver.c | 4 ++-- src/storage/storage_backend.c | 18 +++++++----------- src/storage/storage_backend_fs.c | 4 ++-- src/storage/storage_backend_iscsi.c | 9 +++------ src/storage/storage_backend_scsi.c | 2 +- src/uml/uml_driver.c | 8 ++++---- src/util/cgroup.c | 10 +++++----- src/util/dnsmasq.c | 5 +++-- src/util/files.c | 32 ++++++++++++++++++++++++++++++++ src/util/files.h | 6 ++++++ src/util/macvtap.c | 4 ++-- src/util/stats_linux.c | 3 ++- src/util/util.c | 10 ++++------ src/xen/block_stats.c | 3 ++- src/xen/xen_driver.c | 7 ++++--- src/xen/xen_hypervisor.c | 8 +++----- tests/nodeinfotest.c | 5 +++-- tests/testutils.c | 8 ++++---- tests/xencapstest.c | 7 +++---- 26 files changed, 164 insertions(+), 90 deletions(-) Index: libvirt-acl/src/util/files.c =================================================================== --- libvirt-acl.orig/src/util/files.c +++ libvirt-acl/src/util/files.c @@ -44,3 +44,35 @@ int virClose(int *fdptr, bool preserve_e return rc; } + + +int virFclose(FILE **file, bool preserve_errno) +{ + int saved_errno; + int rc = 0; + + if (*file) { + if (preserve_errno) + saved_errno = errno; + rc = fclose(*file); + *file = NULL; + if (preserve_errno) + errno = saved_errno; + } + + return rc; +} + + +FILE *virFdopen(int *fdptr, const char *mode) +{ + FILE *file = NULL; + + if (*fdptr >= 0) { + file = fdopen(*fdptr, mode); + if (file) + *fdptr = -1; + } + + return file; +} Index: libvirt-acl/src/util/files.h =================================================================== --- libvirt-acl.orig/src/util/files.h +++ libvirt-acl/src/util/files.h @@ -27,6 +27,7 @@ # define __VIR_FILES_H_ # include <stdbool.h> +# include <stdio.h> # include "internal.h" # include "ignore-value.h" @@ -34,13 +35,18 @@ /* Don't call this directly - use the macros below */ int virClose(int *fdptr, bool preserve_errno) ATTRIBUTE_RETURN_CHECK; +int virFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK; +FILE *virFdopen(int *fdptr, const char *mode); /* For use on normal paths; caller must check return value, and failure sets errno per close(). */ # define VIR_CLOSE(FD) virClose(&(FD), false) +# define VIR_FCLOSE(FILE) virFclose(&(FILE), false) +# define VIR_FDOPEN(FD, MODE) virFdopen(&(FD), MODE) /* For use on cleanup paths; errno is unaffected by close, and no return value to worry about. */ # define VIR_FORCE_CLOSE(FD) ignore_value(virClose(&(FD), true)) +# define VIR_FORCE_FCLOSE(FILE) ignore_value(virFclose(&(FILE), true)) #endif /* __VIR_FILES_H */ Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -344,6 +344,8 @@ virFDStreamCreateFile; # files.h virClose; +virFclose; +virFdopen; # hash.h Index: libvirt-acl/daemon/libvirtd.c =================================================================== --- libvirt-acl.orig/daemon/libvirtd.c +++ libvirt-acl/daemon/libvirtd.c @@ -512,7 +512,7 @@ static int qemudWritePidFile(const char return -1; } - if (!(fh = fdopen(fd, "w"))) { + if (!(fh = VIR_FDOPEN(fd, "w"))) { VIR_ERROR(_("Failed to fdopen pid file '%s' : %s"), pidFile, virStrerror(errno, ebuf, sizeof ebuf)); VIR_FORCE_CLOSE(fd); @@ -522,11 +522,11 @@ static int qemudWritePidFile(const char if (fprintf(fh, "%lu\n", (unsigned long)getpid()) < 0) { VIR_ERROR(_("%s: Failed to write to pid file '%s' : %s"), argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf)); - fclose(fh); + VIR_FORCE_FCLOSE(fh); return -1; } - if (fclose(fh) == EOF) { + if (VIR_FCLOSE(fh) == EOF) { VIR_ERROR(_("%s: Failed to close pid file '%s' : %s"), argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf)); return -1; Index: libvirt-acl/src/nodeinfo.c =================================================================== --- libvirt-acl.orig/src/nodeinfo.c +++ libvirt-acl/src/nodeinfo.c @@ -46,6 +46,7 @@ #include "virterror_internal.h" #include "count-one-bits.h" #include "intprops.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -102,8 +103,7 @@ get_cpu_value(unsigned int cpu, const ch } cleanup: - if (pathfp) - fclose(pathfp); + VIR_FORCE_FCLOSE(pathfp); VIR_FREE(path); return value; @@ -155,7 +155,7 @@ static unsigned long count_thread_siblin } cleanup: - fclose(pathfp); + VIR_FORCE_FCLOSE(pathfp); VIR_FREE(path); return ret; @@ -329,7 +329,7 @@ int nodeGetInfo(virConnectPtr conn ATTRI return -1; } ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo); - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); if (ret < 0) return -1; Index: libvirt-acl/src/openvz/openvz_conf.c =================================================================== --- libvirt-acl.orig/src/openvz/openvz_conf.c +++ libvirt-acl/src/openvz/openvz_conf.c @@ -528,7 +528,7 @@ int openvzLoadDomains(struct openvz_driv dom = NULL; } - fclose(fp); + VIR_FORCE_FCLOSE(fp); return 0; @@ -536,7 +536,7 @@ int openvzLoadDomains(struct openvz_driv virReportOOMError(); cleanup: - fclose(fp); + VIR_FORCE_FCLOSE(fp); if (dom) virDomainObjUnref(dom); return -1; @@ -906,10 +906,10 @@ openvzSetDefinedUUID(int vpsid, unsigned virUUIDFormat(uuid, uuidstr); - /* Record failure if fprintf or fclose fails, + /* Record failure if fprintf or VIR_FCLOSE fails, and be careful always to close the stream. */ - if ((fprintf(fp, "\n#UUID: %s\n", uuidstr) < 0) - + (fclose(fp) == EOF)) + if ((fprintf(fp, "\n#UUID: %s\n", uuidstr) < 0) || + (VIR_FCLOSE(fp) == EOF)) goto cleanup; } @@ -996,7 +996,7 @@ int openvzGetVEID(const char *name) { } ok = fscanf(fp, "%d\n", &veid ) == 1; - fclose(fp); + VIR_FORCE_FCLOSE(fp); if (ok && veid >= 0) return veid; Index: libvirt-acl/src/openvz/openvz_driver.c =================================================================== --- libvirt-acl.orig/src/openvz/openvz_driver.c +++ libvirt-acl/src/openvz/openvz_driver.c @@ -154,7 +154,7 @@ openvzDomainDefineCmd(const char *args[] max_veid = veid; } } - fclose(fp); + VIR_FORCE_FCLOSE(fp); if (max_veid == 0) { max_veid = 100; @@ -189,7 +189,7 @@ no_memory: return -1; cleanup: - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; #undef ADD_ARG Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -4588,7 +4588,7 @@ static int qemudGetProcessInfo(unsigned /* startstack -> processor */ "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d", &usertime, &systime, &cpu) != 3) { - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); VIR_WARN0("cannot parse process status data"); errno = -EINVAL; return -1; @@ -4608,7 +4608,7 @@ static int qemudGetProcessInfo(unsigned VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d", pid, tid, usertime, systime, cpu); - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); return 0; } Index: libvirt-acl/src/storage/storage_backend.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend.c +++ libvirt-acl/src/storage/storage_backend.c @@ -1398,7 +1398,7 @@ virStorageBackendRunProgRegex(virStorage goto cleanup; } - if ((list = fdopen(fd, "r")) == NULL) { + if ((list = VIR_FDOPEN(fd, "r")) == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot read fd")); goto cleanup; @@ -1458,10 +1458,8 @@ virStorageBackendRunProgRegex(virStorage VIR_FREE(reg); - if (list) - fclose(list); - else - VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(list); + VIR_FORCE_CLOSE(fd); while ((err = waitpid(child, &exitstatus, 0) == -1) && errno == EINTR); @@ -1531,9 +1529,9 @@ virStorageBackendRunProgNul(virStoragePo goto cleanup; } - if ((fp = fdopen(fd, "r")) == NULL) { + if ((fp = VIR_FDOPEN(fd, "r")) == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot read fd")); + "%s", _("cannot open file using fd")); goto cleanup; } @@ -1573,10 +1571,8 @@ virStorageBackendRunProgNul(virStoragePo VIR_FREE(v[i]); VIR_FREE(v); - if (fp) - fclose (fp); - else - VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(fp); + VIR_FORCE_CLOSE(fd); while ((w_err = waitpid (child, &exitstatus, 0) == -1) && errno == EINTR) /* empty */ ; Index: libvirt-acl/src/storage/storage_backend_fs.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_fs.c +++ libvirt-acl/src/storage/storage_backend_fs.c @@ -284,12 +284,12 @@ virStorageBackendFileSystemIsMounted(vir while ((getmntent_r(mtab, &ent, buf, sizeof(buf))) != NULL) { if (STREQ(ent.mnt_dir, pool->def->target.path)) { - fclose(mtab); + VIR_FORCE_FCLOSE(mtab); return 1; } } - fclose(mtab); + VIR_FORCE_FCLOSE(mtab); return 0; } Index: libvirt-acl/src/storage/storage_backend_iscsi.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_iscsi.c +++ libvirt-acl/src/storage/storage_backend_iscsi.c @@ -188,7 +188,7 @@ virStorageBackendIQNFound(virStoragePool goto out; } - if ((fp = fdopen(fd, "r")) == NULL) { + if ((fp = VIR_FDOPEN(fd, "r")) == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to open stream for file descriptor " "when reading output from '%s': '%s'"), @@ -235,11 +235,8 @@ out: } VIR_FREE(line); - if (fp != NULL) { - fclose(fp); - } else { - VIR_FORCE_CLOSE(fd); - } + VIR_FORCE_FCLOSE(fp); + VIR_FORCE_CLOSE(fd); return ret; } Index: libvirt-acl/src/storage/storage_backend_scsi.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_scsi.c +++ libvirt-acl/src/storage/storage_backend_scsi.c @@ -70,7 +70,7 @@ getDeviceType(uint32_t host, } gottype = fgets(typestr, 3, typefile); - fclose(typefile); + VIR_FORCE_FCLOSE(typefile); if (gottype == NULL) { virReportSystemError(errno, Index: libvirt-acl/src/uml/uml_driver.c =================================================================== --- libvirt-acl.orig/src/uml/uml_driver.c +++ libvirt-acl/src/uml/uml_driver.c @@ -587,11 +587,11 @@ reopen: if (fscanf(file, "%d", &vm->pid) != 1) { errno = EINVAL; - fclose(file); + VIR_FORCE_FCLOSE(file); goto cleanup; } - if (fclose(file) < 0) + if (VIR_FCLOSE(file) < 0) goto cleanup; rc = 0; @@ -1096,7 +1096,7 @@ static int umlGetProcessInfo(unsigned lo if (fscanf(pidinfo, "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu", &usertime, &systime) != 2) { umlDebug("not enough arg"); - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); return -1; } @@ -1109,7 +1109,7 @@ static int umlGetProcessInfo(unsigned lo umlDebug("Got %llu %llu %llu", usertime, systime, *cpuTime); - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); return 0; } Index: libvirt-acl/src/util/cgroup.c =================================================================== --- libvirt-acl.orig/src/util/cgroup.c +++ libvirt-acl/src/util/cgroup.c @@ -31,6 +31,7 @@ #include "memory.h" #include "cgroup.h" #include "logging.h" +#include "files.h" #define CGROUP_MAX_VAL 512 @@ -127,13 +128,12 @@ static int virCgroupDetectMounts(virCgro } } - fclose(mounts); + VIR_FORCE_FCLOSE(mounts); return 0; no_memory: - if (mounts) - fclose(mounts); + VIR_FORCE_FCLOSE(mounts); return -ENOMEM; } @@ -192,12 +192,12 @@ static int virCgroupDetectPlacement(virC } } - fclose(mapping); + VIR_FORCE_FCLOSE(mapping); return 0; no_memory: - fclose(mapping); + VIR_FORCE_FCLOSE(mapping); return -ENOMEM; } Index: libvirt-acl/src/util/dnsmasq.c =================================================================== --- libvirt-acl.orig/src/util/dnsmasq.c +++ libvirt-acl/src/util/dnsmasq.c @@ -44,6 +44,7 @@ #include "memory.h" #include "virterror_internal.h" #include "logging.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_NETWORK #define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile" @@ -171,7 +172,7 @@ hostsfileWrite(const char *path, for (i = 0; i < nhosts; i++) { if (fputs(hosts[i].host, f) == EOF || fputc('\n', f) == EOF) { rc = errno; - fclose(f); + VIR_FORCE_FCLOSE(f); if (istmp) unlink(tmp); @@ -180,7 +181,7 @@ hostsfileWrite(const char *path, } } - if (fclose(f) == EOF) { + if (VIR_FCLOSE(f) == EOF) { rc = errno; goto cleanup; } Index: libvirt-acl/src/util/macvtap.c =================================================================== --- libvirt-acl.orig/src/util/macvtap.c +++ libvirt-acl/src/util/macvtap.c @@ -436,11 +436,11 @@ int openTap(const char *ifname, virReportSystemError(errno, "%s",_("cannot determine macvtap's tap device " "interface index")); - fclose(file); + VIR_FORCE_FCLOSE(file); return -1; } - fclose(file); + VIR_FORCE_FCLOSE(file); if (snprintf(tapname, sizeof(tapname), "/dev/tap%d", ifindex) >= sizeof(tapname)) { Index: libvirt-acl/src/util/util.c =================================================================== --- libvirt-acl.orig/src/util/util.c +++ libvirt-acl/src/util/util.c @@ -1816,7 +1816,7 @@ int virFileWritePidPath(const char *pidf goto cleanup; } - if (!(file = fdopen(fd, "w"))) { + if (!(file = VIR_FDOPEN(fd, "w"))) { rc = errno; VIR_FORCE_CLOSE(fd); goto cleanup; @@ -1830,10 +1830,8 @@ int virFileWritePidPath(const char *pidf rc = 0; cleanup: - if (file && - fclose(file) < 0) { + if (VIR_FCLOSE(file) < 0) rc = errno; - } return rc; } @@ -1864,11 +1862,11 @@ int virFileReadPid(const char *dir, if (fscanf(file, "%d", pid) != 1) { rc = EINVAL; - fclose(file); + VIR_FORCE_FCLOSE(file); goto cleanup; } - if (fclose(file) < 0) { + if (VIR_FCLOSE(file) < 0) { rc = errno; goto cleanup; } Index: libvirt-acl/src/xen/xen_driver.c =================================================================== --- libvirt-acl.orig/src/xen/xen_driver.c +++ libvirt-acl/src/xen/xen_driver.c @@ -47,6 +47,7 @@ #include "pci.h" #include "uuid.h" #include "fdstream.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_XEN @@ -216,10 +217,10 @@ xenUnifiedProbe (void) return 1; #endif #ifdef __sun - FILE *fh; + int fd; - if (fh = fopen("/dev/xen/domcaps", "r")) { - fclose(fh); + if (fd = open("/dev/xen/domcaps", O_RDONLY)) { + VIR_FORCE_CLOSE(fd); return 1; } #endif Index: libvirt-acl/src/xen/xen_hypervisor.c =================================================================== --- libvirt-acl.orig/src/xen/xen_hypervisor.c +++ libvirt-acl/src/xen/xen_hypervisor.c @@ -2641,7 +2641,7 @@ xenHypervisorMakeCapabilities(virConnect capabilities = fopen ("/sys/hypervisor/properties/capabilities", "r"); if (capabilities == NULL) { if (errno != ENOENT) { - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); virReportSystemError(errno, _("cannot read file %s"), "/sys/hypervisor/properties/capabilities"); @@ -2654,10 +2654,8 @@ xenHypervisorMakeCapabilities(virConnect cpuinfo, capabilities); - if (cpuinfo) - fclose(cpuinfo); - if (capabilities) - fclose(capabilities); + VIR_FORCE_FCLOSE(cpuinfo); + VIR_FORCE_FCLOSE(capabilities); return caps; #endif /* __sun */ Index: libvirt-acl/tests/nodeinfotest.c =================================================================== --- libvirt-acl.orig/tests/nodeinfotest.c +++ libvirt-acl/tests/nodeinfotest.c @@ -9,6 +9,7 @@ #include "internal.h" #include "nodeinfo.h" #include "util.h" +#include "files.h" #ifndef __linux__ @@ -49,10 +50,10 @@ static int linuxTestCompareFiles(const c fprintf(stderr, "\n%s\n", error->message); virFreeError(error); } - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); return -1; } - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); /* 'nodes' is filled using libnuma.so from current machine * topology, which makes it unsuitable for the test suite Index: libvirt-acl/tests/testutils.c =================================================================== --- libvirt-acl.orig/tests/testutils.c +++ libvirt-acl/tests/testutils.c @@ -180,26 +180,26 @@ int virtTestLoadFile(const char *file, if (fstat(fileno(fp), &st) < 0) { fprintf (stderr, "%s: failed to fstat: %s\n", file, strerror(errno)); - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; } if (st.st_size > (buflen-1)) { fprintf (stderr, "%s: larger than buffer (> %d)\n", file, buflen-1); - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; } if (st.st_size) { if (fread(*buf, st.st_size, 1, fp) != 1) { fprintf (stderr, "%s: read failed: %s\n", file, strerror(errno)); - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; } } (*buf)[st.st_size] = '\0'; - fclose(fp); + VIR_FORCE_FCLOSE(fp); return st.st_size; } Index: libvirt-acl/tests/xencapstest.c =================================================================== --- libvirt-acl.orig/tests/xencapstest.c +++ libvirt-acl/tests/xencapstest.c @@ -9,6 +9,7 @@ #include "xml.h" #include "testutils.h" #include "xen/xen_hypervisor.h" +#include "files.h" static char *progname; static char *abs_srcdir; @@ -63,10 +64,8 @@ static int testCompareFiles(const char * fail: free(actualxml); - if (fp1) - fclose(fp1); - if (fp2) - fclose(fp2); + VIR_FORCE_FCLOSE(fp1); + VIR_FORCE_FCLOSE(fp2); virCapabilitiesFree(caps); return ret; Index: libvirt-acl/docs/hacking.html.in =================================================================== --- libvirt-acl.orig/docs/hacking.html.in +++ libvirt-acl/docs/hacking.html.in @@ -414,25 +414,45 @@ <h2><a name="file_handling">File handling</a></h2> <p> - Use of the close() API is deprecated in libvirt code base to help - avoiding double-closing of a file descriptor. Instead of this API, - use the macro from files.h + Usage of the <code>fdopen()</code>, <code>close()</code>, <code>fclose()</code> + APIs is deprecated in libvirt code base to help avoiding double-closing of files + or file descriptors, which is particulary dangerous in a multi-threaded + applications. Instead of these APIs, use the macros from files.h </p> - <ul> + <ul> + <li><p>eg opening a file from a file descriptor</p> + +<pre> + if ((file = VIR_FDOPEN(fd, "r")) == NULL) { + virReportSystemError(errno, "%s", + _("failed to open file from file descriptor")); + return -1; + } + /* fd is now invalid; only access the file using file variable */ +</pre></li> + <li><p>e.g. close a file descriptor</p> <pre> if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("failed to close file")); + virReportSystemError(errno, "%s", _("failed to close file")); } -</pre> - </li> +</pre></li> + + <li><p>eg close a file</p> + +<pre> + if (VIR_FCLOSE(file) < 0) { + virReportSystemError(errno, "%s", _("failed to close file")); + } +</pre></li> - <li><p>eg close a file descriptor in an error path, without losing + <li><p>eg close a file or file descriptor in an error path, without losing the previous <code>errno</code> value</p> <pre> VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(file); </pre> </li> </ul> Index: libvirt-acl/HACKING =================================================================== --- libvirt-acl.orig/HACKING +++ libvirt-acl/HACKING @@ -339,22 +339,43 @@ routines, use the macros from memory.h File handling ============= -Use of the close() API is deprecated in libvirt code base to help avoiding -double-closing of a file descriptor. Instead of this API, use the macro from -files.h +Usage of the "fdopen()", "close()", "fclose()" APIs is deprecated in libvirt +code base to help avoiding double-closing of files or file descriptors, which +is particulary dangerous in a multi-threaded applications. Instead of these +APIs, use the macros from files.h + +- eg opening a file from a file descriptor + + if ((file = VIR_FDOPEN(fd, "r")) == NULL) { + virReportSystemError(errno, "%s", + _("failed to open file from file descriptor")); + return -1; + } + /* fd is now invalid; only access the file using file variable */ + + - e.g. close a file descriptor if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("failed to close file")); + virReportSystemError(errno, "%s", _("failed to close file")); + } + + + +- eg close a file + + if (VIR_FCLOSE(file) < 0) { + virReportSystemError(errno, "%s", _("failed to close file")); } -- eg close a file descriptor in an error path, without losing the previous -"errno" value +- eg close a file or file descriptor in an error path, without losing the +previous "errno" value VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(file); Index: libvirt-acl/src/util/stats_linux.c =================================================================== --- libvirt-acl.orig/src/util/stats_linux.c +++ libvirt-acl/src/util/stats_linux.c @@ -25,6 +25,7 @@ # include "util.h" # include "stats_linux.h" # include "memory.h" +# include "files.h" # define VIR_FROM_THIS VIR_FROM_STATS_LINUX @@ -103,7 +104,7 @@ linuxDomainInterfaceStats(const char *pa return 0; } } - fclose (fp); + VIR_FORCE_FCLOSE(fp); virStatsError(VIR_ERR_INTERNAL_ERROR, "/proc/net/dev: Interface not found"); Index: libvirt-acl/src/xen/block_stats.c =================================================================== --- libvirt-acl.orig/src/xen/block_stats.c +++ libvirt-acl/src/xen/block_stats.c @@ -27,6 +27,7 @@ # include "util.h" # include "block_stats.h" # include "memory.h" +# include "files.h" # define VIR_FROM_THIS VIR_FROM_STATS_LINUX @@ -100,7 +101,7 @@ read_stat (const char *path) /* read, but don't bail out before closing */ i = fread (str, 1, sizeof str - 1, fp); - if (fclose (fp) != 0 /* disk error */ + if (VIR_FCLOSE(fp) != 0 /* disk error */ || i < 1) /* ensure we read at least one byte */ return -1;

2 1