[Qemu-devel] [PATCH 0/2] runner: Control test duration

[Qemu-devel] [PATCH 0/2] runner: Control test duration

[Maria Kustova]

The first patch adds the '--duration SECONDS' argument. After the specified
duration the runner allows to end the current test and then exits.

The second patch adds forced termination of a program under test, if the test
execution takes more than 5 minutes. Terminated tests are marked as failed.

If a program under test hangs, then the specified test duration can be overrun
up to 5 minutes.

The patch series is based on https://github.com/stefanha/qemu/commits/block,
commit 07a45925fa88376f8583a333e74f7eeb0f455685

Maria Kustova (2):
  runner: Add an argument for test duration
  runner: Kill a program under test by time-out

 tests/image-fuzzer/runner.py | 52 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 43 insertions(+), 9 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/2] runner: Add an argument for test duration

[Maria Kustova]

Signed-off-by: Maria Kustova <maria.k@catit.be>
---
 tests/image-fuzzer/runner.py | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
index 3fa7fca..688d470 100755
--- a/tests/image-fuzzer/runner.py
+++ b/tests/image-fuzzer/runner.py
@@ -25,6 +25,7 @@ import subprocess
 import random
 import shutil
 from itertools import count
+import time
 import getopt
 import StringIO
 import resource
@@ -206,7 +207,7 @@ class TestEnv(object):
             elif item[0] == 'qemu-io':
                 current_cmd = list(self.qemu_io)
             else:
-                multilog("Warning: test command '%s' is not defined.\n" \
+                multilog("Warning: test command '%s' is not defined.\n"
                          % item[0], sys.stderr, self.log, self.parent_log)
                 continue
             # Replace all placeholders with their real values
@@ -269,6 +270,7 @@ if __name__ == '__main__':
 
         Optional arguments:
           -h, --help                    display this help and exit
+          -d, --duration=NUMBER         finish tests after NUMBER of seconds
           -c, --command=JSON            run tests for all commands specified in
                                         the JSON array
           -s, --seed=STRING             seed for a test image generation,
@@ -325,10 +327,15 @@ if __name__ == '__main__':
         finally:
             test.finish()
 
+    def is_continue(duration, start_time):
+        """Return True if a new test can be started and False otherwise."""
+        current_time = int(time.time())
+        return (duration is None) or (current_time - start_time < duration)
+
     try:
-        opts, args = getopt.gnu_getopt(sys.argv[1:], 'c:hs:kv',
+        opts, args = getopt.gnu_getopt(sys.argv[1:], 'c:hs:kvd:',
                                        ['command=', 'help', 'seed=', 'config=',
-                                        'keep_passed', 'verbose'])
+                                        'keep_passed', 'verbose', 'duration='])
     except getopt.error, e:
         print >>sys.stderr, \
             "Error: %s\n\nTry 'runner.py --help' for more information" % e
@@ -339,6 +346,8 @@ if __name__ == '__main__':
     log_all = False
     seed = None
     config = None
+    duration = None
+
     for opt, arg in opts:
         if opt in ('-h', '--help'):
             usage()
@@ -357,6 +366,8 @@ if __name__ == '__main__':
             log_all = True
         elif opt in ('-s', '--seed'):
             seed = arg
+        elif opt in ('-d', '--duration'):
+            duration = int(arg)
         elif opt == '--config':
             try:
                 config = json.loads(arg)
@@ -394,9 +405,11 @@ if __name__ == '__main__':
     resource.setrlimit(resource.RLIMIT_CORE, (-1, -1))
     # If a seed is specified, only one test will be executed.
     # Otherwise runner will terminate after a keyboard interruption
-    for test_id in count(1):
+    start_time = int(time.time())
+    test_id = count(1)
+    while is_continue(duration, start_time):
         try:
