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

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

[Michael Goldish]

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

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

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

[Mike Burns]

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

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

[Eduardo Habkost]

On Mon, May 11, 2009 at 01:52:19PM -0400, Mike Burns wrote:
> Michael Goldish wrote:
<snip>
>> 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. 

What is bash-specific about environment variables or command-line args?

Also, exporting the config dictionary as environment variables doesn't
exclude the possibility of simply passing command-line arguments on
install_command, if the user wants to do so. Both methods would work.

-- 
Eduardo

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

[Mike Burns]

Eduardo Habkost wrote:
> On Mon, May 11, 2009 at 01:52:19PM -0400, Mike Burns wrote:
>   
>> Michael Goldish wrote:
>>     
> <snip>
>   
>>> 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. 
>>     
>
> What is bash-specific about environment variables or command-line args?
>
> Also, exporting the config dictionary as environment variables doesn't
> exclude the possibility of simply passing command-line arguments on
> install_command, if the user wants to do so. Both methods would work.
>
>   
I suppose it doesn't. I've found that some languages make it easier to 
deal with command line as opposed to environment, but both would work.

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

[Eduardo Habkost]

On Mon, May 11, 2009 at 01:36:26PM -0400, Michael Goldish wrote:
> 
> ----- "Mike Burns" <mburns@redhat.com> wrote:
> 
> > Eduardo Habkost wrote:
<snip>
> > >
> > > 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.

Agreed. Using str(params[k]) won't hurt.

> 
> 2. Why not just pass the parameters via the command line, e.g.
> install_command = my_script.sh param1 param2

I think keeping the same convention for parameter passing for custom
scripts makes it easier to make custom scripts that behave similarly to
the predefined rules, but with just a few differences.

Also, can't the install rule be used on the cartesian configuration
file? In this case, the install parameters may be specified on a
different config rule.

For example, the Fedora project could use it like this:

cvs_server = "cvs.fedoraproject.org:/..."
variants:
  - CVSF10:
    cvs_branch = F10
  - CVSF11:
    cvs_branch = F11
  - CVSRawhide:
    cvs_branch = devel


variants:
  - install:
    type = kvm_install
    mode = custom
    install_command = "install_from_fedora_cvs.sh"
  - ...

I used Fedora CVS as an example, but the user may use anything we can
imagine, to store KVM code (or pointer to its), possibly having
different branches.

-- 
Eduardo

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

[Mike Burns]

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.  
+#
 # ---------------------
 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)
+        if not install_script:
+            message = "Custom script filename not specified"
+            kvm_log.error(message)
+            raise error.TestError, message
+        for k in params.keys():
+	  kvm_log.info("Adding KVM_INSTALL_%s to Environment" % (k))
+          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))
+	kvm_log.info("Completed %s" % (script))
+
     # invalid installation mode
     else:
         message = "Invalid installation mode: '%s'" % install_mode
-- 
1.6.0.6

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

[Uri Lublin]

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.

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

[Mike Burns]

Uri Lublin wrote:
> 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
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.

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

[Michael Goldish]

----- "Eduardo Habkost" <ehabkost@redhat.com> wrote:

> On Mon, May 11, 2009 at 01:36:26PM -0400, Michael Goldish wrote:
> > 
> > ----- "Mike Burns" <mburns@redhat.com> wrote:
> > 
> > > Eduardo Habkost wrote:
> <snip>
> > > >
> > > > 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.
> 
> Agreed. Using str(params[k]) won't hurt.
> 
> > 
> > 2. Why not just pass the parameters via the command line, e.g.
> > install_command = my_script.sh param1 param2
> 
> I think keeping the same convention for parameter passing for custom
> scripts makes it easier to make custom scripts that behave similarly
> to the predefined rules, but with just a few differences.

OK, this makes sense. In any case I think we should accept a command
string rather than a script filename, in order to allow both methods
of passing parameters.

> Also, can't the install rule be used on the cartesian configuration
> file? In this case, the install parameters may be specified on a
> different config rule.

It's certainly possible. We chose to put kvm_install in the control
file because currently it's the only test we run only once per job.
There is no natural place for kvm_install in kvm_tests.cfg because
everything else there gets multiplied.

