From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Burns Subject: Re: [PATCH][KVM-AUTOTEST] Add custom install option for kvm_install Date: Mon, 01 Jun 2009 14:25:25 -0400 Message-ID: <4A241D15.5050704@redhat.com> References: <1242142476-2878-1-git-send-email-mburns@redhat.com> <4A23F7D6.3070109@redhat.com> Reply-To: mburns@redhat.com Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, ulublin@redhat.com To: Uri Lublin Return-path: Received: from mx2.redhat.com ([66.187.237.31]:55895 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752782AbZFASZ0 (ORCPT ); Mon, 1 Jun 2009 14:25:26 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n51IPSq3006728 for ; Mon, 1 Jun 2009 14:25:28 -0400 In-Reply-To: <4A23F7D6.3070109@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Uri Lublin wrote: > On 05/12/2009 06:34 PM, Mike Burns wrote: >> From: Michael Burns >> >> >> Signed-off-by: Michael Burns >> --- >> client/tests/kvm_runtest_2/control | 18 +++++++++++++++++- >> client/tests/kvm_runtest_2/kvm_install.py | 15 +++++++++++++++ >> 2 files changed, 32 insertions(+), 1 deletions(-) >> >> diff --git a/client/tests/kvm_runtest_2/control >> b/client/tests/kvm_runtest_2/control >> index fd68e94..d6e26bc 100644 >> --- a/client/tests/kvm_runtest_2/control >> +++ b/client/tests/kvm_runtest_2/control >> @@ -41,6 +41,19 @@ link_if_not_exist(pwd, qemu_img, 'qemu-img') >> >> # --------------------- >> # Build and install kvm >> +# >> +# Details of Install options >> +# Mode: custom >> +# Description: install from custom install script >> +# Parameters needed: >> +# install_script: >> +# location of script relative to the kvm-runtest_2 directory. >> +# Script will be executed from test.bindir (generally >> kvm_runtest_2) >> +# parameters for the script can be passed either as >> environment variables >> +# in the params array below or in the definition of >> install_script. >> +# If they are passed as part of params, then they will be >> accessible as >> +# KVM_INSTALL_ in the OS Environment when your script runs. >> +# > > I think this, being the only explanation about kvm_install, can be > confusing to the user. We can add a link to the wiki instead: > http://kvm.et.redhat.com/page/KVM-Autotest/ControlFile That is a good point. I was debating whether to put them all there or not, but didn't want to clutter up my patch too much. I decided afterwards to add the page to the wiki, so I'll link there instead when I repost. > >> # --------------------- >> params = { >> "name": "kvm_install", >> @@ -57,7 +70,10 @@ params = { >> >> ## Install from git >> "git_repo": >> 'git://git.kernel.org/pub/scm/linux/kernel/git/avi/kvm.git', >> - "user_git_repo": >> 'git://git.kernel.org/pub/scm/linux/kernel/git/avi/kvm-userspace.git' >> + "user_git_repo": >> 'git://git.kernel.org/pub/scm/linux/kernel/git/avi/kvm-userspace.git', >> + >> + ## Custom install >> + "install_script": 'custom_kvm_install.sh param1' >> } >> >> # Comment the job.run_test line if you do not want to install kvm >> on the host. >> diff --git a/client/tests/kvm_runtest_2/kvm_install.py >> b/client/tests/kvm_runtest_2/kvm_install.py >> index 8be5a93..234c77a 100755 >> --- a/client/tests/kvm_runtest_2/kvm_install.py >> +++ b/client/tests/kvm_runtest_2/kvm_install.py >> @@ -77,6 +77,21 @@ def run_kvm_install(test, params, env): >> elif install_mode == "localsrc": >> __install_kvm(test, srcdir) >> >> + # install from custom script >> + elif install_mode == "custom": >> + install_script = params.get("install_script") >> + script = os.path.join(test.bindir,install_script) > > This line (script = ..) should be located below the following if > statement. > if "install_script" is not in params (and install_script is None), > os.path.join fails. > Ok, will fix >> + if not install_script: >> + message = "Custom script filename not specified" >> + kvm_log.error(message) >> + raise error.TestError, message >> + for k in params.keys(): > > Fix white-space. > >> + kvm_log.info("Adding KVM_INSTALL_%s to Environment" % (k)) > > kvm_log.debug > >> + os.putenv("KVM_INSTALL_%s" % (k), str(params[k])) >> + kvm_log.info("Running " + script + " to install kvm") >> + os.system("cd %s; %s" % (test.bindir, script)) > > if the script fails, quit (raise). > we're going to change this to utils.system instead to use the built-in error handling. >> + kvm_log.info("Completed %s" % (script)) >> + >> # invalid installation mode >> else: >> message = "Invalid installation mode: '%s'" % install_mode > > > > Sorry for the late reply, > Uri.