[PATCH 1/5] KVM test: support kernel -append command line options

[PATCH 1/5] KVM test: support kernel -append command line options

[Lucas Meneghel Rodrigues]

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_vm.py |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 966fb3c..87b037f 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -287,6 +287,9 @@ class VM:
         def add_initrd(help, filename):
             return " -initrd '%s'" % filename
 
+        def add_kernel_cmdline(help, cmdline):
+            return " -append %s" % cmdline
+
         # End of command line option wrappers
 
         if name is None: name = self.name
@@ -394,6 +397,10 @@ class VM:
             kernel = kvm_utils.get_path(root_dir, kernel)
             qemu_cmd += add_kernel(help, kernel)
 
+        kernel_cmdline = params.get("kernel_cmdline")
+        if kernel_cmdline:
+            qemu_cmd += add_kernel_cmdline(help, kernel_cmdline)
+
         initrd = params.get("initrd")
         if initrd:
             initrd = kvm_utils.get_path(root_dir, initrd)
-- 
1.7.0.1

[PATCH 2/5] KVM test: add boolean 'testdev' VM parameter for RHEL-6 style unit tests

[Lucas Meneghel Rodrigues]

Should be set to "yes" to enable testdev.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_vm.py |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 87b037f..7b1fc05 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -290,6 +290,10 @@ class VM:
         def add_kernel_cmdline(help, cmdline):
             return " -append %s" % cmdline
 
+        def add_testdev(help, filename):
+            return (" -chardev file,id=testlog,path=%s"
+                    " -device testdev,chardev=testlog" % filename)
+
         # End of command line option wrappers
 
         if name is None: name = self.name
@@ -424,6 +428,9 @@ class VM:
         elif params.get("uuid"):
             qemu_cmd += add_uuid(help, params.get("uuid"))
 
+        if params.get("testdev") == "yes":
+            qemu_cmd += add_testdev(help, self.get_testlog_filename())
+
         # If the PCI assignment step went OK, add each one of the PCI assigned
         # devices to the qemu command line.
         if self.pci_assignable:
-- 
1.7.0.1

[PATCH 3/5] KVM test: add wrapper for RHEL-6 style unittests

[Lucas Meneghel Rodrigues]

From: Michael Goldish <mgoldish@redhat.com>

Based on Naphtali Sprei's patches.

Changes from v2:
- Determine dynamically what tests are available and
what are the options for them, based on the unittest
directory (that is supposed to be linked during the
build test)

Changes from v1:
- Determine success/failure by exit status instead of output
- Restructure loop so that vm.is_dead() is called less often
- Copy test log to debugdir/unittest.log
- Change parameters passed to wait_for()

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/tests/kvm/tests/unittest.py |  114 ++++++++++++++++++++++++++++++++++++
 1 files changed, 114 insertions(+), 0 deletions(-)
 create mode 100644 client/tests/kvm/tests/unittest.py

