* [Qemu-devel] [PATCH] qemu-test: qemu-test script cleanups
@ 2012-01-13 22:05 Ryan Harper
2012-01-13 22:05 ` [Qemu-devel] [PATCH 1/4] update get_file_size to not throw error on non-existant files Ryan Harper
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Ryan Harper @ 2012-01-13 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Ryan Harper
Ran qemu-test for the first time and found a few places to clean up the output.
Did tab removal in qemu-test and then updated the indentation to be consistent.
Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 1/4] update get_file_size to not throw error on non-existant files
2012-01-13 22:05 [Qemu-devel] [PATCH] qemu-test: qemu-test script cleanups Ryan Harper
@ 2012-01-13 22:05 ` Ryan Harper
2012-01-13 22:05 ` [Qemu-devel] [PATCH 2/4] Add cleanup function Ryan Harper
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Ryan Harper @ 2012-01-13 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Ryan Harper
In some cases, when qemu is just starting up the logfile hasn't yet
been created and will throw an error message into the output:
ls: cannot access .tmp-3070/logfile-3070.log: No such file or directory
if we check to see if the file is readable first then we know the file
is present and we can extract the size.
Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
qemu-test | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/qemu-test b/qemu-test
index c6ea595..cd102a7 100755
--- a/qemu-test
+++ b/qemu-test
@@ -54,9 +54,10 @@ checkpid() {
}
get_file_size() {
- ls -al $1 | cut -f5 -d' ' 2>/dev/null
- if test $? != 0; then
- echo 0
+ if test -r "${1}" ; then
+ ls -al $1 | cut -f5 -d' '
+ else
+ echo 0
fi
}
--
1.7.6
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/4] Add cleanup function
2012-01-13 22:05 [Qemu-devel] [PATCH] qemu-test: qemu-test script cleanups Ryan Harper
2012-01-13 22:05 ` [Qemu-devel] [PATCH 1/4] update get_file_size to not throw error on non-existant files Ryan Harper
@ 2012-01-13 22:05 ` Ryan Harper
2012-01-13 23:17 ` Eric Blake
2012-01-13 22:05 ` [Qemu-devel] [PATCH 3/4] Remove tabs and replace with four spaces Ryan Harper
2012-01-13 22:05 ` [Qemu-devel] [PATCH 4/4] Apply consistent indentation Ryan Harper
3 siblings, 1 reply; 10+ messages in thread
From: Ryan Harper @ 2012-01-13 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Ryan Harper
Create a cleanup function and call it from all exits so we don't leave
temp files and directories around since we change the name on each invocation.
Also, no need to delete the files in the tmpdir, so just remove the tmpdir
if it exists.
Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
qemu-test | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/qemu-test b/qemu-test
index cd102a7..71c1ba1 100755
--- a/qemu-test
+++ b/qemu-test
@@ -1,7 +1,14 @@
#!/bin/sh
+cleanup() {
+ if test -n "$tmpdir"; then
+ rm -rf $tmpdir;
+ fi
+}
+
if test -z "$1" -o -z "$2"; then
echo "Usage: $0 QEMU TEST1 [TEST2 ...]"
+ cleanup
exit 1
fi
@@ -23,6 +30,7 @@ if ! which qmp >/dev/null 2>/dev/null; then
if ! test -x "${qmp}"; then
echo "Please set QEMU_SRC to set to a recent qemu.git tree"
+ cleanup
exit 1
fi
else
@@ -182,7 +190,6 @@ QEMU_TEST=1
. "$1"
rc=$?
-rm -f $tmplog $tmppid $tmpqmp $tmpinitrd
-rm -rf $tmpdir $tmprc
+cleanup
exit $rc
--
1.7.6
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 3/4] Remove tabs and replace with four spaces.
2012-01-13 22:05 [Qemu-devel] [PATCH] qemu-test: qemu-test script cleanups Ryan Harper
2012-01-13 22:05 ` [Qemu-devel] [PATCH 1/4] update get_file_size to not throw error on non-existant files Ryan Harper
2012-01-13 22:05 ` [Qemu-devel] [PATCH 2/4] Add cleanup function Ryan Harper
@ 2012-01-13 22:05 ` Ryan Harper
2012-01-13 23:10 ` Anthony Liguori
2012-01-13 22:05 ` [Qemu-devel] [PATCH 4/4] Apply consistent indentation Ryan Harper
3 siblings, 1 reply; 10+ messages in thread
From: Ryan Harper @ 2012-01-13 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Ryan Harper
Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
qemu-test | 62 ++++++++++++++++++++++++++++++------------------------------
1 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/qemu-test b/qemu-test
index 71c1ba1..445ca6d 100755
--- a/qemu-test
+++ b/qemu-test
@@ -29,9 +29,9 @@ if ! which qmp >/dev/null 2>/dev/null; then
qmp="${QEMU_SRC}/QMP/qmp"
if ! test -x "${qmp}"; then
- echo "Please set QEMU_SRC to set to a recent qemu.git tree"
+ echo "Please set QEMU_SRC to set to a recent qemu.git tree"
cleanup
- exit 1
+ exit 1
fi
else
qmp=`which qmp | head -1`
@@ -74,13 +74,13 @@ qmp() {
qmp_pid=$!
count=0
while checkpid $qmp_pid; do
- sleep 1
- count=$(($count + 1))
- if test $count -gt $QMP_TIMEOUT; then
- echo $count, $QMP_TIMEOUT
- kill -9 $qmp_pid
- return 1
- fi
+ sleep 1
+ count=$(($count + 1))
+ if test $count -gt $QMP_TIMEOUT; then
+ echo $count, $QMP_TIMEOUT
+ kill -9 $qmp_pid
+ return 1
+ fi
done
return 0
}
@@ -104,11 +104,11 @@ choose() {
target=$(($RANDOM % $#))
count=0
for i in "$@"; do
- if test $count = $target; then
- echo $i
- return 0
- fi
- count=$(($count + 1))
+ if test $count = $target; then
+ echo $i
+ return 0
+ fi
+ count=$(($count + 1))
done
# not supposed to happen...
@@ -117,7 +117,7 @@ choose() {
choose_bool() {
if test `choose yes no` = "yes"; then
- return 0
+ return 0
fi
return 1
}
@@ -135,34 +135,34 @@ start_qemu() {
qemu_is_okay() {
# it's stopped, that's not necessarly bad
if ! checkpid $pid; then
- return 1
+ return 1
fi
log_size=`get_file_size $tmplog`
if test $last_log_size = $log_size; then
- freeze_count=$(($freeze_count + 1))
+ freeze_count=$(($freeze_count + 1))
else
- freeze_count=0
- last_log_size=$log_size
+ freeze_count=0
+ last_log_size=$log_size
fi
if test $freeze_count -gt $FREEZE_THRESHOLD && checkpid $pid; then
- qemu_pid=`cat $tmppid`
- kill -9 $qemu_pid
- echo "Guest ($qemu_pid) has not had output in $FREEZE_THRESHOLD seconds!"
- return 2
+ qemu_pid=`cat $tmppid`
+ kill -9 $qemu_pid
+ echo "Guest ($qemu_pid) has not had output in $FREEZE_THRESHOLD seconds!"
+ return 2
fi
if ! qmp query-status >/dev/null 2>/dev/null && checkpid $pid; then
- qemu_pid=`cat $tmppid`
-
- kill -9 $qemu_pid
- echo "QEMU is hung!"
- return 3
+ qemu_pid=`cat $tmppid`
+
+ kill -9 $qemu_pid
+ echo "QEMU is hung!"
+ return 3
fi
# it's stopped, that's not necessarly bad
if ! checkpid $pid; then
- return 1
+ return 1
fi
}
@@ -170,7 +170,7 @@ get_qemu_status() {
wait $pid
rc=`cat $tmprc`
if test $(($rc & 1)) = 1; then
- rc=$(($rc / 2))
+ rc=$(($rc / 2))
fi
return $rc
}
@@ -179,7 +179,7 @@ qemu() {
start_qemu "$@"
while qemu_is_okay; do
- sleep 1
+ sleep 1
done
get_qemu_status
--
1.7.6
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 4/4] Apply consistent indentation
2012-01-13 22:05 [Qemu-devel] [PATCH] qemu-test: qemu-test script cleanups Ryan Harper
` (2 preceding siblings ...)
2012-01-13 22:05 ` [Qemu-devel] [PATCH 3/4] Remove tabs and replace with four spaces Ryan Harper
@ 2012-01-13 22:05 ` Ryan Harper
3 siblings, 0 replies; 10+ messages in thread
From: Ryan Harper @ 2012-01-13 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Ryan Harper
Inner blocks of if/for/while are all indented.
Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
qemu-test | 56 ++++++++++++++++++++++++++++----------------------------
1 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/qemu-test b/qemu-test
index 445ca6d..9750a3f 100755
--- a/qemu-test
+++ b/qemu-test
@@ -2,7 +2,7 @@
cleanup() {
if test -n "$tmpdir"; then
- rm -rf $tmpdir;
+ rm -rf $tmpdir;
fi
}
@@ -29,9 +29,9 @@ if ! which qmp >/dev/null 2>/dev/null; then
qmp="${QEMU_SRC}/QMP/qmp"
if ! test -x "${qmp}"; then
- echo "Please set QEMU_SRC to set to a recent qemu.git tree"
- cleanup
- exit 1
+ echo "Please set QEMU_SRC to set to a recent qemu.git tree"
+ cleanup
+ exit 1
fi
else
qmp=`which qmp | head -1`
@@ -63,9 +63,9 @@ checkpid() {
get_file_size() {
if test -r "${1}" ; then
- ls -al $1 | cut -f5 -d' '
+ ls -al $1 | cut -f5 -d' '
else
- echo 0
+ echo 0
fi
}
@@ -104,11 +104,11 @@ choose() {
target=$(($RANDOM % $#))
count=0
for i in "$@"; do
- if test $count = $target; then
- echo $i
- return 0
- fi
- count=$(($count + 1))
+ if test $count = $target; then
+ echo $i
+ return 0
+ fi
+ count=$(($count + 1))
done
# not supposed to happen...
@@ -117,7 +117,7 @@ choose() {
choose_bool() {
if test `choose yes no` = "yes"; then
- return 0
+ return 0
fi
return 1
}
@@ -135,34 +135,34 @@ start_qemu() {
qemu_is_okay() {
# it's stopped, that's not necessarly bad
if ! checkpid $pid; then
- return 1
+ return 1
fi
log_size=`get_file_size $tmplog`
if test $last_log_size = $log_size; then
- freeze_count=$(($freeze_count + 1))
+ freeze_count=$(($freeze_count + 1))
else
- freeze_count=0
- last_log_size=$log_size
+ freeze_count=0
+ last_log_size=$log_size
fi
if test $freeze_count -gt $FREEZE_THRESHOLD && checkpid $pid; then
- qemu_pid=`cat $tmppid`
- kill -9 $qemu_pid
- echo "Guest ($qemu_pid) has not had output in $FREEZE_THRESHOLD seconds!"
- return 2
+ qemu_pid=`cat $tmppid`
+ kill -9 $qemu_pid
+ echo "Guest ($qemu_pid) has not had output in $FREEZE_THRESHOLD seconds!"
+ return 2
fi
if ! qmp query-status >/dev/null 2>/dev/null && checkpid $pid; then
- qemu_pid=`cat $tmppid`
-
- kill -9 $qemu_pid
- echo "QEMU is hung!"
- return 3
+ qemu_pid=`cat $tmppid`
+
+ kill -9 $qemu_pid
+ echo "QEMU is hung!"
+ return 3
fi
# it's stopped, that's not necessarly bad
if ! checkpid $pid; then
- return 1
+ return 1
fi
}
@@ -170,7 +170,7 @@ get_qemu_status() {
wait $pid
rc=`cat $tmprc`
if test $(($rc & 1)) = 1; then
- rc=$(($rc / 2))
+ rc=$(($rc / 2))
fi
return $rc
}
@@ -179,7 +179,7 @@ qemu() {
start_qemu "$@"
while qemu_is_okay; do
- sleep 1
+ sleep 1
done
get_qemu_status
--
1.7.6
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Remove tabs and replace with four spaces.
2012-01-13 22:05 ` [Qemu-devel] [PATCH 3/4] Remove tabs and replace with four spaces Ryan Harper
@ 2012-01-13 23:10 ` Anthony Liguori
0 siblings, 0 replies; 10+ messages in thread
From: Anthony Liguori @ 2012-01-13 23:10 UTC (permalink / raw)
To: Ryan Harper; +Cc: qemu-devel
On 01/13/2012 04:05 PM, Ryan Harper wrote:
> Signed-off-by: Ryan Harper<ryanh@us.ibm.com>
You should replace them with 8 spaces and then you wouldn't need the next patch.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Add cleanup function
2012-01-13 22:05 ` [Qemu-devel] [PATCH 2/4] Add cleanup function Ryan Harper
@ 2012-01-13 23:17 ` Eric Blake
2012-01-16 17:16 ` Ryan Harper
0 siblings, 1 reply; 10+ messages in thread
From: Eric Blake @ 2012-01-13 23:17 UTC (permalink / raw)
To: Ryan Harper; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]
On 01/13/2012 03:05 PM, Ryan Harper wrote:
> Create a cleanup function and call it from all exits so we don't leave
> temp files and directories around since we change the name on each invocation.
>
> Also, no need to delete the files in the tmpdir, so just remove the tmpdir
> if it exists.
>
> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> ---
> qemu-test | 11 +++++++++--
> 1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-test b/qemu-test
> index cd102a7..71c1ba1 100755
> --- a/qemu-test
> +++ b/qemu-test
> @@ -1,7 +1,14 @@
> #!/bin/sh
>
> +cleanup() {
> + if test -n "$tmpdir"; then
> + rm -rf $tmpdir;
> + fi
> +}
> +
> if test -z "$1" -o -z "$2"; then
> echo "Usage: $0 QEMU TEST1 [TEST2 ...]"
> + cleanup
> exit 1
Is it worth using 'trap cleanup 0' to install the cleanup handler up
front, instead of modifying all exit call sites?
--
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: 620 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Add cleanup function
2012-01-13 23:17 ` Eric Blake
@ 2012-01-16 17:16 ` Ryan Harper
2012-01-17 22:03 ` Eric Blake
0 siblings, 1 reply; 10+ messages in thread
From: Ryan Harper @ 2012-01-16 17:16 UTC (permalink / raw)
To: Eric Blake; +Cc: Ryan Harper, qemu-devel
* Eric Blake <eblake@redhat.com> [2012-01-13 17:18]:
> On 01/13/2012 03:05 PM, Ryan Harper wrote:
> > Create a cleanup function and call it from all exits so we don't leave
> > temp files and directories around since we change the name on each invocation.
> >
> > Also, no need to delete the files in the tmpdir, so just remove the tmpdir
> > if it exists.
> >
> > Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> > ---
> > qemu-test | 11 +++++++++--
> > 1 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/qemu-test b/qemu-test
> > index cd102a7..71c1ba1 100755
> > --- a/qemu-test
> > +++ b/qemu-test
> > @@ -1,7 +1,14 @@
> > #!/bin/sh
> >
> > +cleanup() {
> > + if test -n "$tmpdir"; then
> > + rm -rf $tmpdir;
> > + fi
> > +}
> > +
> > if test -z "$1" -o -z "$2"; then
> > echo "Usage: $0 QEMU TEST1 [TEST2 ...]"
> > + cleanup
> > exit 1
>
> Is it worth using 'trap cleanup 0' to install the cleanup handler up
> front, instead of modifying all exit call sites?
I thought about that, but it seemed to require switching to /bin/bash
and I know Anthony had written the scripts carefully to be /bin/sh.
>
> --
> Eric Blake eblake@redhat.com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Add cleanup function
2012-01-16 17:16 ` Ryan Harper
@ 2012-01-17 22:03 ` Eric Blake
2012-01-20 15:33 ` Ryan Harper
0 siblings, 1 reply; 10+ messages in thread
From: Eric Blake @ 2012-01-17 22:03 UTC (permalink / raw)
To: Ryan Harper; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]
On 01/16/2012 10:16 AM, Ryan Harper wrote:
>>> if test -z "$1" -o -z "$2"; then
>>> echo "Usage: $0 QEMU TEST1 [TEST2 ...]"
>>> + cleanup
>>> exit 1
>>
>> Is it worth using 'trap cleanup 0' to install the cleanup handler up
>> front, instead of modifying all exit call sites?
>
> I thought about that, but it seemed to require switching to /bin/bash
Not really.
>
> and I know Anthony had written the scripts carefully to be /bin/sh.
POSIX requires /bin/sh to support 'trap cleanup 0', and I don't know of
any counter-example shells that fail to do this. There are non-POSIX
shells where installing a trap 0 handler from inside a function body
invokes the handler upon exiting the function, instead of exiting the
overall script, but even Solaris /bin/sh knows how to correctly handle a
trap 0 handler installed outside of any function calls.
https://www.gnu.org/software/autoconf/manual/autoconf.html#trap
--
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: 620 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Add cleanup function
2012-01-17 22:03 ` Eric Blake
@ 2012-01-20 15:33 ` Ryan Harper
0 siblings, 0 replies; 10+ messages in thread
From: Ryan Harper @ 2012-01-20 15:33 UTC (permalink / raw)
To: Eric Blake; +Cc: Ryan Harper, qemu-devel
* Eric Blake <eblake@redhat.com> [2012-01-17 16:03]:
> On 01/16/2012 10:16 AM, Ryan Harper wrote:
> >>> if test -z "$1" -o -z "$2"; then
> >>> echo "Usage: $0 QEMU TEST1 [TEST2 ...]"
> >>> + cleanup
> >>> exit 1
> >>
> >> Is it worth using 'trap cleanup 0' to install the cleanup handler up
> >> front, instead of modifying all exit call sites?
> >
> > I thought about that, but it seemed to require switching to /bin/bash
>
> Not really.
>
> >
> > and I know Anthony had written the scripts carefully to be /bin/sh.
>
> POSIX requires /bin/sh to support 'trap cleanup 0', and I don't know of
I was using trap cleanup SIGINT; which /bin/sh didn't like:
(finalgravity) qemu-test % ./qemu-test ~/work/git/qemu/x86_64-softmmu/qemu-system-x86_64 tests/virtio-serial.sh
trap: SIGINT: bad trap
but with 0 instead, that seems to work.
> any counter-example shells that fail to do this. There are non-POSIX
> shells where installing a trap 0 handler from inside a function body
> invokes the handler upon exiting the function, instead of exiting the
> overall script, but even Solaris /bin/sh knows how to correctly handle a
> trap 0 handler installed outside of any function calls.
>
> https://www.gnu.org/software/autoconf/manual/autoconf.html#trap
>
> --
> Eric Blake eblake@redhat.com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-01-20 15:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-13 22:05 [Qemu-devel] [PATCH] qemu-test: qemu-test script cleanups Ryan Harper
2012-01-13 22:05 ` [Qemu-devel] [PATCH 1/4] update get_file_size to not throw error on non-existant files Ryan Harper
2012-01-13 22:05 ` [Qemu-devel] [PATCH 2/4] Add cleanup function Ryan Harper
2012-01-13 23:17 ` Eric Blake
2012-01-16 17:16 ` Ryan Harper
2012-01-17 22:03 ` Eric Blake
2012-01-20 15:33 ` Ryan Harper
2012-01-13 22:05 ` [Qemu-devel] [PATCH 3/4] Remove tabs and replace with four spaces Ryan Harper
2012-01-13 23:10 ` Anthony Liguori
2012-01-13 22:05 ` [Qemu-devel] [PATCH 4/4] Apply consistent indentation Ryan Harper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.