* [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes
@ 2015-10-30 19:25 Jeff Cody
2015-10-30 19:25 ` [Qemu-devel] [PATCH v3 1/2] " Jeff Cody
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jeff Cody @ 2015-10-30 19:25 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jsnow, qemu-block, mreitz
Changes from v2:
* Pulled patch 2 into this series
* Patch 1: Moved non-empty test conditionals inside
pid file existance check (thanks Max)
Added a fix in for patch 058, for its
self-launched qemu-nbd instance (thanks Max)
* Patch 2: Only print the valgrind logs if the exit error
matches (thanks Max)
Changes from v1:
* use 'read' instead of 'cat' (thanks Eric)
* quote variable in variable test (thanks Eric)
Jeff Cody (2):
qemu-iotests: fix cleanup of background processes
qemu-iotests: fix -valgrind option for check
tests/qemu-iotests/039.out | 30 +++++++++++++++++++++++++-----
tests/qemu-iotests/058 | 12 ++++++++----
tests/qemu-iotests/061.out | 12 ++++++++++--
tests/qemu-iotests/137.out | 6 +++++-
tests/qemu-iotests/common | 9 ++-------
tests/qemu-iotests/common.config | 32 +++++++++++++++++++++++++++++---
tests/qemu-iotests/common.qemu | 18 ++++++++++++------
tests/qemu-iotests/common.rc | 18 +++++-------------
8 files changed, 96 insertions(+), 41 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH v3 1/2] qemu-iotests: fix cleanup of background processes 2015-10-30 19:25 [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes Jeff Cody @ 2015-10-30 19:25 ` Jeff Cody 2015-10-30 21:13 ` Max Reitz 2015-10-30 19:25 ` [Qemu-devel] [PATCH v3 2/2] qemu-iotests: fix -valgrind option for check Jeff Cody 2015-11-02 20:31 ` [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes Max Reitz 2 siblings, 1 reply; 10+ messages in thread From: Jeff Cody @ 2015-10-30 19:25 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, jsnow, qemu-block, mreitz Commit 934659c switched the iotests to run qemu and qemu-nbd from a bash subshell, in order to catch segfaults. Unfortunately, this means the process PID cannot be captured via '$!'. We stopped killing qemu and qemu-nbd processes, leaving a lot of orphaned, running qemu processes after executing iotests. Since the process is using exec in the subshell, the PID is the same as the subshell PID. Track these PIDs for cleanup using pidfiles in the $TEST_DIR. Only track the qemu PID, however, if requested - not all usage requires killing the process. Reported-by: John Snow <jsnow@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> --- tests/qemu-iotests/058 | 12 ++++++++---- tests/qemu-iotests/common.config | 14 ++++++++++++-- tests/qemu-iotests/common.qemu | 18 ++++++++++++------ tests/qemu-iotests/common.rc | 8 +++++--- 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058 index f2bdd0b..63a6598 100755 --- a/tests/qemu-iotests/058 +++ b/tests/qemu-iotests/058 @@ -32,11 +32,17 @@ status=1 # failure is the default! nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket nbd_snapshot_img="nbd:unix:$nbd_unix_socket" +rm -f "${TEST_DIR}/qemu-nbd.pid" _cleanup_nbd() { - if [ -n "$NBD_SNAPSHOT_PID" ]; then - kill "$NBD_SNAPSHOT_PID" + local NBD_SNAPSHOT_PID + if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then + read NBD_SNAPSHOT_PID < "${TEST_DIR}/qemu-nbd.pid" + rm -f "${TEST_DIR}/qemu-nbd.pid" + if [ -n "$NBD_SNAPSHOT_PID" ]; then + kill "$NBD_SNAPSHOT_PID" + fi fi rm -f "$nbd_unix_socket" } @@ -60,7 +66,6 @@ _export_nbd_snapshot() { _cleanup_nbd $QEMU_NBD -v -t -k "$nbd_unix_socket" "$TEST_IMG" -l $1 & - NBD_SNAPSHOT_PID=$! _wait_for_nbd } @@ -68,7 +73,6 @@ _export_nbd_snapshot1() { _cleanup_nbd $QEMU_NBD -v -t -k "$nbd_unix_socket" "$TEST_IMG" -l snapshot.name=$1 & - NBD_SNAPSHOT_PID=$! _wait_for_nbd } diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index 596bb2b..4d8665f 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -44,6 +44,8 @@ export HOST_OPTIONS=${HOST_OPTIONS:=local.config} export CHECK_OPTIONS=${CHECK_OPTIONS:="-g auto"} export PWD=`pwd` +export _QEMU_HANDLE=0 + # $1 = prog to look for, $2* = default pathnames if not found in $PATH set_prog_path() { @@ -105,7 +107,12 @@ fi _qemu_wrapper() { - (exec "$QEMU_PROG" $QEMU_OPTIONS "$@") + ( + if [ ! -z "${QEMU_NEED_PID}" ]; then + echo $BASHPID > "${TEST_DIR}/qemu-${_QEMU_HANDLE}.pid" + fi + exec "$QEMU_PROG" $QEMU_OPTIONS "$@" + ) } _qemu_img_wrapper() @@ -120,7 +127,10 @@ _qemu_io_wrapper() _qemu_nbd_wrapper() { - (exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@") + ( + echo $BASHPID > "${TEST_DIR}/qemu-nbd.pid" + exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@" + ) } export QEMU=_qemu_wrapper diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu index e3faa53..5db0611 100644 --- a/tests/qemu-iotests/common.qemu +++ b/tests/qemu-iotests/common.qemu @@ -30,8 +30,6 @@ QEMU_COMM_TIMEOUT=10 QEMU_FIFO_IN="${TEST_DIR}/qmp-in-$$" QEMU_FIFO_OUT="${TEST_DIR}/qmp-out-$$" -QEMU_PID= -_QEMU_HANDLE=0 QEMU_HANDLE=0 # If bash version is >= 4.1, these will be overwritten and dynamic @@ -153,11 +151,11 @@ function _launch_qemu() mkfifo "${fifo_out}" mkfifo "${fifo_in}" + QEMU_NEED_PID='y'\ ${QEMU} -nographic -serial none ${comm} -machine accel=qtest "${@}" \ >"${fifo_out}" \ 2>&1 \ <"${fifo_in}" & - QEMU_PID[${_QEMU_HANDLE}]=$! if [[ "${BASH_VERSINFO[0]}" -ge "5" || ("${BASH_VERSINFO[0]}" -ge "4" && "${BASH_VERSINFO[1]}" -ge "1") ]] @@ -196,10 +194,18 @@ function _cleanup_qemu() # QEMU_PID[], QEMU_IN[], QEMU_OUT[] all use same indices for i in "${!QEMU_OUT[@]}" do - if [ -z "${wait}" ]; then - kill -KILL ${QEMU_PID[$i]} 2>/dev/null + local QEMU_PID + if [ -f "${TEST_DIR}/qemu-${i}.pid" ]; then + read QEMU_PID < "${TEST_DIR}/qemu-${i}.pid" + rm -f "${TEST_DIR}/qemu-${i}.pid" + if [ -z "${wait}" ] && [ ! -z "${QEMU_PID}" ]; then + kill -KILL ${QEMU_PID} 2>/dev/null + fi + if [ ! -z "${QEMU_PID}" ]; then + wait ${QEMU_PID} 2>/dev/null # silent kill + fi fi - wait ${QEMU_PID[$i]} 2>/dev/null # silent kill + if [ -n "${wait}" ]; then cat <&${QEMU_OUT[$i]} | _filter_testdir | _filter_qemu \ | _filter_qemu_io | _filter_qmp diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 28e4bea..4878e99 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -154,7 +154,6 @@ _make_test_img() # Start an NBD server on the image file, which is what we'll be talking to if [ $IMGPROTO = "nbd" ]; then eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT $TEST_IMG_FILE &" - QEMU_NBD_PID=$! sleep 1 # FIXME: qemu-nbd needs to be listening before we continue fi } @@ -175,8 +174,11 @@ _cleanup_test_img() case "$IMGPROTO" in nbd) - if [ -n "$QEMU_NBD_PID" ]; then - kill $QEMU_NBD_PID + if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then + local QEMU_NBD_PID + read QEMU_NBD_PID < "${TEST_DIR}/qemu-nbd.pid" + kill ${QEMU_NBD_PID} + rm -f "${TEST_DIR}/qemu-nbd.pid" fi rm -f "$TEST_IMG_FILE" ;; -- 1.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] qemu-iotests: fix cleanup of background processes 2015-10-30 19:25 ` [Qemu-devel] [PATCH v3 1/2] " Jeff Cody @ 2015-10-30 21:13 ` Max Reitz 2015-11-02 7:37 ` Markus Armbruster 0 siblings, 1 reply; 10+ messages in thread From: Max Reitz @ 2015-10-30 21:13 UTC (permalink / raw) To: Jeff Cody, qemu-devel; +Cc: kwolf, jsnow, qemu-block [-- Attachment #1: Type: text/plain, Size: 1957 bytes --] On 30.10.2015 20:25, Jeff Cody wrote: > Commit 934659c switched the iotests to run qemu and qemu-nbd from a bash > subshell, in order to catch segfaults. Unfortunately, this means the > process PID cannot be captured via '$!'. We stopped killing qemu and > qemu-nbd processes, leaving a lot of orphaned, running qemu processes > after executing iotests. > > Since the process is using exec in the subshell, the PID is the > same as the subshell PID. > > Track these PIDs for cleanup using pidfiles in the $TEST_DIR. Only > track the qemu PID, however, if requested - not all usage requires > killing the process. > > Reported-by: John Snow <jsnow@redhat.com> > Signed-off-by: Jeff Cody <jcody@redhat.com> > --- > tests/qemu-iotests/058 | 12 ++++++++---- > tests/qemu-iotests/common.config | 14 ++++++++++++-- > tests/qemu-iotests/common.qemu | 18 ++++++++++++------ > tests/qemu-iotests/common.rc | 8 +++++--- > 4 files changed, 37 insertions(+), 15 deletions(-) > > diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058 > index f2bdd0b..63a6598 100755 > --- a/tests/qemu-iotests/058 > +++ b/tests/qemu-iotests/058 > @@ -32,11 +32,17 @@ status=1 # failure is the default! > > nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket > nbd_snapshot_img="nbd:unix:$nbd_unix_socket" > +rm -f "${TEST_DIR}/qemu-nbd.pid" > > _cleanup_nbd() > { > - if [ -n "$NBD_SNAPSHOT_PID" ]; then > - kill "$NBD_SNAPSHOT_PID" > + local NBD_SNAPSHOT_PID > + if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then > + read NBD_SNAPSHOT_PID < "${TEST_DIR}/qemu-nbd.pid" > + rm -f "${TEST_DIR}/qemu-nbd.pid" > + if [ -n "$NBD_SNAPSHOT_PID" ]; then No, I won't complain about using ! -z "" elsewhere and -n "" here. :-) Reviewed-by: Max Reitz <mreitz@redhat.com> > + kill "$NBD_SNAPSHOT_PID" > + fi > fi > rm -f "$nbd_unix_socket" > } [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] qemu-iotests: fix cleanup of background processes 2015-10-30 21:13 ` Max Reitz @ 2015-11-02 7:37 ` Markus Armbruster 2015-11-02 16:03 ` Max Reitz 0 siblings, 1 reply; 10+ messages in thread From: Markus Armbruster @ 2015-11-02 7:37 UTC (permalink / raw) To: Max Reitz; +Cc: kwolf, Jeff Cody, jsnow, qemu-devel, qemu-block Max Reitz <mreitz@redhat.com> writes: > On 30.10.2015 20:25, Jeff Cody wrote: >> Commit 934659c switched the iotests to run qemu and qemu-nbd from a bash >> subshell, in order to catch segfaults. Unfortunately, this means the >> process PID cannot be captured via '$!'. We stopped killing qemu and >> qemu-nbd processes, leaving a lot of orphaned, running qemu processes >> after executing iotests. >> >> Since the process is using exec in the subshell, the PID is the >> same as the subshell PID. >> >> Track these PIDs for cleanup using pidfiles in the $TEST_DIR. Only >> track the qemu PID, however, if requested - not all usage requires >> killing the process. >> >> Reported-by: John Snow <jsnow@redhat.com> >> Signed-off-by: Jeff Cody <jcody@redhat.com> >> --- >> tests/qemu-iotests/058 | 12 ++++++++---- >> tests/qemu-iotests/common.config | 14 ++++++++++++-- >> tests/qemu-iotests/common.qemu | 18 ++++++++++++------ >> tests/qemu-iotests/common.rc | 8 +++++--- >> 4 files changed, 37 insertions(+), 15 deletions(-) >> >> diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058 >> index f2bdd0b..63a6598 100755 >> --- a/tests/qemu-iotests/058 >> +++ b/tests/qemu-iotests/058 >> @@ -32,11 +32,17 @@ status=1 # failure is the default! >> >> nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket >> nbd_snapshot_img="nbd:unix:$nbd_unix_socket" >> +rm -f "${TEST_DIR}/qemu-nbd.pid" >> >> _cleanup_nbd() >> { >> - if [ -n "$NBD_SNAPSHOT_PID" ]; then >> - kill "$NBD_SNAPSHOT_PID" >> + local NBD_SNAPSHOT_PID >> + if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then >> + read NBD_SNAPSHOT_PID < "${TEST_DIR}/qemu-nbd.pid" >> + rm -f "${TEST_DIR}/qemu-nbd.pid" >> + if [ -n "$NBD_SNAPSHOT_PID" ]; then > > No, I won't complain about using ! -z "" elsewhere and -n "" here. :-) The little pedant in me screams "but I will!", and the little prankster next to him is clapping enthusiastically. Kidding aside: not worth a respin, but could be cleaned up on commit (maintainer's discretion). > Reviewed-by: Max Reitz <mreitz@redhat.com> > >> + kill "$NBD_SNAPSHOT_PID" >> + fi >> fi >> rm -f "$nbd_unix_socket" >> } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] qemu-iotests: fix cleanup of background processes 2015-11-02 7:37 ` Markus Armbruster @ 2015-11-02 16:03 ` Max Reitz 2015-11-02 16:32 ` Eric Blake 0 siblings, 1 reply; 10+ messages in thread From: Max Reitz @ 2015-11-02 16:03 UTC (permalink / raw) To: Markus Armbruster; +Cc: kwolf, Jeff Cody, jsnow, qemu-devel, qemu-block [-- Attachment #1: Type: text/plain, Size: 2596 bytes --] On 02.11.2015 08:37, Markus Armbruster wrote: > Max Reitz <mreitz@redhat.com> writes: > >> On 30.10.2015 20:25, Jeff Cody wrote: >>> Commit 934659c switched the iotests to run qemu and qemu-nbd from a bash >>> subshell, in order to catch segfaults. Unfortunately, this means the >>> process PID cannot be captured via '$!'. We stopped killing qemu and >>> qemu-nbd processes, leaving a lot of orphaned, running qemu processes >>> after executing iotests. >>> >>> Since the process is using exec in the subshell, the PID is the >>> same as the subshell PID. >>> >>> Track these PIDs for cleanup using pidfiles in the $TEST_DIR. Only >>> track the qemu PID, however, if requested - not all usage requires >>> killing the process. >>> >>> Reported-by: John Snow <jsnow@redhat.com> >>> Signed-off-by: Jeff Cody <jcody@redhat.com> >>> --- >>> tests/qemu-iotests/058 | 12 ++++++++---- >>> tests/qemu-iotests/common.config | 14 ++++++++++++-- >>> tests/qemu-iotests/common.qemu | 18 ++++++++++++------ >>> tests/qemu-iotests/common.rc | 8 +++++--- >>> 4 files changed, 37 insertions(+), 15 deletions(-) >>> >>> diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058 >>> index f2bdd0b..63a6598 100755 >>> --- a/tests/qemu-iotests/058 >>> +++ b/tests/qemu-iotests/058 >>> @@ -32,11 +32,17 @@ status=1 # failure is the default! >>> >>> nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket >>> nbd_snapshot_img="nbd:unix:$nbd_unix_socket" >>> +rm -f "${TEST_DIR}/qemu-nbd.pid" >>> >>> _cleanup_nbd() >>> { >>> - if [ -n "$NBD_SNAPSHOT_PID" ]; then >>> - kill "$NBD_SNAPSHOT_PID" >>> + local NBD_SNAPSHOT_PID >>> + if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then >>> + read NBD_SNAPSHOT_PID < "${TEST_DIR}/qemu-nbd.pid" >>> + rm -f "${TEST_DIR}/qemu-nbd.pid" >>> + if [ -n "$NBD_SNAPSHOT_PID" ]; then >> >> No, I won't complain about using ! -z "" elsewhere and -n "" here. :-) > > The little pedant in me screams "but I will!", and the little prankster > next to him is clapping enthusiastically. > > Kidding aside: not worth a respin, but could be cleaned up on commit > (maintainer's discretion). Oh, if that's the case, then I have another thing for you: The use of == in patch 2! ;-) (I'm a bit disappointed Eric doesn't have a mail filter for #!/bin/(ba)?sh ... if.*== for his mail client.) Max > >> Reviewed-by: Max Reitz <mreitz@redhat.com> >> >>> + kill "$NBD_SNAPSHOT_PID" >>> + fi >>> fi >>> rm -f "$nbd_unix_socket" >>> } [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] qemu-iotests: fix cleanup of background processes 2015-11-02 16:03 ` Max Reitz @ 2015-11-02 16:32 ` Eric Blake 0 siblings, 0 replies; 10+ messages in thread From: Eric Blake @ 2015-11-02 16:32 UTC (permalink / raw) To: Max Reitz, Markus Armbruster Cc: kwolf, Jeff Cody, jsnow, qemu-devel, qemu-block [-- Attachment #1: Type: text/plain, Size: 1659 bytes --] On 11/02/2015 09:03 AM, Max Reitz wrote: >>>> +++ b/tests/qemu-iotests/058 >>>> @@ -32,11 +32,17 @@ status=1 # failure is the default! >>>> >>>> nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket >>>> nbd_snapshot_img="nbd:unix:$nbd_unix_socket" >>>> +rm -f "${TEST_DIR}/qemu-nbd.pid" >>>> >>>> _cleanup_nbd() >>>> { >>>> - if [ -n "$NBD_SNAPSHOT_PID" ]; then >>>> - kill "$NBD_SNAPSHOT_PID" >>>> + local NBD_SNAPSHOT_PID >>>> + if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then >>>> + read NBD_SNAPSHOT_PID < "${TEST_DIR}/qemu-nbd.pid" >>>> + rm -f "${TEST_DIR}/qemu-nbd.pid" >>>> + if [ -n "$NBD_SNAPSHOT_PID" ]; then >>> >>> No, I won't complain about using ! -z "" elsewhere and -n "" here. :-) >> >> The little pedant in me screams "but I will!", and the little prankster >> next to him is clapping enthusiastically. >> >> Kidding aside: not worth a respin, but could be cleaned up on commit >> (maintainer's discretion). > > Oh, if that's the case, then I have another thing for you: The use of == > in patch 2! ;-) > > (I'm a bit disappointed Eric doesn't have a mail filter for > #!/bin/(ba)?sh ... if.*== for his mail client.) I already know that most (if not all) of qemu-iotests is specifically /bin/bash. But if we want to, we can ditch -n and -z, and just use: if [[ $NBD_SNAPSHOT_PID ]]; then and similarly. In fact, I actually prefer embracing bash-isms when we know we are using bash, to make it obvious that we know we are not catering to /bin/sh. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] qemu-iotests: fix -valgrind option for check 2015-10-30 19:25 [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes Jeff Cody 2015-10-30 19:25 ` [Qemu-devel] [PATCH v3 1/2] " Jeff Cody @ 2015-10-30 19:25 ` Jeff Cody 2015-10-30 21:14 ` Max Reitz 2015-11-02 20:31 ` [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes Max Reitz 2 siblings, 1 reply; 10+ messages in thread From: Jeff Cody @ 2015-10-30 19:25 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, jsnow, qemu-block, mreitz Commit 934659c switched the iotests to run qemu-io from a bash subshell, in order to catch segfaults. This method is incompatible with the current valgrind_qemu_io() bash function. Move the valgrind usage into the exec subshell in _qemu_io_wrapper(), while making sure the original return value is passed back to the caller. Update test output for tests 039, 061, and 137 as it looks for the specific subshell command when the process is terminated. Reported-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> --- tests/qemu-iotests/039.out | 30 +++++++++++++++++++++++++----- tests/qemu-iotests/061.out | 12 ++++++++++-- tests/qemu-iotests/137.out | 6 +++++- tests/qemu-iotests/common | 9 ++------- tests/qemu-iotests/common.config | 18 +++++++++++++++++- tests/qemu-iotests/common.rc | 10 ---------- 6 files changed, 59 insertions(+), 26 deletions(-) diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out index 03a31c5..32c8846 100644 --- a/tests/qemu-iotests/039.out +++ b/tests/qemu-iotests/039.out @@ -11,7 +11,11 @@ No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x1 ERROR cluster 5 refcount=0 reference=1 ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0 @@ -46,7 +50,11 @@ read 512/512 bytes at offset 0 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x1 ERROR cluster 5 refcount=0 reference=1 Rebuilding refcount structure @@ -60,7 +68,11 @@ incompatible_features 0x0 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x0 No errors were found on the image. @@ -79,7 +91,11 @@ No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x1 ERROR cluster 5 refcount=0 reference=1 ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0 @@ -89,7 +105,11 @@ Data may be corrupted, or further writes to the image may corrupt it. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x0 No errors were found on the image. *** done diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out index b16bea9..f2598a8 100644 --- a/tests/qemu-iotests/061.out +++ b/tests/qemu-iotests/061.out @@ -57,7 +57,11 @@ No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 wrote 131072/131072 bytes at offset 0 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) magic 0x514649fb version 3 backing_file_offset 0x0 @@ -215,7 +219,11 @@ No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 wrote 131072/131072 bytes at offset 0 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) magic 0x514649fb version 3 backing_file_offset 0x0 diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out index cf55a41..88c702c 100644 --- a/tests/qemu-iotests/137.out +++ b/tests/qemu-iotests/137.out @@ -31,7 +31,11 @@ Cache clean interval too big Unsupported value 'blubb' for qcow2 option 'overlap-check'. Allowed are any of the following: none, constant, cached, all wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./common.config: Killed ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" ) +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"; +fi ) incompatible_features 0x0 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 wrote 65536/65536 bytes at offset 0 diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common index 25c351b..ff84f4b 100644 --- a/tests/qemu-iotests/common +++ b/tests/qemu-iotests/common @@ -41,7 +41,6 @@ sortme=false expunge=true have_test_arg=false randomize=false -valgrind=false cachemode=false rm -f $tmp.list $tmp.tmp $tmp.sed @@ -53,6 +52,7 @@ export CACHEMODE="writeback" export QEMU_IO_OPTIONS="" export CACHEMODE_IS_DEFAULT=true export QEMU_OPTIONS="-nodefaults" +export VALGRIND_QEMU= for r do @@ -278,7 +278,7 @@ testlist options ;; -valgrind) - valgrind=true + VALGRIND_QEMU='y' xpand=false ;; @@ -436,8 +436,3 @@ fi if [ "$IMGPROTO" = "nbd" ] ; then [ "$QEMU_NBD" = "" ] && _fatal "qemu-nbd not found" fi - -if $valgrind; then - export REAL_QEMU_IO="$QEMU_IO_PROG" - export QEMU_IO_PROG=valgrind_qemu_io -fi diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index 4d8665f..7d7650b 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -122,7 +122,23 @@ _qemu_img_wrapper() _qemu_io_wrapper() { - (exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@") + local VALGRIND_LOGFILE=/tmp/$$.valgrind + local RETVAL + ( + if [ "${VALGRIND_QEMU}" == "y" ]; then + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" + else + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" + fi + ) + RETVAL=$? + if [ "${VALGRIND_QEMU}" == "y" ]; then + if [ $RETVAL == 99 ]; then + cat "${VALGRIND_LOGFILE}" + fi + rm -f "${VALGRIND_LOGFILE}" + fi + (exit $RETVAL) } _qemu_nbd_wrapper() diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 4878e99..d9913f8 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -70,16 +70,6 @@ else TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT fi -function valgrind_qemu_io() -{ - valgrind --log-file=/tmp/$$.valgrind --error-exitcode=99 $REAL_QEMU_IO "$@" - if [ $? != 0 ]; then - cat /tmp/$$.valgrind - fi - rm -f /tmp/$$.valgrind -} - - _optstr_add() { if [ -n "$1" ]; then -- 1.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] qemu-iotests: fix -valgrind option for check 2015-10-30 19:25 ` [Qemu-devel] [PATCH v3 2/2] qemu-iotests: fix -valgrind option for check Jeff Cody @ 2015-10-30 21:14 ` Max Reitz 0 siblings, 0 replies; 10+ messages in thread From: Max Reitz @ 2015-10-30 21:14 UTC (permalink / raw) To: Jeff Cody, qemu-devel; +Cc: kwolf, jsnow, qemu-block [-- Attachment #1: Type: text/plain, Size: 1072 bytes --] On 30.10.2015 20:25, Jeff Cody wrote: > Commit 934659c switched the iotests to run qemu-io from a bash subshell, > in order to catch segfaults. This method is incompatible with the > current valgrind_qemu_io() bash function. > > Move the valgrind usage into the exec subshell in _qemu_io_wrapper(), > while making sure the original return value is passed back to the > caller. > > Update test output for tests 039, 061, and 137 as it looks for the > specific subshell command when the process is terminated. > > Reported-by: Kevin Wolf <kwolf@redhat.com> > Signed-off-by: Jeff Cody <jcody@redhat.com> > --- > tests/qemu-iotests/039.out | 30 +++++++++++++++++++++++++----- > tests/qemu-iotests/061.out | 12 ++++++++++-- > tests/qemu-iotests/137.out | 6 +++++- > tests/qemu-iotests/common | 9 ++------- > tests/qemu-iotests/common.config | 18 +++++++++++++++++- > tests/qemu-iotests/common.rc | 10 ---------- > 6 files changed, 59 insertions(+), 26 deletions(-) Reviewed-by: Max Reitz <mreitz@redhat.com> [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes 2015-10-30 19:25 [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes Jeff Cody 2015-10-30 19:25 ` [Qemu-devel] [PATCH v3 1/2] " Jeff Cody 2015-10-30 19:25 ` [Qemu-devel] [PATCH v3 2/2] qemu-iotests: fix -valgrind option for check Jeff Cody @ 2015-11-02 20:31 ` Max Reitz 2015-11-02 20:35 ` Max Reitz 2 siblings, 1 reply; 10+ messages in thread From: Max Reitz @ 2015-11-02 20:31 UTC (permalink / raw) To: Jeff Cody, qemu-devel; +Cc: kwolf, jsnow, qemu-block [-- Attachment #1: Type: text/plain, Size: 1378 bytes --] On 30.10.2015 20:25, Jeff Cody wrote: > Changes from v2: > > * Pulled patch 2 into this series > > * Patch 1: Moved non-empty test conditionals inside > pid file existance check (thanks Max) > > Added a fix in for patch 058, for its > self-launched qemu-nbd instance (thanks Max) > > * Patch 2: Only print the valgrind logs if the exit error > matches (thanks Max) > > Changes from v1: > > * use 'read' instead of 'cat' (thanks Eric) > * quote variable in variable test (thanks Eric) > > Jeff Cody (2): > qemu-iotests: fix cleanup of background processes > qemu-iotests: fix -valgrind option for check > > tests/qemu-iotests/039.out | 30 +++++++++++++++++++++++++----- > tests/qemu-iotests/058 | 12 ++++++++---- > tests/qemu-iotests/061.out | 12 ++++++++++-- > tests/qemu-iotests/137.out | 6 +++++- > tests/qemu-iotests/common | 9 ++------- > tests/qemu-iotests/common.config | 32 +++++++++++++++++++++++++++++--- > tests/qemu-iotests/common.qemu | 18 ++++++++++++------ > tests/qemu-iotests/common.rc | 18 +++++------------- > 8 files changed, 96 insertions(+), 41 deletions(-) Thanks, applied to my block tree with the three occurrences of ! -n in patch 1 changed to -z: https://github.com/XanClic/qemu/commits/block Max [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes 2015-11-02 20:31 ` [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes Max Reitz @ 2015-11-02 20:35 ` Max Reitz 0 siblings, 0 replies; 10+ messages in thread From: Max Reitz @ 2015-11-02 20:35 UTC (permalink / raw) To: Jeff Cody, qemu-devel; +Cc: kwolf, jsnow, qemu-block [-- Attachment #1: Type: text/plain, Size: 1454 bytes --] On 02.11.2015 21:31, Max Reitz wrote: > On 30.10.2015 20:25, Jeff Cody wrote: >> Changes from v2: >> >> * Pulled patch 2 into this series >> >> * Patch 1: Moved non-empty test conditionals inside >> pid file existance check (thanks Max) >> >> Added a fix in for patch 058, for its >> self-launched qemu-nbd instance (thanks Max) >> >> * Patch 2: Only print the valgrind logs if the exit error >> matches (thanks Max) >> >> Changes from v1: >> >> * use 'read' instead of 'cat' (thanks Eric) >> * quote variable in variable test (thanks Eric) >> >> Jeff Cody (2): >> qemu-iotests: fix cleanup of background processes >> qemu-iotests: fix -valgrind option for check >> >> tests/qemu-iotests/039.out | 30 +++++++++++++++++++++++++----- >> tests/qemu-iotests/058 | 12 ++++++++---- >> tests/qemu-iotests/061.out | 12 ++++++++++-- >> tests/qemu-iotests/137.out | 6 +++++- >> tests/qemu-iotests/common | 9 ++------- >> tests/qemu-iotests/common.config | 32 +++++++++++++++++++++++++++++--- >> tests/qemu-iotests/common.qemu | 18 ++++++++++++------ >> tests/qemu-iotests/common.rc | 18 +++++------------- >> 8 files changed, 96 insertions(+), 41 deletions(-) > > Thanks, applied to my block tree with the three occurrences of ! -n in > patch 1 changed to -z: Err, of course it's the other way around, ! -z to -n. Max [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-11-02 20:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-30 19:25 [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes Jeff Cody 2015-10-30 19:25 ` [Qemu-devel] [PATCH v3 1/2] " Jeff Cody 2015-10-30 21:13 ` Max Reitz 2015-11-02 7:37 ` Markus Armbruster 2015-11-02 16:03 ` Max Reitz 2015-11-02 16:32 ` Eric Blake 2015-10-30 19:25 ` [Qemu-devel] [PATCH v3 2/2] qemu-iotests: fix -valgrind option for check Jeff Cody 2015-10-30 21:14 ` Max Reitz 2015-11-02 20:31 ` [Qemu-devel] [PATCH v3 0/2] qemu-iotests: fix cleanup of background processes Max Reitz 2015-11-02 20:35 ` Max Reitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).