[Qemu-devel] [PATCH] qemu-test: qemu-test script cleanups

[Qemu-devel] [PATCH] qemu-test: qemu-test script cleanups

[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>

[Qemu-devel] [PATCH 1/4] update get_file_size to not throw error on non-existant files

[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

[Qemu-devel] [PATCH 2/4] Add cleanup function

[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

Re: [Qemu-devel] [PATCH 2/4] Add cleanup function

[Eric Blake]

[-- 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 --]

Re: [Qemu-devel] [PATCH 2/4] Add cleanup function

[Ryan Harper]

* 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

Re: [Qemu-devel] [PATCH 2/4] Add cleanup function

[Eric Blake]

[-- 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 --]

Re: [Qemu-devel] [PATCH 2/4] Add cleanup function

[Ryan Harper]

* 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

[Qemu-devel] [PATCH 3/4] Remove tabs and replace with four spaces.

[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

Re: [Qemu-devel] [PATCH 3/4] Remove tabs and replace with four spaces.

[Anthony Liguori]

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

[Qemu-devel] [PATCH 4/4] Apply consistent indentation

[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