diff --git a/client/tests/kvm/tests/unittest.py b/client/tests/kvm/tests/unittest.py
new file mode 100644
index 0000000..84e778c
--- /dev/null
+++ b/client/tests/kvm/tests/unittest.py
@@ -0,0 +1,114 @@
+import logging, time, os, shutil, glob, ConfigParser
+from autotest_lib.client.common_lib import error
+import kvm_subprocess, kvm_test_utils, kvm_utils, kvm_preprocessing
+
+
+def run_unittest(test, params, env):
+    """
+    KVM RHEL-6 style unit test:
+    1) Resume a stopped VM
+    2) Wait for VM to terminate
+    3) If qemu exited with code = 0, the unittest passed. Otherwise, it failed
+    4) Collect all logs generated
+
+    @param test: kvm test object
+    @param params: Dictionary with the test parameters
+    @param env: Dictionary with test environment
+    """
+    unittest_dir = os.path.join(test.bindir, 'unittests')
+    if not os.path.isdir(unittest_dir):
+        raise error.TestError("No unittest dir %s available (did you run the "
+                              "build test first?)" % unittest_dir)
+    os.chdir(unittest_dir)
+    unittest_list = glob.glob('*.flat')
+    if not unittest_list:
+        raise error.TestError("No unittest files available (did you run the "
+                              "build test first?)")
+    logging.debug('Flat file list: %s', unittest_list)
+
+    unittest_cfg = os.path.join(unittest_dir, 'unittests.cfg')
+    parser = ConfigParser.ConfigParser()
+    parser.read(unittest_cfg)
+    test_list = parser.sections()
+
+    if not test_list:
+        raise error.TestError("No tests listed on config file %s" %
+                              unittest_cfg)
+    logging.debug('Unit test list: %s' % test_list)
+
+    if params.get('test_list', None):
+        test_list = eval(params.get('test_list'))
+        logging.info('Original test list overriden by user')
+        logging.info('User defined unit test list: %s' % test_list)
+
+    nfail = 0
+    tests_failed = []
+
+    timeout = int(params.get('unittest_timeout', 600))
+
+    for t in test_list:
+        file = None
+        if parser.has_option(t, 'file'):
+            file = parser.get(t, 'file')
+
+        if file is None:
+            raise error.TestError('Unittest config file %s has section %s but '
+                                  'no mandatory option file.' %
+                                  (unittest_cfg, t))
+
+        if file not in unittest_list:
+            raise error.TestError('Unittest file %s referenced in config file '
+                                  '%s but was not find under the unittest dir' %
+                                  (file, unittest_cfg))
+
+        smp = None
+        if parser.has_option(t, 'smp'):
+            smp = int(parser.get(t, 'smp'))
+
+        extra_params = None
+        if parser.has_option(t, 'extra_params'):
+            extra_params = parser.get(t, 'extra_params')
+
+        vm_name = params.get("main_vm")
+        testlog_path = os.path.join(test.debugdir, "%s.log" % t)
+
+        params['kernel'] = os.path.join(unittest_dir, file)
+        logging.info('Running %s', t)
+
+        if smp is not None:
+            params['smp'] = smp
+            logging.info('SMP: %s', smp)
+
+        if extra_params is not None:
+            params['extra_params'] = extra_params
+            logging.info('Extra params: %s', extra_params)
+
+        try:
+            try:
+                kvm_preprocessing.preprocess_vm(test, params, env, vm_name)
+                vm = kvm_utils.env_get_vm(env, vm_name)
+                vm.monitor.cmd("cont")
+                logging.info("Waiting for unittest %s to complete, timeout %s, "
+                             "output in %s", t, timeout,
+                             vm.get_testlog_filename())
+                if not kvm_utils.wait_for(vm.is_dead, timeout):
+                    raise error.TestFail("Timeout elapsed (%ss)" % timeout)
+                # Check qemu's exit status
+                status = vm.process.get_status()
+                if status != 0:
+                    nfail += 1
+                    tests_failed.append(t)
+                    logging.error("Unit test %s failed", t)
+            except Exception, e:
+                nfail += 1
+                logging.error('Exception happened during %s: %s', t, str(e))
+        finally:
+            try:
+                shutil.copy(vm.get_testlog_filename(), testlog_path)
+                logging.info("Unit test log collected and available under %s",
+                             testlog_path)
+            except NameError, IOError:
+                logging.error("Not possible to collect logs")
+
+    if nfail != 0:
+        raise error.TestFail("Unit tests failed: %s" % " ".join(tests_failed))
-- 
1.7.0.1

[PATCH 4/5] KVM test: add sample RHEL-6 style unittest config file

[Lucas Meneghel Rodrigues]

From: Michael Goldish <mgoldish@redhat.com>

Based on Naphtali Sprei's patches.

Changes from v1:
- Remove reference_output parameter

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/unittests.cfg.sample |   56 +++++++++++++++++++++++++++++++++
 1 files changed, 56 insertions(+), 0 deletions(-)
 create mode 100644 client/tests/kvm/unittests.cfg.sample

diff --git a/client/tests/kvm/unittests.cfg.sample b/client/tests/kvm/unittests.cfg.sample
new file mode 100644
index 0000000..dd5f9a0
--- /dev/null
+++ b/client/tests/kvm/unittests.cfg.sample
@@ -0,0 +1,56 @@
+# Copy this file to unittests.cfg and edit it.
+#
+# Define the objects we'll be using
+vms = vm1
+
+# Choose the main VM
+main_vm = vm1
+
+# Some preprocessor/postprocessor params
+start_vm = yes
+kill_vm = yes
+kill_vm_gracefully = no
+
+# Monitor
+monitors = humanmonitor1
+main_monitor = humanmonitor1
+monitor_type = human
+
+# Screendump specific stuff
+convert_ppm_files_to_png_on_error = yes
+#keep_ppm_files = yes
+#keep_ppm_files_on_error = yes
+take_regular_screendumps = yes
+keep_screendumps_on_error = yes
+screendump_delay = 5
+screendump_quality = 30
+screendump_temp_dir = /dev/shm
+
+# Some default VM params
+qemu_binary = qemu
+qemu_img_binary = qemu-img
+smp = 1
+mem = 512
+display = vnc
+
+# Default scheduler params
+used_cpus = 1
+used_mem = 512
+
+# NIC parameters
+run_tcpdump = no
+
+# Misc
+run_kvm_stat = yes
+
+# Tests
+variants:
+    - unittest:
+        type = unittest
+        vms = ''
+        unittest_timeout = 600
+        testdev = yes
+        extra_params += " -S"
+        # In case you want to execute only a subset of the tests defined on the
+        # unittests.cfg file on qemu-kvm, uncomment and edit test_list
+        #test_list = ['idt_test', 'hypercall', 'vmexit', 'realmode']
-- 
1.7.0.1

