[PATCH][KVM-AUTOTEST] Have Custom install chdir to test.bindir

[PATCH][KVM-AUTOTEST] Have Custom install chdir to test.bindir

[Mike Burns]

If you are using custom install and load_modules=yes, then autotest needs to
change to the test.bindir so that it looks for kernel modules in the right
place.

Signed-off-by: Mike Burns <mburns@redhat.com>
---
 client/tests/kvm_runtest_2/kvm_install.py |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm_runtest_2/kvm_install.py b/client/tests/kvm_runtest_2/kvm_install.py
index d1aceb2..ebd8b7d 100755
--- a/client/tests/kvm_runtest_2/kvm_install.py
+++ b/client/tests/kvm_runtest_2/kvm_install.py
@@ -79,6 +79,7 @@ def run_kvm_install(test, params, env):
 
     # install from custom script
     elif install_mode == "custom":
+        os.chdir(test.bindir)
         install_script = params.get("install_script")
         script = os.path.join(test.bindir,install_script)
         if not install_script:
-- 
1.5.5.6

[PATCH][KVM-AUTOTEST] Check exit status of custom install script and fail if script failed.

[Mike Burns]

Signed-off-by: Mike Burns <mburns@redhat.com>
---
 client/tests/kvm_runtest_2/kvm_install.py |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm_runtest_2/kvm_install.py b/client/tests/kvm_runtest_2/kvm_install.py
index ebd8b7d..392ef0c 100755
--- a/client/tests/kvm_runtest_2/kvm_install.py
+++ b/client/tests/kvm_runtest_2/kvm_install.py
@@ -90,7 +90,12 @@ def run_kvm_install(test, params, env):
 	  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))
+        install_result = os.system("cd %s; %s" % (test.bindir, script))
+	if os.WEXITSTATUS(install_result) != 0:
+          message = "Custom Script encountered an error"
+          kvm_log.error(message)
+          raise error.TestError, message
+
 	kvm_log.info("Completed %s" % (script))
 
     # invalid installation mode
-- 
1.5.5.6

Re: [PATCH][KVM-AUTOTEST] Check exit status of custom install script and fail if script failed.

[Avi Kivity]

Mike Burns wrote:
> Signed-off-by: Mike Burns <mburns@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_install.py |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/client/tests/kvm_runtest_2/kvm_install.py b/client/tests/kvm_runtest_2/kvm_install.py
> index ebd8b7d..392ef0c 100755
> --- a/client/tests/kvm_runtest_2/kvm_install.py
> +++ b/client/tests/kvm_runtest_2/kvm_install.py
> @@ -90,7 +90,12 @@ def run_kvm_install(test, params, env):
>  	  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))
> +        install_result = os.system("cd %s; %s" % (test.bindir, script))
> +	if os.WEXITSTATUS(install_result) != 0:
> +          message = "Custom Script encountered an error"
> +          kvm_log.error(message)
> +          raise error.TestError, message
> +
>   

How about a helper that does os.system()  (or rather, 
commands.getstatusoutput()) and throws an exception on failure?  I 
imagine it could be used in many places.

Oh and "Custom script encountered an error" is unfriendly.  Mention the 
command line and the actual output.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH][KVM-AUTOTEST] Have Custom install chdir to test.bindir

[Avi Kivity]

Mike Burns wrote:
> If you are using custom install and load_modules=yes, then autotest needs to
> change to the test.bindir so that it looks for kernel modules in the right
> place.
>
> Signed-off-by: Mike Burns <mburns@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_install.py |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/client/tests/kvm_runtest_2/kvm_install.py b/client/tests/kvm_runtest_2/kvm_install.py
> index d1aceb2..ebd8b7d 100755
> --- a/client/tests/kvm_runtest_2/kvm_install.py
> +++ b/client/tests/kvm_runtest_2/kvm_install.py
> @@ -79,6 +79,7 @@ def run_kvm_install(test, params, env):
>  
>      # install from custom script
>      elif install_mode == "custom":
> +        os.chdir(test.bindir)
>          install_script = params.get("install_script")
>          script = os.path.join(test.bindir,install_script)
>          if not install_script:
>   

Don't you need to chdir back afterwards?

Opportunity for a with statement.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH][KVM-AUTOTEST] Check exit status of custom install script and fail if script failed.

[Lucas Meneghel Rodrigues]

On Sun, 2009-05-24 at 17:48 +0300, Avi Kivity wrote:
> Mike Burns wrote:
> > Signed-off-by: Mike Burns <mburns@redhat.com>
> > ---
> >  client/tests/kvm_runtest_2/kvm_install.py |    7 ++++++-
> >  1 files changed, 6 insertions(+), 1 deletions(-)
> >
> > diff --git a/client/tests/kvm_runtest_2/kvm_install.py b/client/tests/kvm_runtest_2/kvm_install.py
> > index ebd8b7d..392ef0c 100755
> > --- a/client/tests/kvm_runtest_2/kvm_install.py
> > +++ b/client/tests/kvm_runtest_2/kvm_install.py
> > @@ -90,7 +90,12 @@ def run_kvm_install(test, params, env):
> >  	  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))
> > +        install_result = os.system("cd %s; %s" % (test.bindir, script))
> > +	if os.WEXITSTATUS(install_result) != 0:
> > +          message = "Custom Script encountered an error"
> > +          kvm_log.error(message)
> > +          raise error.TestError, message
> > +
> >   
> 
> How about a helper that does os.system()  (or rather, 
> commands.getstatusoutput()) and throws an exception on failure?  I 
> imagine it could be used in many places.

