One thing that just occurred to me is, if we have a test config "library" as you said, it's perfectly possible to put the actual test set definitions and other config files inside the control file as strings. This way one can control configuration inside the control file, making it more convenient for usage, let's say, inside the autotest web interface. Since the control file would be reduced in size, the configuration snippets being in the control file would not be a huge problem, while keeping the original autotest philosophy of keeping stuff inside the control file... What do you think? On Wed, 2009-10-28 at 05:48 -0400, Michael Goldish wrote: > I think this patch is a good idea. Comments: > > - If we've decided to put all the configuration in config files > (rather than the control file) let's split up the main one into > kvm_test_library.cfg and kvm_tests.cfg.sample. The latter should > look like this: > > include kvm_test_library.cfg > include kvm_cdkeys.cfg > > variants: > - fc8_quick: > ... > - other_test_set: > ... > > only fc8_quick > > ... and the former should contain everything else. > > - I think we should allow the user to modify his/her configuration > in the control file using parse_string(). This is actually useful, > especially when running from the server. (At least I use it.) > > - If kvm_build.cfg or kvm_tests.cfg are missing, we should continue > without them, or create them (blank), instead of failing due to an > unhandled exception raised by the config parser. This should be > true at least for kvm_build.cfg, which isn't always required for the > tests to run. > > - We should consider removing hardcoded references to > /tmp/kvm_autotest_root/. We use that path locally in TLV but that's > not a good reason to put it upstream IMO. > We should also stop creating symlinks to that path. > Instead we can allow more flexibility using config files. > For example, we can put this in kvm_tests.cfg.sample, between the > 'include' and the 'variants': > > # Uncomment and modify the following lines if you wish to modify > # the paths of the image files, ISO files, step files or qemu binaries. > # If you keep these lines commented, all mentioned paths are expected > # to be found in the KVM test dir (tests/kvm/). For example, all > # image files are expected under tests/kvm/images/. > > # qemu_binary ?<= /tmp/kvm_autotest_root/ > # qemu_img_binary ?<= /tmp/kvm_autotest_root/ > # image_name.* ?<= /tmp/kvm_autotest_root/ > # cdrom.* ?<= /tmp/kvm_autotest_root/ > # steps ?<= /tmp/kvm_autotest_root/ > > The ?<= operator prepends strings to keys that match a regex, so > 'image_name.* ?<= something/' will prepend 'something/' to image_name, > image_name_vm2 and so on. > In kvm_test_library.cfg (currently kvm_tests.cfg.sample) all > 'image_name' parameters, for example, begin with 'images/', which means > all image files should be under tests/kvm/images. > If we prepend /tmp/kvm_autotest_root/ to that, all image files will be > expected under /tmp/kvm_autotest_root/images/. > > Maybe we should leave these lines uncommented by default, in case anyone > uses /tmp/kvm_autotest_root and relies on the control file creating the > symlinks. > > - kvm_utils.run_tests() should return True if all tests passed and > otherwise False. We need this in order to quit if the build test(s) > failed. > > - I suggest we use kvm_utils.parse_host_specific_cfg() instead of > kvm_utils.get_test_list(). I think 'parse_host_specific_cfg' better > describes what the function does. I also think it should take > 'kvm_address_pools.cfg' as a parameter so we don't have to hardcode it > into the function. > > > Please see additional comments below. > > > ----- "Lucas Meneghel Rodrigues" <lmr@xxxxxxxxxx> wrote: > > > As pointed out before, the KVM reference control files > > could use a little clean up. This patch implements severe > > cleanup of the main control file by: > > > > * Refactoring the code present there, moving it to the > > kvm_utils.py library > > * Treat the build test exactly the same way as other > > tests, moving the config stuff that used to be in the > > control file realm out to its own configuration file, > > for the sake of consistency. > > * Since a frequent complaint amongst users is that by > > default the build test will build from release, and > > that build might be unsuccessful depending on the system > > being used (older kernel versions and other sorts of > > stuff), change the default build method to 'noinstall'. > > > > This way the control file becomes way shorter, fairly > > well organized, and we have a consistent configuration > > schema across the board, based on configuration files. > > > > If people are OK with this change, final path will > > change the control.parallel file as well. > > > > Important: For the default control file, I removed > > the part of the code that parses new strings for the > > test configuration because I believe those > > modifications belong to the test config files, they > > could be just commented. For cases where keeping > > all stuff inside the config file is not possible, > > appropriate code will be added (in other example > > control files). > > > > Signed-off-by: Lucas Meneghel Rodrigues <lmr@xxxxxxxxxx> > > --- > > client/tests/kvm/control | 203 > > +++------------------------------ > > client/tests/kvm/kvm_build.cfg.sample | 82 +++++++++++++ > > client/tests/kvm/kvm_utils.py | 112 ++++++++++++++++++ > > 3 files changed, 211 insertions(+), 186 deletions(-) > > create mode 100644 client/tests/kvm/kvm_build.cfg.sample > > > > diff --git a/client/tests/kvm/control b/client/tests/kvm/control > > index 431baf9..7acec14 100644 > > --- a/client/tests/kvm/control > > +++ b/client/tests/kvm/control > > @@ -20,194 +20,25 @@ KVM (both kernelspace and userspace) code. > > For online docs, please refer to > > http://www.linux-kvm.org/page/KVM-Autotest > > """ > > > > +import sys, os, logging > > +# Add the KVM tests dir to the python path > > +kvm_test_dir = os.path.join(os.environ['AUTODIR'],'tests/kvm') > > +sys.path.append(kvm_test_dir) > > +import kvm_utils, kvm_config > > > > -import sys, os > > > > -#----------------------------------------------------------------------------- > > -# set English environment (command output might be localized, need to > > be safe) > > -#----------------------------------------------------------------------------- > > -os.environ['LANG'] = 'en_US.UTF-8' > > +## Prepare the environment for the KVM test. > > +kvm_utils.prepare_test_run(basedir=kvm_test_dir, > > + rootdir='/tmp/kvm_autotest_root') > > > > -#--------------------------------------------------------- > > -# Enable modules import from current directory (tests/kvm) > > -#--------------------------------------------------------- > > -pwd = os.path.join(os.environ['AUTODIR'],'tests/kvm') > > -sys.path.append(pwd) > > +cfg_names = ['kvm_build.cfg', 'kvm_tests.cfg'] > > > > -# ------------------------ > > -# create required symlinks > > -# ------------------------ > > -# When dispatching tests from autotest-server the links we need do > > not exist on > > -# the host (the client). The following lines create those symlinks. > > Change > > -# 'rootdir' here and/or mount appropriate directories in it. > > -# > > -# When dispatching tests on local host (client mode) one can either > > setup kvm > > -# links, or same as server mode use rootdir and set all appropriate > > links and > > -# mount-points there. For example, guest installation tests need to > > know where > > -# to find the iso-files. > > -# > > -# We create the links only if not already exist, so if one already > > set up the > > -# links for client/local run we do not touch the links. > > -rootdir='/tmp/kvm_autotest_root' > > -iso=os.path.join(rootdir, 'iso') > > -images=os.path.join(rootdir, 'images') > > -qemu=os.path.join(rootdir, 'qemu') > > -qemu_img=os.path.join(rootdir, 'qemu-img') > > +for cfg_name in cfg_names: > > + cfg_path = os.path.join(kvm_test_dir, cfg_name) > > + cfg = kvm_config.config(cfg_path) > > + list = kvm_utils.get_test_list(cfg, cfg_name, kvm_test_dir) > > + logging.info("Running test set defined on config file %s", > > cfg_name) > > + kvm_utils.run_tests(list, job) > > I think we should unroll this loop and allow users to modify the main > and/or build configurations here. For example: > > build_cfg_path = os.path.join(kvm_test_dir, "kvm_build.cfg") > test_cfg_path = os.path.join(kvm_test_dir, "kvm_tests.cfg") > open(build_cfg_path, 'a').close() > open(test_cfg_path, 'a').close() > > cfg = kvm_config.config(build_cfg_path) > cfg.parse_string(""" > # Make any desired changes to the build configuration here. For example: > # release_tag = 84 > """) > if not kvm_utils.run_tests(cfg.get_list(), job.run_test): > sys.exit(1) > > cfg = kvm_config.config(test_cfg_path) > cfg.parse_string(""" > # Make any desired changes to the test configuration here. For example: > # display = sdl > # install|setup: timeout_multiplier = 3 > """) > kvm_utils.parse_host_specific_cfg(cfg, os.path.join(kvm_test_dir, > "kvm_address_pools.cfg")) > kvm_utils.run_tests(cfg.get_list(), job.run_test) > > Alternatively, instead of creating the files in case they don't exit, > we can put the code that parses and runs tests inside 'if' statements. > I'm not sure which option is shorter. > > > > > - > > -def link_if_not_exist(ldir, target, link_name): > > - t = target > > - l = os.path.join(ldir, link_name) > > - if not os.path.exists(l): > > - os.system('ln -s %s %s' % (t, l)) > > - > > -# Create links only if not already exist > > -link_if_not_exist(pwd, '../../', 'autotest') > > -link_if_not_exist(pwd, iso, 'isos') > > -link_if_not_exist(pwd, images, 'images') > > -link_if_not_exist(pwd, qemu, 'qemu') > > -link_if_not_exist(pwd, qemu_img, 'qemu-img') > > - > > -# -------------------------------------------------------- > > -# Params that will be passed to the KVM install/build test > > -# -------------------------------------------------------- > > -params = { > > - "name": "build", > > - "shortname": "build", > > - "type": "build", > > - "mode": "release", > > - #"mode": "snapshot", > > - #"mode": "localtar", > > - #"mode": "localsrc", > > - #"mode": "git", > > - #"mode": "noinstall", > > - #"mode": "koji", > > - > > - ## Are we going to load modules built by this test? > > - ## Defaults to 'yes', so if you are going to provide only > > userspace code to > > - ## be built by this test, please set load_modules to 'no', and > > make sure > > - ## the kvm and kvm-[vendor] module is already loaded by the time > > you start > > - ## it. > > - "load_modules": "no", > > - > > - ## Install from a kvm release ("mode": "release"). You can > > optionally > > - ## specify a release tag. If you omit it, the test will get the > > latest > > - ## release tag available. > > - #"release_tag": '84', > > - "release_dir": 'http://downloads.sourceforge.net/project/kvm/', > > - # This is the place that contains the sourceforge project list of > > files > > - "release_listing": 'http://sourceforge.net/projects/kvm/files/', > > - > > - ## Install from a kvm snapshot location ("mode": "snapshot"). You > > can > > - ## optionally specify a snapshot date. If you omit it, the test > > will get > > - ## yesterday's snapshot. > > - #"snapshot_date": '20090712' > > - #"snapshot_dir": 'http://foo.org/kvm-snapshots/', > > - > > - ## Install from a tarball ("mode": "localtar") > > - #"tarball": "/tmp/kvm-84.tar.gz", > > - > > - ## Install from a local source code dir ("mode": "localsrc") > > - #"srcdir": "/path/to/source-dir" > > - > > - ## Install from koji build server ("mode": "koji") > > - ## Koji is the Fedora Project buildserver. It is possible to > > install > > - ## packages right from Koji if you provide a release tag or a > > build. > > - ## Tag (if available) > > - #"koji_tag": 'dist-f11', > > - ## Build (if available, is going to override tag). > > - #"koji_build": 'qemu-0.10-16.fc11', > > - ## Command to interact with the build server > > - #"koji_cmd": '/usr/bin/koji', > > - ## The name of the source package that's being built > > - #"src_pkg": 'qemu', > > - ## Name of the rpms we need installed > > - #"pkg_list": ['qemu-kvm', 'qemu-kvm-tools', 'qemu-system-x86', > > 'qemu-common', 'qemu-img'], > > - ## Paths of the qemu relevant executables that should be checked > > - #"qemu_bin_paths": ['/usr/bin/qemu-kvm', '/usr/bin/qemu-img'], > > - > > - ## Install from git ("mode": "git") > > - ## If you provide only "git_repo" and "user_git_repo", the build > > test > > - ## will assume it will perform all build from the userspace dir, > > building > > - ## modules trough make -C kernel LINUX=%s sync. As of today > > (07-13-2009) > > - ## we need 3 git repos, "git_repo" (linux sources), > > "user_git_repo" and > > - ## "kmod_repo" to build KVM userspace + kernel modules. > > - #"git_repo": > > 'git://git.kernel.org/pub/scm/linux/kernel/git/avi/kvm.git', > > - #"kernel_branch": 'kernel_branch_name', > > - #"kernel_lbranch": 'kernel_lbranch_name', > > - #"kernel_tag": 'kernel_tag_name', > > - #"user_git_repo": > > 'git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git', > > - #"user_branch": 'user_branch_name', > > - #"user_lbranch": 'user_lbranch_name', > > - #"user_tag": 'user_tag_name', > > - #"kmod_repo": > > 'git://git.kernel.org/pub/scm/virt/kvm/kvm-kmod.git', > > - #"kmod_branch": 'kmod_branch_name', > > - #"kmod_lbranch": 'kmod_lbranch_name', > > - #"kmod_tag": 'kmod_tag_name', > > -} > > - > > -# If you don't want to execute the build stage, just use 'noinstall' > > as the > > -# install type. If you run the tests from autotest-server, make sure > > that > > -# /tmp/kvm-autotest-root/qemu is a link to your existing executable. > > Note that > > -# if kvm_install is chose to run, it overwrites existing qemu and > > qemu-img > > -# links to point to the newly built executables. > > -r = True > > -r = job.run_test("kvm", params=params, tag=params.get("shortname")) > > -if not r: > > - print 'kvm_installation failed ... exiting' > > - sys.exit(1) > > - > > -# ---------------------------------------------------------- > > -# Get test set (dictionary list) from the configuration file > > -# ---------------------------------------------------------- > > -import kvm_config > > - > > -filename = os.path.join(pwd, "kvm_tests.cfg") > > -cfg = kvm_config.config(filename) > > - > > -# If desirable, make changes to the test configuration here. For > > example: > > -# cfg.parse_string("install|setup: timeout_multiplier = 2") > > -# cfg.parse_string("only fc8_quick") > > -# cfg.parse_string("display = sdl") > > - > > -filename = os.path.join(pwd, "kvm_address_pools.cfg") > > -if os.path.exists(filename): > > - cfg.parse_file(filename) > > - hostname = os.uname()[1].split(".")[0] > > - if cfg.filter("^" + hostname): > > - cfg.parse_string("only ^%s" % hostname) > > - else: > > - cfg.parse_string("only ^default_host") > > - > > -list = cfg.get_list() > > - > > - > > -# ------------- > > -# Run the tests > > -# ------------- > > -status_dict = {} > > - > > -for dict in list: > > - if dict.get("skip") == "yes": > > - continue > > - dependencies_satisfied = True > > - for dep in dict.get("depend"): > > - for test_name in status_dict.keys(): > > - if not dep in test_name: > > - continue > > - if not status_dict[test_name]: > > - dependencies_satisfied = False > > - break > > - if dependencies_satisfied: > > - test_iterations = int(dict.get("iterations", 1)) > > - current_status = job.run_test("kvm", params=dict, > > - tag=dict.get("shortname"), > > - iterations=test_iterations) > > - else: > > - current_status = False > > - status_dict[dict.get("name")] = current_status > > - > > -# create the html report in result dir > > -reporter = os.path.join(pwd, 'html_report.py') > > -html_file = os.path.join(job.resultdir,'results.html') > > -os.system('%s -r %s -f %s -R' % (reporter, job.resultdir, > > html_file)) > > +## Generate a nice HTML report inside results_dir > > +kvm_utils.create_report(kvm_test_dir, job.resultsdir) > > diff --git a/client/tests/kvm/kvm_build.cfg.sample > > b/client/tests/kvm/kvm_build.cfg.sample > > new file mode 100644 > > index 0000000..692a747 > > --- /dev/null > > +++ b/client/tests/kvm/kvm_build.cfg.sample > > @@ -0,0 +1,82 @@ > > +# This configuration file holds the KVM build test config > > parameters. > > +# The default is noinstall (won't attempt to build KVM), so if you > > stick with it > > +# please make sure: > > +# 1) You have setup symbolic links to qemu and qemu-img binaries on > > the > > +# KVM test dir. > > +# 2) The appropriate KVM modules are already loaded on your machine. > > + > > +variants: > > + - build: > > + type = build > > + # Load modules built/installed by the build test? > > + load_modules = no > > + variants: > > + - noinstall: > > + mode = noinstall > > + - release: > > + mode = release > > + ## Install from a kvm release. You can optionally > > specify > > + ## a release tag. If you omit it, the build test will > > get > > + ## the latest release tag available at that moment. > > + # release_tag = 84 > > + release_dir = > > http://downloads.sourceforge.net/project/kvm/ > > + release_listing = > > http://sourceforge.net/projects/kvm/files/ > > + - snapshot: > > + mode = snapshot > > + ## Install from a kvm snapshot location. You can > > optionally > > + ## specify a snapshot date. If you omit it, the test > > will get > > + ## yesterday's snapshot. > > + # snapshot_date = 20090712 > > + snapshot_dir = http://foo.org/kvm-snapshots/ > > + - localtar: > > + mode = localtar > > + ## Install from tarball located on the host's > > filesystem. > > + tarball = /tmp/kvm-84.tar.gz > > + - localsrc: > > + mode = localsrc > > + ## Install from tarball located on the host's > > filesystem. > > + srcdir = /tmp/kvm-84 > > + - git: > > + mode = git > > + ## Install KVM from git repositories. > > + ## If you provide only "git_repo" and > > "user_git_repo", the > > + ## build test will assume it will perform all build > > from the > > + ## userspace dir, building modules trough > > + ## make -C kernel LINUX=%s sync. As of today > > (07-13-2009) > > + ## upstream needs 3 git repos: > > + ## * git_repo (linux sources) > > + ## * user_git_repo (qemu sources) > > + ## * kmod_repo" to build KVM userspace + kernel > > modules. > > + git_repo = > > git://git.kernel.org/pub/scm/linux/kernel/git/avi/kvm.git > > + # kernel_branch = kernel_branch_name > > + # kernel_lbranch = kernel_lbranch_name > > + # kernel_tag = kernel_tag_name > > + user_git_repo = > > git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git > > + # user_branch = user_branch_name > > + # user_lbranch = user_lbranch_name > > + # user_tag = user_tag_name > > + kmod_repo = > > git://git.kernel.org/pub/scm/virt/kvm/kvm-kmod.git > > + # kmod_branch = kmod_branch_name > > + # kmod_lbranch = kmod_lbranch_name > > + # kmod_tag = kmod_tag_name > > + - koji: > > + mode = koji > > + ## Install KVM from koji (Fedora build server) > > + ## It is possible to install packages right from Koji > > if you > > + ## provide a release tag or a build. > > + ## Tag (if available) > > + koji_tag = dist-f11 > > + ## Build (if available, is going to override tag). > > + koji_build = qemu-0.10-16.fc11 > > + ## Command to interact with the build server > > + koji_cmd = /usr/bin/koji > > + ## The name of the source package that will be built > > + src_pkg = qemu > > + ## Name of the rpms we need installed > > + pkg_list = ['qemu-kvm', 'qemu-kvm-tools', > > 'qemu-system-x86', 'qemu-common', 'qemu-img'] > > + ## Paths of the qemu relevant executables that should > > be checked > > + qemu_bin_paths = ['/usr/bin/qemu-kvm', > > '/usr/bin/qemu-img'] > > + > > + > > +# Choose your install method here > > +only build.noinstall > > \ No newline at end of file > > diff --git a/client/tests/kvm/kvm_utils.py > > b/client/tests/kvm/kvm_utils.py > > index f72984a..90b3def 100644 > > --- a/client/tests/kvm/kvm_utils.py > > +++ b/client/tests/kvm/kvm_utils.py > > @@ -871,3 +871,115 @@ def unmap_url_cache(cachedir, url, > > expected_hash, method="md5"): > > file_path = utils.unmap_url(cachedir, src, cachedir) > > > > return file_path > > + > > + > > +def prepare_test_run(basedir, rootdir='/tmp/kvm_autotest_root'): > > + """ > > + Prepare the environment for a KVM test run by linking some > > previously > > + prepared directories to the test bindir. > > + > > + @param basedir: Location of the KVM test directory > > + @param rootdir: Base directory that contains the KVM support > > directory > > + structure: > > + * isos (holds guest install isos) > > + * images (will hold guest disk images) > > + * qemu (qemu binary) > > + * qemu-img (qemu-img binary) > > + """ > > + # set English environment (command output might be localized, > > + # need to be safe) > > + os.environ['LANG'] = 'en_US.UTF-8' > > + > > + # Symbolic links we desire to see in the KVM test dir > > + list_symlinks = ['isos', 'images', 'qemu', 'qemu-img'] > > + for symlink_basename in list_symlinks: > > + link_src = os.path.join(rootdir, symlink_basename) > > + link_dst = os.path.join(basedir, symlink_basename) > > + if not os.path.islink(link_dst): > > + os.symlink(link_src, link_dst) > > + # Testing whether our symlinks are good and in shape > > + link_good = os.path.exists(os.readlink(link_dst)) > > + if not link_good: > > + if symlink_basename in ['isos', 'images']: > > + logging.critical("Mandatory directory %s is a broken > > symlink. " > > + "This job will be aborted so you can > > fix the " > > + "problem. Make sure %s points to a > > valid " > > + "directory before proceeding.", > > + symlink_basename, link_dst) > > + raise error.JobError("Mandatory directory %s is a > > broken " > > + "symlink. Please correct it." > > % > > + (link_dst)) > > + else: > > + logging.warning("Symlink %s is broken. You might have > > problems " > > + "later in the test if the KVM build > > test mode " > > + "selected is 'noinstall' (default). > > ", link_dst) > > As mentioned before I'm not sure we need this function at all. > If the user has to modify kvm_tests.cfg.sample anyway, he/she will see our > comments regarding the qemu/images/isos/steps paths. > > > +def get_test_list(cfg, cfg_name, base_dir): > > + """ > > + Return a list of test dictionaries composed by the KVM > > configuration system. > > + This might look like a delegate of the get_list() method of the > > + configuration, but in fact we might want to do custom config > > processing > > + depending on the config file name provided. > > + > > + @param cfg: KVM test configuration object > > + @param cfg_name: Basename of the configuration file that > > generated the > > + configuration object > > + @param base_dir: Directory that holds the config files > > + > > + @return: A list with test parameters dictionaries. > > + """ > > + if cfg_name == 'kvm_tests.cfg': > > + pools = os.path.join(base_dir, "kvm_address_pools.cfg") > > + if os.path.exists(pools): > > + cfg.parse_file(pools) > > + hostname = os.uname()[1].split(".")[0] > > + if cfg.filter("^" + hostname): > > + cfg.parse_string("only ^%s" % hostname) > > + else: > > + cfg.parse_string("only ^default_host") > > + > > + return cfg.get_list() > > Already commented on this one above. > > > +def run_tests(test_list, job): > > + """ > > + Runs the sequence of KVM tests based on the list of dictionaries > > + generated by the configuration system, handling dependencies. > > + > > + @param test_list: List with all dictionary test parameters. > > + @param job: Autotest job object. > > + """ > > + status_dict = {} > > + > > + for dict in test_list: > > + if dict.get("skip") == "yes": > > + continue > > + dependencies_satisfied = True > > + for dep in dict.get("depend"): > > + for test_name in status_dict.keys(): > > + if not dep in test_name: > > + continue > > + if not status_dict[test_name]: > > + dependencies_satisfied = False > > + break > > + if dependencies_satisfied: > > + test_iterations = int(dict.get("iterations", 1)) > > + current_status = job.run_test("kvm", params=dict, > > + tag=dict.get("shortname"), > > + > > iterations=test_iterations) > > + else: > > + current_status = False > > + status_dict[dict.get("name")] = current_status > > Already commented on this one above. > > > +def create_report(report_dir, results_dir): > > + """ > > + Creates a neatly arranged HTML results report in the results > > dir. > > + > > + @param report_dir: Directory where the report script is located. > > + @param results_dir: Directory where the results will be output. > > + """ > > + reporter = os.path.join(report_dir, 'html_report.py') > > + html_file = os.path.join(results_dir, 'results.html') > > + os.system('%s -r %s -f %s -R' % (reporter, results_dir, > > html_file)) > > -- > > 1.6.2.5 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