-            run_test(str(test_id), seed, work_dir, run_log, cleanup,
+            run_test(str(test_id.next()), seed, work_dir, run_log, cleanup,
                      log_all, command, config)
         except (KeyboardInterrupt, SystemExit):
             sys.exit(1)
-- 
1.9.3

[Qemu-devel] [PATCH 2/2] runner: Kill a program under test by time-out

[Maria Kustova]

Signed-off-by: Maria Kustova <maria.k@catit.be>
---
 tests/image-fuzzer/runner.py | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
index 688d470..4399529 100755
--- a/tests/image-fuzzer/runner.py
+++ b/tests/image-fuzzer/runner.py
@@ -65,14 +65,35 @@ def run_app(fd, q_args):
     """Start an application with specified arguments and return its exit code
     or kill signal depending on the result of execution.
     """
+
+    class Alarm(Exception):
+        """Exception for signal.alarm events."""
+        pass
+
+    def handler(*arg):
+        """Notify that an alarm event occurred."""
+        raise Alarm
+
+    signal.signal(signal.SIGALRM, handler)
+    signal.alarm(300)
+    term_signal = signal.SIGKILL
     devnull = open('/dev/null', 'r+')
     process = subprocess.Popen(q_args, stdin=devnull,
                                stdout=subprocess.PIPE,
                                stderr=subprocess.PIPE)
-    out, err = process.communicate()
-    fd.write(out)
-    fd.write(err)
-    return process.returncode
+    try:
+        out, err = process.communicate()
+        signal.alarm(0)
+        fd.write(out)
+        fd.write(err)
+        fd.flush()
+        return process.returncode
+
+    except Alarm:
+        os.kill(process.pid, term_signal)
+        fd.write('The command was terminated by timeout.\n')
+        fd.flush()
+        return -term_signal
 
 
 class TestException(Exception):
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/2] runner: Add an argument for test duration

[Fam Zheng]

On Fri, 08/15 17:55, Maria Kustova wrote:
> Signed-off-by: Maria Kustova <maria.k@catit.be>
> ---
>  tests/image-fuzzer/runner.py | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
> index 3fa7fca..688d470 100755
> --- a/tests/image-fuzzer/runner.py
> +++ b/tests/image-fuzzer/runner.py
> @@ -25,6 +25,7 @@ import subprocess
>  import random
>  import shutil
>  from itertools import count
> +import time
>  import getopt
>  import StringIO
>  import resource
> @@ -206,7 +207,7 @@ class TestEnv(object):
>              elif item[0] == 'qemu-io':
>                  current_cmd = list(self.qemu_io)
>              else:
> -                multilog("Warning: test command '%s' is not defined.\n" \
> +                multilog("Warning: test command '%s' is not defined.\n"

The change seems irrelevant.

>                           % item[0], sys.stderr, self.log, self.parent_log)
>                  continue
>              # Replace all placeholders with their real values
> @@ -269,6 +270,7 @@ if __name__ == '__main__':
>  
>          Optional arguments:
>            -h, --help                    display this help and exit
> +          -d, --duration=NUMBER         finish tests after NUMBER of seconds
>            -c, --command=JSON            run tests for all commands specified in
>                                          the JSON array
>            -s, --seed=STRING             seed for a test image generation,
> @@ -325,10 +327,15 @@ if __name__ == '__main__':
>          finally:
>              test.finish()
>  
> +    def is_continue(duration, start_time):

Maybe "should_continue" ?

> +        """Return True if a new test can be started and False otherwise."""
> +        current_time = int(time.time())
> +        return (duration is None) or (current_time - start_time < duration)
> +
>      try:
> -        opts, args = getopt.gnu_getopt(sys.argv[1:], 'c:hs:kv',
> +        opts, args = getopt.gnu_getopt(sys.argv[1:], 'c:hs:kvd:',
>                                         ['command=', 'help', 'seed=', 'config=',
> -                                        'keep_passed', 'verbose'])
> +                                        'keep_passed', 'verbose', 'duration='])
>      except getopt.error, e:
>          print >>sys.stderr, \
>              "Error: %s\n\nTry 'runner.py --help' for more information" % e
> @@ -339,6 +346,8 @@ if __name__ == '__main__':
>      log_all = False
>      seed = None
>      config = None
> +    duration = None
> +
>      for opt, arg in opts:
>          if opt in ('-h', '--help'):
>              usage()
> @@ -357,6 +366,8 @@ if __name__ == '__main__':
>              log_all = True
>          elif opt in ('-s', '--seed'):
>              seed = arg
> +        elif opt in ('-d', '--duration'):
> +            duration = int(arg)
>          elif opt == '--config':
>              try:
>                  config = json.loads(arg)
> @@ -394,9 +405,11 @@ if __name__ == '__main__':
>      resource.setrlimit(resource.RLIMIT_CORE, (-1, -1))
>      # If a seed is specified, only one test will be executed.
>      # Otherwise runner will terminate after a keyboard interruption
> -    for test_id in count(1):
> +    start_time = int(time.time())
> +    test_id = count(1)
> +    while is_continue(duration, start_time):
>          try:
> -            run_test(str(test_id), seed, work_dir, run_log, cleanup,
> +            run_test(str(test_id.next()), seed, work_dir, run_log, cleanup,
>                       log_all, command, config)
>          except (KeyboardInterrupt, SystemExit):
>              sys.exit(1)
> -- 
> 1.9.3
> 

Modulus the above two minor issues,

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH 2/2] runner: Kill a program under test by time-out

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]