utils.system() does that. If we have exit code != 0, it throws an
error.CmdError exception.

-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [PATCH][KVM-AUTOTEST] Check exit status of custom install script and fail if script failed.

[Avi Kivity]

Lucas Meneghel Rodrigues wrote:
> On Sun, 2009-05-24 at 17:48 +0300, Avi Kivity wrote:
>   
>> Mike Burns wrote:
>>     
>>> Signed-off-by: Mike Burns <mburns@redhat.com>
>>> ---
>>>  client/tests/kvm_runtest_2/kvm_install.py |    7 ++++++-
>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/client/tests/kvm_runtest_2/kvm_install.py b/client/tests/kvm_runtest_2/kvm_install.py
>>> index ebd8b7d..392ef0c 100755
>>> --- a/client/tests/kvm_runtest_2/kvm_install.py
>>> +++ b/client/tests/kvm_runtest_2/kvm_install.py
>>> @@ -90,7 +90,12 @@ def run_kvm_install(test, params, env):
>>>  	  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))
>>> +        install_result = os.system("cd %s; %s" % (test.bindir, script))
>>> +	if os.WEXITSTATUS(install_result) != 0:
>>> +          message = "Custom Script encountered an error"
>>> +          kvm_log.error(message)
>>> +          raise error.TestError, message
>>> +
>>>   
>>>       
>> How about a helper that does os.system()  (or rather, 
>> commands.getstatusoutput()) and throws an exception on failure?  I 
>> imagine it could be used in many places.
>>     
>
> utils.system() does that. If we have exit code != 0, it throws an
> error.CmdError exception.
>   

Well let's use it then.  Every time I see 'raise' used I'm going to 
complain, so it will be a lot more efficient as well as smaller code.

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

Re: [PATCH][KVM-AUTOTEST] Check exit status of custom install script and fail if script failed.

[Mike Burns]

Avi Kivity wrote:
> Lucas Meneghel Rodrigues wrote:
>> On Sun, 2009-05-24 at 17:48 +0300, Avi Kivity wrote:
>>  
>>> Mike Burns wrote:
>>>    
>>>> Signed-off-by: Mike Burns <mburns@redhat.com>
>>>> ---
>>>>  client/tests/kvm_runtest_2/kvm_install.py |    7 ++++++-
>>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/client/tests/kvm_runtest_2/kvm_install.py
>>>> b/client/tests/kvm_runtest_2/kvm_install.py
>>>> index ebd8b7d..392ef0c 100755
>>>> --- a/client/tests/kvm_runtest_2/kvm_install.py
>>>> +++ b/client/tests/kvm_runtest_2/kvm_install.py
>>>> @@ -90,7 +90,12 @@ def run_kvm_install(test, params, env):
>>>>        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))
>>>> +        install_result = os.system("cd %s; %s" % (test.bindir,
>>>> script))
>>>> +    if os.WEXITSTATUS(install_result) != 0:
>>>> +          message = "Custom Script encountered an error"
>>>> +          kvm_log.error(message)
>>>> +          raise error.TestError, message
>>>> +
>>>>         
>>> How about a helper that does os.system()  (or rather,
>>> commands.getstatusoutput()) and throws an exception on failure?  I
>>> imagine it could be used in many places.
>>>     
>>
>> utils.system() does that. If we have exit code != 0, it throws an
>> error.CmdError exception.
>>   
>
> Well let's use it then.  Every time I see 'raise' used I'm going to
> complain, so it will be a lot more efficient as well as smaller code.
>
Agreed.  I'll rework this and my other patches and get them re-posted in
the next day or two.

Mike

Re: [PATCH][KVM-AUTOTEST] Have Custom install chdir to test.bindir

[Mike Burns]

Avi Kivity wrote:
> Mike Burns wrote:
>> If you are using custom install and load_modules=yes, then autotest
>> needs to
>> change to the test.bindir so that it looks for kernel modules in the
>> right
>> place.
>>
>> Signed-off-by: Mike Burns <mburns@redhat.com>
>> ---
>>  client/tests/kvm_runtest_2/kvm_install.py |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/client/tests/kvm_runtest_2/kvm_install.py
>> b/client/tests/kvm_runtest_2/kvm_install.py
>> index d1aceb2..ebd8b7d 100755
>> --- a/client/tests/kvm_runtest_2/kvm_install.py
>> +++ b/client/tests/kvm_runtest_2/kvm_install.py
>> @@ -79,6 +79,7 @@ def run_kvm_install(test, params, env):
>>  
>>      # install from custom script
>>      elif install_mode == "custom":
>> +        os.chdir(test.bindir)
>>          install_script = params.get("install_script")
>>          script = os.path.join(test.bindir,install_script)
>>          if not install_script:
>>   
>
> Don't you need to chdir back afterwards?
>
> Opportunity for a with statement.
>
No, the custom install option essentially is replacing the call to
__install_kvm which does a chdir and doesn't ever revert back to the
original directory.  Either this is handled somewhere outside of the
kvm_install.py script or it is not necessary.

Mike