[PATCH 5/5] KVM test: Make it possible to run VMs without NICs

[Lucas Meneghel Rodrigues]

For unittesting, for example, is interesting that we
run the VM with the bare mininum number of parameters.
This fix allows that.

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/tests/kvm/kvm_vm.py |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 7b1fc05..3c01fa0 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -118,8 +118,9 @@ class VM:
         self.root_dir = root_dir
         self.address_cache = address_cache
         self.netdev_id = []
-        for nic in params.get("nics").split():
-            self.netdev_id.append(kvm_utils.generate_random_id())
+        if params.get("nics"):
+            for nic in params.get("nics").split():
+                self.netdev_id.append(kvm_utils.generate_random_id())
 
         # Find a unique identifier for this VM
         while True:
-- 
1.7.0.1

Re: [PATCH 5/5] KVM test: Make it possible to run VMs without NICs

[Michael Goldish]

On 06/25/2010 02:33 AM, Lucas Meneghel Rodrigues wrote:
> For unittesting, for example, is interesting that we
> run the VM with the bare mininum number of parameters.
> This fix allows that.
> 
> Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
> ---
>  client/tests/kvm/kvm_vm.py |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 7b1fc05..3c01fa0 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -118,8 +118,9 @@ class VM:
>          self.root_dir = root_dir
>          self.address_cache = address_cache
>          self.netdev_id = []
> -        for nic in params.get("nics").split():
> -            self.netdev_id.append(kvm_utils.generate_random_id())
> +        if params.get("nics"):
> +            for nic in params.get("nics").split():

That's exactly what kvm_utils.get_sub_dict_names() does.  It may be a
long name for something so simple but it's used everywhere in kvm-autotest.

> +                self.netdev_id.append(kvm_utils.generate_random_id())

I think the 3 lines above belong in VM.create(), not VM.__init__(),
because VM params are routinely changed in calls to VM.create().  If the
code stays in __init__() the changed params will not affect
self.netdev_id.  A good place for it would be near the code that handles
-redir.

>  
>          # Find a unique identifier for this VM
>          while True:

Re: [PATCH 3/5] KVM test: add wrapper for RHEL-6 style unittests

[Michael Goldish]

On 06/25/2010 02:33 AM, Lucas Meneghel Rodrigues wrote:
> From: Michael Goldish <mgoldish@redhat.com>
> 
> Based on Naphtali Sprei's patches.
> 
> Changes from v2:
> - Determine dynamically what tests are available and
> what are the options for them, based on the unittest
> directory (that is supposed to be linked during the
> build test)
> 
> Changes from v1:
> - Determine success/failure by exit status instead of output
> - Restructure loop so that vm.is_dead() is called less often
> - Copy test log to debugdir/unittest.log
> - Change parameters passed to wait_for()
> 
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
> ---
>  client/tests/kvm/tests/unittest.py |  114 ++++++++++++++++++++++++++++++++++++
>  1 files changed, 114 insertions(+), 0 deletions(-)
>  create mode 100644 client/tests/kvm/tests/unittest.py
> 
> diff --git a/client/tests/kvm/tests/unittest.py b/client/tests/kvm/tests/unittest.py
> new file mode 100644
> index 0000000..84e778c
> --- /dev/null
> +++ b/client/tests/kvm/tests/unittest.py
> @@ -0,0 +1,114 @@
> +import logging, time, os, shutil, glob, ConfigParser
> +from autotest_lib.client.common_lib import error
> +import kvm_subprocess, kvm_test_utils, kvm_utils, kvm_preprocessing
> +
> +
> +def run_unittest(test, params, env):
> +    """
> +    KVM RHEL-6 style unit test:
> +    1) Resume a stopped VM
> +    2) Wait for VM to terminate
> +    3) If qemu exited with code = 0, the unittest passed. Otherwise, it failed
> +    4) Collect all logs generated
> +
> +    @param test: kvm test object
> +    @param params: Dictionary with the test parameters
> +    @param env: Dictionary with test environment
> +    """
> +    unittest_dir = os.path.join(test.bindir, 'unittests')
> +    if not os.path.isdir(unittest_dir):
> +        raise error.TestError("No unittest dir %s available (did you run the "
> +                              "build test first?)" % unittest_dir)
> +    os.chdir(unittest_dir)

Why chdir()?

