Skip to contentSkip to conversation

don't run test 167 if killall is not installed

PATCH In review Christoph Hellwig 3 participants 7 messages May 15, 2008 – May 19, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 don't run test 167 if killall is not installed

Test 167 will leave runaway fsstress processes around in case killall is not installed and thus make all following tests fail. This patch checks for killall beeing installed and error out otherwise.

Index: xfstests/167
===================================================================
RCS file: /cvs/xfs-cmds/xfstests/167,v
retrieving revision 1.3

167 +3 -0
diff -u -p -r1.3 167
@@ -13,6 +13,7 @@ [email protected]
 seq=`basename $0`
 echo "QA output created by $seq"
 
+killall="/usr/bin/killall"
 here=`pwd`
 tmp=/tmp/$$
 rm -f $seq.full
@@ -43,6 +44,8 @@ workout()
 _supported_fs xfs
 _supported_os Linux
 
+[ -x $killall ] || _notrun "$killall executable not found"
+
 _setup_testdir
 _require_scratch
 _scratch_mkfs_xfs >/dev/null 2>&1
#1 lore raw
Quoted 5 lines · Christoph Hellwig

Test 167 will leave runaway fsstress processes around in case killall is not installed and thus make all following tests fail. This patch checks for killall beeing installed and error out otherwise.

Looks reasonable.

However:

* could use set_prog_path like we do in common.config.

* I wonder if one could use "kill -$pgid" on the process group for fsstress instead of killall (I've never tried it :).

--Tim

Quoted 27 lines

Index: xfstests/167
===================================================================
RCS file: /cvs/xfs-cmds/xfstests/167,v
retrieving revision 1.3
diff -u -p -r1.3 167

167 +3 -0
@@ -13,6 +13,7 @@ [email protected]
 seq=`basename $0`
 echo "QA output created by $seq"
 
+killall="/usr/bin/killall"
 here=`pwd`
 tmp=/tmp/$$
 rm -f $seq.full
@@ -43,6 +44,8 @@ workout()
 _supported_fs xfs
 _supported_os Linux
 
+[ -x $killall ] || _notrun "$killall executable not found"
+
 _setup_testdir
 _require_scratch
 _scratch_mkfs_xfs >/dev/null 2>&1
#2 lore raw
Quoted 8 lines · Tim Shimmin

Looks reasonable.

However:

* could use set_prog_path like we do in common.config.

* I wonder if one could use "kill -$pgid" on the process group for fsstress instead of killall (I've never tried it :).

Dave already commited the original version, but I'll send an update to use set_prog_path later today.

#3 lore raw
Quoted 5 lines · Christoph Hellwig
Quoted 4 lines · Tim Shimmin

Looks reasonable.

However:

Quoted 7 lines
Quoted 4 lines

* could use set_prog_path like we do in common.config.

* I wonder if one could use "kill -$pgid" on the process group for fsstress instead of killall (I've never tried it :).

Dave already commited the original version, but I'll send an update to use set_prog_path later today.

Sorry, didn't see that Tim replied as well. set_prog_path is fine by me too, but stopping every test from running because killall is not present is a bit of overkill, isn't it?

Cheers,

Dave.

Signature · 3 lines
Dave Chinner
Principal Engineer
SGI Australian Software Group
#4 lore raw
Quoted 6 lines · David Chinner
Quoted 2 lines

Dave already commited the original version, but I'll send an update to use set_prog_path later today.

Sorry, didn't see that Tim replied as well. set_prog_path is fine by me too, but stopping every test from running because killall is not present is a bit of overkill, isn't it?

My plan was to use it in 167 not common.config. But I don't really care either way.

#5 lore raw
Quoted 9 lines · Christoph Hellwig
Quoted 5 lines · David Chinner
Quoted 2 lines

Dave already commited the original version, but I'll send an update to use set_prog_path later today.

Sorry, didn't see that Tim replied as well. set_prog_path is fine by me too, but stopping every test from running because killall is not present is a bit of overkill, isn't it?

My plan was to use it in 167 not common.config. But I don't really care either way.

Yeah, I was just meaning for 167 too.

--Tim

#6 lore raw
In reply to Christoph Hellwig
Quoted 3 lines · Christoph Hellwig

Test 167 will leave runaway fsstress processes around in case killall is not installed and thus make all following tests fail. This patch checks for killall beeing installed and error out otherwise.

Looks sane to me. I'll check it in in a minute....

Cheers,

Dave.

Signature · 3 lines
Dave Chinner
Principal Engineer
SGI Australian Software Group