From mboxrd@z Thu Jan  1 00:00:00 1970
From: Uri Lublin <uril@redhat.com>
Subject: Re: [PATCH][KVM-AUTOTEST] Add custom install option for kvm_install
Date: Mon, 01 Jun 2009 18:46:30 +0300
Message-ID: <4A23F7D6.3070109@redhat.com>
References: <1242142476-2878-1-git-send-email-mburns@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org, ulublin@redhat.com
To: Mike Burns <mburns@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:53239 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751891AbZFAPqb (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 1 Jun 2009 11:46:31 -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 n51FkX9u027374
	for <kvm@vger.kernel.org>; Mon, 1 Jun 2009 11:46:33 -0400
In-Reply-To: <1242142476-2878-1-git-send-email-mburns@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 05/12/2009 06:34 PM, Mike Burns wrote:
> From: Michael Burns<mburns@redhat.com>
>
>
> Signed-off-by: Michael Burns<mburns@redhat.com>
> ---
>   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_<s>  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

>   # ---------------------
>   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.

> +        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).

> +	kvm_log.info("Completed %s" % (script))
> +
>       # invalid installation mode
>       else:
>           message = "Invalid installation mode: '%s'" % install_mode



Sorry for the late reply,
     Uri.