> +    unittest_list = glob.glob('*.flat')
> +    if not unittest_list:
> +        raise error.TestError("No unittest files available (did you run the "
> +                              "build test first?)")
> +    logging.debug('Flat file list: %s', unittest_list)
> +
> +    unittest_cfg = os.path.join(unittest_dir, 'unittests.cfg')
> +    parser = ConfigParser.ConfigParser()
> +    parser.read(unittest_cfg)
> +    test_list = parser.sections()
> +
> +    if not test_list:
> +        raise error.TestError("No tests listed on config file %s" %
> +                              unittest_cfg)
> +    logging.debug('Unit test list: %s' % test_list)
> +
> +    if params.get('test_list', None):
> +        test_list = eval(params.get('test_list'))

I wonder why you use eval() here.  It's somewhat inconsistent with how
we usually define lists in config files.

> +        logging.info('Original test list overriden by user')
> +        logging.info('User defined unit test list: %s' % test_list)
> +
> +    nfail = 0
> +    tests_failed = []
> +
> +    timeout = int(params.get('unittest_timeout', 600))
> +
> +    for t in test_list:
> +        file = None
> +        if parser.has_option(t, 'file'):
> +            file = parser.get(t, 'file')
> +
> +        if file is None:
> +            raise error.TestError('Unittest config file %s has section %s but '
> +                                  'no mandatory option file.' %
> +                                  (unittest_cfg, t))
> +
> +        if file not in unittest_list:
> +            raise error.TestError('Unittest file %s referenced in config file '
> +                                  '%s but was not find under the unittest dir' %
> +                                  (file, unittest_cfg))
> +
> +        smp = None
> +        if parser.has_option(t, 'smp'):
> +            smp = int(parser.get(t, 'smp'))
> +
> +        extra_params = None
> +        if parser.has_option(t, 'extra_params'):
> +            extra_params = parser.get(t, 'extra_params')
> +
> +        vm_name = params.get("main_vm")
> +        testlog_path = os.path.join(test.debugdir, "%s.log" % t)
> +
> +        params['kernel'] = os.path.join(unittest_dir, file)
> +        logging.info('Running %s', t)
> +
> +        if smp is not None:
> +            params['smp'] = smp
> +            logging.info('SMP: %s', smp)
> +
> +        if extra_params is not None:
> +            params['extra_params'] = extra_params
> +            logging.info('Extra params: %s', extra_params)

1. Why not just use

params.update(parser.items(t))

instead of hardcoding 'smp', 'extra_params' etc.  It seems more flexible
but maybe I'm missing something.

2. Why not keep only 'extra_params' and dump the other keys?  The cfg
file is qemu-kvm version controlled, so it makes sense to give kvm
developers direct control over the command line.  The parameter can be
named 'args' or 'cmdline' or something like that.  If we do that, we
also have to make sure that nothing unnecessary is added to the command
line by default in VM.create() (e.g. -smp should be added only if the
user sets 'smp' to some value).  The required stuff will be added as
usual (qemu binary, monitor, serial console, testdev).  What do you
think?  If any kvm developer is reading this, please let us know if you
prefer to control the command line directly or via kvm-autotest parameters.

> +        try:
> +            try:
> +                kvm_preprocessing.preprocess_vm(test, params, env, vm_name)
> +                vm = kvm_utils.env_get_vm(env, vm_name)

I'm not sure preprocess_vm() is better than just vm.create().
preprocess_vm() does unnecessary stuff in this context.
Also, this would be the 2nd time preprocess_vm() is called, so in fact
what you get is a VM that is started by the first preprocess_vm() and
then restarted by the 2nd preprocess_vm().
To avoid that we can set 'start_vm = no' in unittests.cfg.sample (the
kvm-autotest one).

> +                vm.monitor.cmd("cont")
> +                logging.info("Waiting for unittest %s to complete, timeout %s, "
> +                             "output in %s", t, timeout,
> +                             vm.get_testlog_filename())
> +                if not kvm_utils.wait_for(vm.is_dead, timeout):
> +                    raise error.TestFail("Timeout elapsed (%ss)" % timeout)
> +                # Check qemu's exit status
> +                status = vm.process.get_status()
> +                if status != 0:
> +                    nfail += 1
> +                    tests_failed.append(t)
> +                    logging.error("Unit test %s failed", t)
> +            except Exception, e:
> +                nfail += 1
> +                logging.error('Exception happened during %s: %s', t, str(e))
> +        finally:
> +            try:
> +                shutil.copy(vm.get_testlog_filename(), testlog_path)

Maybe we should use os.link() before starting the test.  That way if the
host dies during the test the result dir will already have a hard link
to the testlog.

> +                logging.info("Unit test log collected and available under %s",
> +                             testlog_path)
> +            except NameError, IOError:
> +                logging.error("Not possible to collect logs")
> +
> +    if nfail != 0:
> +        raise error.TestFail("Unit tests failed: %s" % " ".join(tests_failed))