On Fri, Aug 15, 2014 at 05:55:49PM +0400, Maria Kustova wrote:
> Signed-off-by: Maria Kustova <maria.k@catit.be>
> ---
>  tests/image-fuzzer/runner.py | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
> index 688d470..4399529 100755
> --- a/tests/image-fuzzer/runner.py
> +++ b/tests/image-fuzzer/runner.py
> @@ -65,14 +65,35 @@ def run_app(fd, q_args):
>      """Start an application with specified arguments and return its exit code
>      or kill signal depending on the result of execution.
>      """
> +
> +    class Alarm(Exception):
> +        """Exception for signal.alarm events."""
> +        pass
> +
> +    def handler(*arg):
> +        """Notify that an alarm event occurred."""
> +        raise Alarm
> +
> +    signal.signal(signal.SIGALRM, handler)
> +    signal.alarm(300)

What is the purpose of this patch?

The hardcoded 5-minute timeout suggests you're trying to catch runaway
processes.  So this has nothing to do with the new --duration option?

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 2/2] runner: Kill a program under test by time-out

[M.Kustova]

On Mon, Aug 18, 2014 at 5:02 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Fri, Aug 15, 2014 at 05:55:49PM +0400, Maria Kustova wrote:
>> Signed-off-by: Maria Kustova <maria.k@catit.be>
>> ---
>>  tests/image-fuzzer/runner.py | 29 +++++++++++++++++++++++++----
>>  1 file changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
>> index 688d470..4399529 100755
>> --- a/tests/image-fuzzer/runner.py
>> +++ b/tests/image-fuzzer/runner.py
>> @@ -65,14 +65,35 @@ def run_app(fd, q_args):
>>      """Start an application with specified arguments and return its exit code
>>      or kill signal depending on the result of execution.
>>      """
>> +
>> +    class Alarm(Exception):
>> +        """Exception for signal.alarm events."""
>> +        pass
>> +
>> +    def handler(*arg):
>> +        """Notify that an alarm event occurred."""
>> +        raise Alarm
>> +
>> +    signal.signal(signal.SIGALRM, handler)
>> +    signal.alarm(300)
>
> What is the purpose of this patch?
>
> The hardcoded 5-minute timeout suggests you're trying to catch runaway
> processes.  So this has nothing to do with the new --duration option?

Yes, this patch doesn't relate to the implementation of the '--duration' option.
It helps to mark hung tests as failed.

Maria