Re: [PATCH][KVM-AUTOTEST] Add custom install option for kvm_install

[Mike Burns <mburns@redhat.com>]

Michael Goldish wrote:
> ----- "Mike Burns" <mburns@redhat.com> 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.
>   
I put this in the latest post of the patch.  I figured it didn't hurt 
anything being there.
> 2. Why not just pass the parameters via the command line, e.g.
> install_command = my_script.sh param1 param2
>   
That would work as well.  It could be done either way, but if someone 
wanted to use something other than bash, then this way might be easier. 
> 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.
>   
A couple reasons...
1.  I didn't know about test.bindir
2.  I wanted something within the client directory since that is what 
gets copied when running in server mode.  I can see the point in forcing 
it to be in kvm_runtest_2, but if someone wanted a single location for 
all custom scripts that do not apply to just kvm_runtest_2, then i could 
see them creating a directory at the highest level possible.  It is 
easily switched if that is what we want to do instead, but that was my 
thought.
> 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))
>   
I agree.  The script should probably be run from the kvm_runtest_2 
directory. 

In addition, I'm going to be adding a page to the wiki about the control 
file and what the different things that can be set there are.  I'm going 
to concentrate on the params section first, but eventually we should 
expand it to include everything. 

Mike