> For example, the Fedora project could use it like this:
> 
> cvs_server = "cvs.fedoraproject.org:/..."
> variants:
>   - CVSF10:
>     cvs_branch = F10
>   - CVSF11:
>     cvs_branch = F11
>   - CVSRawhide:
>     cvs_branch = devel
> 
> 
> variants:
>   - install:
>     type = kvm_install
>     mode = custom
>     install_command = "install_from_fedora_cvs.sh"
>   - ...
> 
> I used Fedora CVS as an example, but the user may use anything we can
> imagine, to store KVM code (or pointer to its), possibly having
> different branches.

This can be done, though the order and syntax of the 'variants' blocks
should be different, more like:

variants:
  - kvm_install:
    cvs_server = "cvs.fedoraproject.org:/..."
    type = kvm_install
    mode = custom
    install_command = "install_from_fedora_cvs.sh"
  - everything_else_in_kvm_tests.cfg:
        <tests>
        ...
        <guests>
        ...
        <smp/up>
        ...
        <ide/scsi/virtio_blk>
        ...

variants:
  - CVSF10:
    kvm_install:
        cvs_branch = F10
  - CVSF11:
    kvm_install:
        cvs_branch = F11
  - CVSRawhide:
    kvm_install:
        cvs_branch = devel

# test sets
variants:
    - nightly:
        ...

That is, assuming you want to
- install KVM from F10 branch
- run all tests
- install KVM from F11 branch
- run all tests
- install KVM from devel branch
- run all tests

If you meant something different please correct me.

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

[Eduardo Pereira Habkost]

Excerpts from Michael Goldish's message of Mon May 11 16:00:29 -0300 2009:
> 
> ----- "Eduardo Habkost" <ehabkost@redhat.com> wrote:
> 
<snip>
> > Also, can't the install rule be used on the cartesian configuration
> > file? In this case, the install parameters may be specified on a
> > different config rule.
> 
> It's certainly possible. We chose to put kvm_install in the control
> file because currently it's the only test we run only once per job.
> There is no natural place for kvm_install in kvm_tests.cfg because
> everything else there gets multiplied.

True. I haven't thought of that.

> 
> > For example, the Fedora project could use it like this:
> > 
> > cvs_server = "cvs.fedoraproject.org:/..."
> > variants:
> >   - CVSF10:
> >     cvs_branch = F10
> >   - CVSF11:
> >     cvs_branch = F11
> >   - CVSRawhide:
> >     cvs_branch = devel
> > 
> > 
> > variants:
> >   - install:
> >     type = kvm_install
> >     mode = custom
> >     install_command = "install_from_fedora_cvs.sh"
> >   - ...
> > 
> > I used Fedora CVS as an example, but the user may use anything we can
> > imagine, to store KVM code (or pointer to its), possibly having
> > different branches.
> 
> This can be done, though the order and syntax of the 'variants' blocks
> should be different, more like:
> 
> variants:
>   - kvm_install:
>     cvs_server = "cvs.fedoraproject.org:/..."
>     type = kvm_install
>     mode = custom
>     install_command = "install_from_fedora_cvs.sh"
>   - everything_else_in_kvm_tests.cfg:
>         <tests>
>         ...
>         <guests>
>         ...
>         <smp/up>
>         ...
>         <ide/scsi/virtio_blk>
>         ...

We could have something to make it easier to build test runs like ,
without having to nest the whole original file. But I can't think of a
good solution.


> 
> variants:
>   - CVSF10:
>     kvm_install:
>         cvs_branch = F10
>   - CVSF11:
>     kvm_install:
>         cvs_branch = F11
>   - CVSRawhide:
>     kvm_install:
>         cvs_branch = devel
> 
> # test sets
> variants:
>     - nightly:
>         ...
> 
> That is, assuming you want to
> - install KVM from F10 branch
> - run all tests
> - install KVM from F11 branch
> - run all tests
> - install KVM from devel branch
> - run all tests
> 
> If you meant something different please correct me.

Yeah. I am not really used to the configuration file format, but that
was the idea.  :)
-- 
Eduardo

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

[Mike Burns]

