From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Goldish Subject: Re: [PATCH][KVM-AUTOTEST] Add custom install option for kvm_install Date: Mon, 11 May 2009 13:36:26 -0400 (EDT) Message-ID: <443364434.369371242063386949.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com> References: <1254147621.368741242063155059.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Uri Lublin , Eduardo Habkost To: mburns@redhat.com Return-path: Received: from mx1.redhat.com ([66.187.233.31]:51271 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752392AbZEKRg1 (ORCPT ); Mon, 11 May 2009 13:36:27 -0400 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n4BHaSfj016279 for ; Mon, 11 May 2009 13:36:28 -0400 In-Reply-To: <1254147621.368741242063155059.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: ----- "Mike Burns" wrote: > Eduardo Habkost wrote: > > Hi, > > > > Excerpts from Michael Burns's message of Fri May 08 15:55:30 -0300 > 2009: > > > >> --- a/client/tests/kvm_runtest_2/kvm_install.py > >> +++ b/client/tests/kvm_runtest_2/kvm_install.py > >> @@ -77,6 +77,17 @@ 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(os.environ['AUTODIR'],install_script) > >> + if not install_script: > >> + message = "Custom script filename not specified" > >> + kvm_log.error(message) > >> + raise error.TestError, message > >> + kvm_log.info("Running " + script + " to install kvm") > >> + os.system(script) > >> > > > > What if we had some way to pass the other parameters from 'params' > to > > the custom script? > > > > Maybe something like (untested): > > > > for k in params.keys(): > > os.putenv("KVM_INSTALL_%s" % (k), params[k]) > > > > Are all values on 'params' guaranteed to be strings, or they can be > set > > to any python value? In the latter case, we could use > str(params[k]), or > > export only the string parameters. > > > That's a good idea. I'm not sure about whether the params are all > strings. I'll try it out and respin the patch after. 1. Not all params are strings -- 'depend' is a list of strings, so str(params[k]) is a good idea. 2. Why not just pass the parameters via the command line, e.g. install_command = my_script.sh param1 param2 3. Why do you use os.environ['AUTODIR'] instead of test.bindir? As far as I know, $AUTODIR is the 'client' dir, while test.bindir takes you directly to kvm_runtest_2. 4. It may be useful to run the script in the kvm_runtest_2 dir, in case the script wants to create symlinks or anything like that. So the last line could be something like os.system("cd %s; %s" % (test.bindir, install_script))