[PATCH kvm-unit-tests v2 0/4] Have x86-run parse unittests.cfg

[PATCH kvm-unit-tests v2 0/4] Have x86-run parse unittests.cfg

[Cole Robinson]

First two patches are trivial bits. Rest rewrites x86-run in python,
which then makes it easy to parse unittests.cfg. This makes it
simpler to invoke individual unittests the same way autotest does.

Kevin has a similar series[1], but I'm reposting this for completeness.

[1] http://www.spinics.net/lists/kvm/msg89471.html

v2:
    Drop patch that removed x86/README
    Don't remove manual invocations examples from README

Cole Robinson (4):
  .gitignore: Add *.flat and config.mak
  x86/run-kvm-unit-tests: Drop it
  Rewrite x86-run in python
  x86-run: Pull extra arguments from unittests.cfg

 .gitignore             |   2 +
 README                 |  14 ++++--
 x86-run                | 133 +++++++++++++++++++++++++++++++++++++++----------
 x86/run-kvm-unit-tests |   6 ---
 4 files changed, 117 insertions(+), 38 deletions(-)
 delete mode 100644 x86/run-kvm-unit-tests

-- 
1.8.1.4

[PATCH kvm-unit-tests v2 1/4] .gitignore: Add *.flat and config.mak

[Cole Robinson]

---
 .gitignore | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index ed857b7..52655e1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,8 +1,10 @@
+config.mak
 .gdbinit
 *.a
 *.d
 *.o
 *.flat
+*.elf
 .pc
 patches
 .stgit-*
-- 
1.8.1.4

[PATCH kvm-unit-tests v2 2/4] x86/run-kvm-unit-tests: Drop it

[Cole Robinson]

Never appears to have done anything useful
---
 x86/run-kvm-unit-tests | 6 ------
 1 file changed, 6 deletions(-)
 delete mode 100644 x86/run-kvm-unit-tests

diff --git a/x86/run-kvm-unit-tests b/x86/run-kvm-unit-tests
deleted file mode 100644
index fed925a..0000000
--- a/x86/run-kvm-unit-tests
+++ /dev/null
@@ -1,6 +0,0 @@
-#!/usr/bin/python
-
-import sys, os, os.path
-
-prog = sys.argv[0]
-dir = os.path.dirname(prog)
-- 
1.8.1.4

[PATCH kvm-unit-tests v2 3/4] Rewrite x86-run in python

[Cole Robinson]

Main motivation here is in the next patch: getting access to ConfigParser
for pulling extra options out of x86/unittests.cfg. Also makes it
easier to get a consistent exit code on failure.
---

v2: Don't drop manual examples from README

 README  |  14 +++++---
 x86-run | 113 ++++++++++++++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 95 insertions(+), 32 deletions(-)

diff --git a/README b/README
index db525e3..274a224 100644
--- a/README
+++ b/README
@@ -19,11 +19,16 @@ Using qemu (supported since qemu 1.3):
 qemu-system-x86_64 -enable-kvm -device pc-testdev -serial stdio -device isa-debug-exit,iobase=0xf4,iosize=0x4 -kernel ./x86/msr.flat
 
 Or use a runner script to detect the correct invocation:
-./x86-run ./x86/msr.flat
-To select a specific qemu binary, specify the QEMU=<path> environment:
-QEMU=/tmp/qemu/x86_64-softmmu/qemu-system-x86_64 ./x86-run ./x86/msr.flat
 
-The exit status of the binary (and the script) is inconsistent: with
+  ./x86-run ./x86/msr.flat
+
+To select a specific qemu binary: specify the QEMU=<path> environment:
+
+  ./x86-run --qemu /path/to/x86_64-softmmu/qemu-system-x86_64 ./x86/msr.flat
+
+./x86-run returns 1 if any passed test fails, 0 otherwise.
+
+The exit status of the qemu* binary though is inconsistent: with
 qemu-system, after the unittest is done, the exit status of qemu is 1,
 different from the 'old style' qemu-kvm, whose exit status in successful
 completion is 0.
@@ -33,4 +38,3 @@ Directory structure:
 ./lib: general services for the tests
 ./lib/<ARCH>: architecture dependent services for the tests
 ./<ARCH>: the sources of the tests and the created objects/images