Eduardo Pereira Habkost wrote:
> Excerpts from Michael Goldish's message of Mon May 11 16:00:29 -0300 2009:
>   
>> ----- "Eduardo Habkost" <ehabkost@redhat.com> wrote:
>>
>>     
> <snip>
>   
>>> Also, can't the install rule be used on the cartesian configuration
>>> file? In this case, the install parameters may be specified on a
>>> different config rule.
>>>       
>> It's certainly possible. We chose to put kvm_install in the control
>> file because currently it's the only test we run only once per job.
>> There is no natural place for kvm_install in kvm_tests.cfg because
>> everything else there gets multiplied.
>>     
>
> True. I haven't thought of that.
>
>   
>>> For example, the Fedora project could use it like this:
>>>
>>> cvs_server = "cvs.fedoraproject.org:/..."
>>> variants:
>>>   - CVSF10:
>>>     cvs_branch = F10
>>>   - CVSF11:
>>>     cvs_branch = F11
>>>   - CVSRawhide:
>>>     cvs_branch = devel
>>>
>>>
>>> variants:
>>>   - install:
>>>     type = kvm_install
>>>     mode = custom
>>>     install_command = "install_from_fedora_cvs.sh"
>>>   - ...
>>>
>>> I used Fedora CVS as an example, but the user may use anything we can
>>> imagine, to store KVM code (or pointer to its), possibly having
>>> different branches.
>>>       
>> This can be done, though the order and syntax of the 'variants' blocks
>> should be different, more like:
>>
>> variants:
>>   - kvm_install:
>>     cvs_server = "cvs.fedoraproject.org:/..."
>>     type = kvm_install
>>     mode = custom
>>     install_command = "install_from_fedora_cvs.sh"
>>   - everything_else_in_kvm_tests.cfg:
>>         <tests>
>>         ...
>>         <guests>
>>         ...
>>         <smp/up>
>>         ...
>>         <ide/scsi/virtio_blk>
>>         ...
>>     
>
> We could have something to make it easier to build test runs like ,
> without having to nest the whole original file. But I can't think of a
> good solution.
>
>
>   
>> variants:
>>   - CVSF10:
>>     kvm_install:
>>         cvs_branch = F10
>>   - CVSF11:
>>     kvm_install:
>>         cvs_branch = F11
>>   - CVSRawhide:
>>     kvm_install:
>>         cvs_branch = devel
>>
>> # test sets
>> variants:
>>     - nightly:
>>         ...
>>
>> That is, assuming you want to
>> - install KVM from F10 branch
>> - run all tests
>> - install KVM from F11 branch
>> - run all tests
>> - install KVM from devel branch
>> - run all tests
>>
>> If you meant something different please correct me.
>>     
>
> Yeah. I am not really used to the configuration file format, but that
> was the idea.  :)
>   

I've reworked the patch to use test.bindir and force it to run from 
kvm_runtest_2.  I also doubled checked that both parameters and 
variables defined in the params object are available to the script.  The 
updated past should go out shortly. 

Right now, nothing from the kvm_tests.cfg file is available.  I don't 
see an obvious way to do that, but we can put that into a separate patch 
if needed later.

Mike

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

[Avi Kivity]

Michael Goldish wrote:
> That is, assuming you want to
> - install KVM from F10 branch
> - run all tests
> - install KVM from F11 branch
> - run all tests
> - install KVM from devel branch
> - run all tests
>
> If you meant something different please correct me.
>   

Note kvm is moving to split userspace/kernel packaging, so this can me 
useful to test version compatibility.

i.e. test kvm-kmod A, B, C vs qemu-kvm X, Y, Z.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

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

[Mike Burns]

From: Michael Burns <mburns@redhat.com>


Signed-off-by: Michael Burns <mburns@redhat.com>
---
 client/tests/kvm_runtest_2/control        |   17 ++++++++++++++++-
 client/tests/kvm_runtest_2/kvm_install.py |   13 +++++++++++++
 2 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm_runtest_2/control b/client/tests/kvm_runtest_2/control