-
diff --git a/x86-run b/x86-run
index e395a70..ab9eed9 100755
--- a/x86-run
+++ b/x86-run
@@ -1,27 +1,86 @@
-#!/usr/bin/bash
-
-qemukvm="${QEMU:-qemu-kvm}"
-qemusystem="${QEMU:-qemu-system-x86_64}"
-if
-	${qemukvm} -device '?' 2>&1 | fgrep -e \"testdev\" -e \"pc-testdev\" > /dev/null;
-then
-	qemu="${qemukvm}"
-else
-	if
-		${qemsystem} -device '?' 2>&1 | fgrep -e \"testdev\" -e \"pc-testdev\" > /dev/null;
-	then
-		qemu="${qemusystem}"
-	else
-		echo QEMU binary ${QEMU} has no support for test device. Exiting.
-		exit 1
-	fi
-fi
-
-if
-	${qemu} -device '?' 2>&1 | fgrep "pc-testdev" > /dev/null;
-then
-	command="${qemu} -enable-kvm -device pc-testdev -serial stdio -device isa-debug-exit,iobase=0xf4,iosize=0x4 -kernel"
-else
-	command="${qemu} -device testdev,chardev=testlog -chardev file,id=testlog,path=msr.out -serial stdio -kernel"
-fi
-exec ${command} "$@"
+#!/usr/bin/env python
+
+import glob
+import optparse
+import os
+import sys
+
+
+def fail(msg):
+    print >> sys.stderr, "ERROR: %s" % msg
+    sys.exit(1)
+
+def supports_testdev(qemu):
+    return os.system("%s -device ? 2>&1 | fgrep -q "
+                     "-e 'testdev' -e 'pc-testdev'" % qemu) == 0
+
+def check_qemu(binpath):
+    if binpath:
+        if not supports_testdev(binpath):
+            fail("%s does not support testdev" % binpath)
+    elif supports_testdev("qemu-kvm"):
+        binpath = "qemu-kvm"
+    elif supports_testdev("qemu-system-x86_64"):
+        binpath = "qemu-system-x86_64"
+    else:
+        fail("Could not find qemu binary that supports testdev")
+
+    cmd = "%s -enable-kvm -serial stdio " % binpath
+
+    if os.system("%s -device ? 2>&1 | fgrep -q 'pc-testdev'" % binpath) == 0:
+        devcmd = ("-device pc-testdev "
+                  "-device isa-debug-exit,iobase=0xf4,iosize=0x4")
+    else:
+        devcmd = ("-device testdev,chardev=testlog "
+                  "-chardev file,id=testlog,path=test.out")
+
+    return cmd + devcmd
+
+
+def parse_args():
+    parser = optparse.OptionParser(
+        usage="%prog x86/$TEST1.flat [x86/$TEST2.flat] ...",
+        description="Launch a test with the correct qemu args")
+
+    parser.add_option("--qemu-bin", dest="qemu",
+                      help="Path to qemu binary to use. If not specified, "
+                           "uses qemu-kvm or qemu-system-x86_64 from $PATH.")
+    parser.add_option("--all", action="store_true", help="Run all x86 tests")
+
+    options, args = parser.parse_args()
+    if options.all:
+        args = glob.glob("x86/*.flat")
+        if not args:
+            parser.error("Didn't find any .flat files in x86/")
+
+    elif not args:
+        parser.error("A .flat test file or --all must be specified")
+
+    return options.qemu, args
+
+
+def main():
+    qemu, testfiles = parse_args()
+    basecmd = check_qemu(qemu)
+    newstyle = "pc-testdev" in basecmd
+
+    ret = 0
+    for testfile in testfiles:
+        cmd = basecmd + " -kernel %s" % testfile
+        print cmd
+
+        cmdret = os.system(cmd) >> 8
+        print
+
+        if cmdret == 0 and not newstyle:
+            continue
+        if cmdret == 1 and newstyle:
+            continue
+        ret = 1
+
+
+    return ret
+
+
+if __name__ == '__main__':
+    sys.exit(main())
-- 
1.8.1.4

[PATCH kvm-unit-tests v2 4/4] x86-run: Pull extra arguments from unittests.cfg

[Cole Robinson]

Some tests want extra arguments as enumerated in unittests.cfg,
use them.

unittests.cfg also has a few sections about invoking certains tests
with different combinations of options, but x86-run doesn't do
anything with that.
---
 x86-run | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/x86-run b/x86-run
index ab9eed9..7eebee2 100755
--- a/x86-run
+++ b/x86-run
@@ -1,5 +1,6 @@
 #!/usr/bin/env python
 
+import ConfigParser
 import glob
 import optparse
 import os
@@ -36,6 +37,24 @@ def check_qemu(binpath):
 
     return cmd + devcmd
 
+cfgfile = None
+def args_for_test(testfile):
+    global cfgfile
+    if not cfgfile:
+        cfgfile = ConfigParser.ConfigParser()
+        cfgfile.read("x86/unittests.cfg")
+
+    testname = os.path.splitext(os.path.basename(testfile))[0]
+    if testname not in cfgfile.sections():
+        return ""
+
+    cmd = ""
+    if cfgfile.has_option(testname, "extra_params"):
+        cmd += cfgfile.get(testname, "extra_params")
+    if cfgfile.has_option(testname, "smp"):
+        cmd += " -smp %s" % int(cfgfile.get(testname, "smp"))
+    return cmd
+
 
 def parse_args():
     parser = optparse.OptionParser(
@@ -67,6 +86,7 @@ def main():
     ret = 0
     for testfile in testfiles:
         cmd = basecmd + " -kernel %s" % testfile
+        cmd += " " + args_for_test(testfile)
         print cmd
 
         cmdret = os.system(cmd) >> 8
-- 
1.8.1.4

Re: [PATCH kvm-unit-tests v2 0/4] Have x86-run parse unittests.cfg

[Kevin Wolf]

Am 14.04.2013 um 20:18 hat Cole Robinson geschrieben:
> First two patches are trivial bits. Rest rewrites x86-run in python,
> which then makes it easy to parse unittests.cfg. This makes it
> simpler to invoke individual unittests the same way autotest does.
> 
> Kevin has a similar series[1], but I'm reposting this for completeness.
> 
> [1] http://www.spinics.net/lists/kvm/msg89471.html

As long as it doesn't take away functionality, it wouldn't even conflict
with my series. Unfortunately, it seems it does: The old x86-run
forwarded any additional options to the qemu binary, now there isn't any
way to specify options on the command line, but it always does its magic.

The other problem I mentioned in the other thread is that you assume
that the kernel file name and the configuration section are the same.
The test cases that exist in the configuration file with several
different argument strings are a strong hint that this assumption is
invalid.

Maybe it's really best to keep x86-run the thin wrapper that is today
and add a second script like my series does for a higher level tool.
(Though I must admit that this wasn't by design, my script was written
before x86-run existed and used to be standalone. I just called into
x86-run with this rebase to avoid duplicating things. But now it
actually seems to be the right choice, even if accidental.)

Also, Signed-off-by is missing throughout the series?

Kevin

Re: [PATCH kvm-unit-tests v2 0/4] Have x86-run parse unittests.cfg

[Paolo Bonzini]

Il 15/04/2013 10:30, Kevin Wolf ha scritto:
> Am 14.04.2013 um 20:18 hat Cole Robinson geschrieben:
>> > First two patches are trivial bits. Rest rewrites x86-run in python,
>> > which then makes it easy to parse unittests.cfg. This makes it
>> > simpler to invoke individual unittests the same way autotest does.
>> > 
>> > Kevin has a similar series[1], but I'm reposting this for completeness.
>> > 
>> > [1] http://www.spinics.net/lists/kvm/msg89471.html
> As long as it doesn't take away functionality, it wouldn't even conflict
> with my series. Unfortunately, it seems it does: The old x86-run
> forwarded any additional options to the qemu binary, now there isn't any
> way to specify options on the command line, but it always does its magic.
> 
> The other problem I mentioned in the other thread is that you assume
> that the kernel file name and the configuration section are the same.
> The test cases that exist in the configuration file with several
> different argument strings are a strong hint that this assumption is
> invalid.
> 
> Maybe it's really best to keep x86-run the thin wrapper that is today
> and add a second script like my series does for a higher level tool.
> (Though I must admit that this wasn't by design, my script was written
> before x86-run existed and used to be standalone. I just called into
> x86-run with this rebase to avoid duplicating things. But now it
> actually seems to be the right choice, even if accidental.)

I also prefer Kevin's series, though rewriting run_tests.sh in Python
would probably make it nicer.

Paolo

> Also, Signed-off-by is missing throughout the series?

Re: [PATCH kvm-unit-tests v2 0/4] Have x86-run parse unittests.cfg

[Cole Robinson]

On 04/15/2013 06:47 AM, Paolo Bonzini wrote:
> Il 15/04/2013 10:30, Kevin Wolf ha scritto:
>> Am 14.04.2013 um 20:18 hat Cole Robinson geschrieben:
>>>> First two patches are trivial bits. Rest rewrites x86-run in python,
>>>> which then makes it easy to parse unittests.cfg. This makes it
>>>> simpler to invoke individual unittests the same way autotest does.
>>>>
>>>> Kevin has a similar series[1], but I'm reposting this for completeness.
>>>>
>>>> [1] http://www.spinics.net/lists/kvm/msg89471.html
>> As long as it doesn't take away functionality, it wouldn't even conflict
>> with my series. Unfortunately, it seems it does: The old x86-run
>> forwarded any additional options to the qemu binary, now there isn't any
>> way to specify options on the command line, but it always does its magic.
>>
>> The other problem I mentioned in the other thread is that you assume
>> that the kernel file name and the configuration section are the same.
>> The test cases that exist in the configuration file with several
>> different argument strings are a strong hint that this assumption is
>> invalid.
>>
>> Maybe it's really best to keep x86-run the thin wrapper that is today
>> and add a second script like my series does for a higher level tool.
>> (Though I must admit that this wasn't by design, my script was written
>> before x86-run existed and used to be standalone. I just called into
>> x86-run with this rebase to avoid duplicating things. But now it
>> actually seems to be the right choice, even if accidental.)
> 
> I also prefer Kevin's series, though rewriting run_tests.sh in Python
> would probably make it nicer.
> 

Yeah the only missing 'feature' that my series has over Kevin's is re-writing
the script in python :)

I'm happy with Kevin's series as well.

- Cole