index fd68e94..5d022b5 100644
--- a/client/tests/kvm_runtest_2/control
+++ b/client/tests/kvm_runtest_2/control
@@ -41,6 +41,18 @@ 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-autotest/client directory
+#   Note:  For custom parameters for your script, define them as params below.
+#       They will be available as KVM_INSTALL_<name> in the OS environment
+#       Example: install_script will be available as KVM_INSTALL_install_script
+#
 # ---------------------
 params = {
     "name": "kvm_install",
@@ -57,7 +69,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": 'tests/kvm_runtest_2/custom_kvm_install.sh'
 }
 
 # 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..bb664b3 100755
--- a/client/tests/kvm_runtest_2/kvm_install.py
+++ b/client/tests/kvm_runtest_2/kvm_install.py
@@ -77,6 +77,19 @@ 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
+        for k in params.keys():
+          os.putenv("KVM_INSTALL_%s" % (k), str(params[k]))
+	kvm_log.info("Running " + script + " to install kvm")
+        os.system(script)
+
     # invalid installation mode
     else:
         message = "Invalid installation mode: '%s'" % install_mode
-- 
1.6.0.6

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

[Lucas Meneghel Rodrigues]

Hi Mike

On Mon, 2009-05-11 at 11:51 -0400, Mike Burns wrote:
> From: Michael Burns <mburns@redhat.com>

The comments I had to make were pretty much covered by Eduardo and Mike
Goldish... 

 1 - We should support passing parameters by both environment variables
and command line arguments
 2 - test.bindir sounds like a better choice of path than the env
variable AUTODIR

> 
> Signed-off-by: Michael Burns <mburns@redhat.com>
> ---
>  client/tests/kvm_runtest_2/control        |   17 ++++++++++++++++-
>  client/tests/kvm_runtest_2/kvm_install.py |   13 +++++++++++++
>  2 files changed, 29 insertions(+), 1 deletions(-)
> 
> diff --git a/client/tests/kvm_runtest_2/control b/client/tests/kvm_runtest_2/control
> index fd68e94..5d022b5 100644
> --- a/client/tests/kvm_runtest_2/control
> +++ b/client/tests/kvm_runtest_2/control
> @@ -41,6 +41,18 @@ 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-autotest/client directory
> +#   Note:  For custom parameters for your script, define them as params below.
> +#       They will be available as KVM_INSTALL_<name> in the OS environment
> +#       Example: install_script will be available as KVM_INSTALL_install_script
> +#
>  # ---------------------
>  params = {
>      "name": "kvm_install",
> @@ -57,7 +69,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": 'tests/kvm_runtest_2/custom_kvm_install.sh'
>  }
>  
>  # 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..bb664b3 100755
> --- a/client/tests/kvm_runtest_2/kvm_install.py
> +++ b/client/tests/kvm_runtest_2/kvm_install.py
> @@ -77,6 +77,19 @@ 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
> +        for k in params.keys():
> +          os.putenv("KVM_INSTALL_%s" % (k), str(params[k]))
> +	kvm_log.info("Running " + script + " to install kvm")
> +        os.system(script)
> +
>      # invalid installation mode
>      else:
>          message = "Invalid installation mode: '%s'" % install_mode

Lucas

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

[Mike Burns]

From: Michael Burns <mburns@redhat.com>


Signed-off-by: Michael Burns <mburns@redhat.com>
---
 client/tests/kvm_runtest_2/control        |   14 +++++++++++++-
 client/tests/kvm_runtest_2/kvm_install.py |   11 +++++++++++
 2 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm_runtest_2/control b/client/tests/kvm_runtest_2/control
index fd68e94..c28dc67 100644
--- a/client/tests/kvm_runtest_2/control
+++ b/client/tests/kvm_runtest_2/control
@@ -41,6 +41,15 @@ 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-autotest/client directory
+#
 # ---------------------
 params = {
     "name": "kvm_install",
@@ -57,7 +66,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": 'tests/kvm_runtest/custom_kvm_install.sh'
 }
 
 # 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..0e077ae 100755
--- 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)
+
     # invalid installation mode
     else:
         message = "Invalid installation mode: '%s'" % install_mode
-- 
1.6.0.6

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

[Eduardo Habkost]

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

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

[Mike Burns]

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. 

Mike