Re: [PATCH][KVM_AUTOTEST] Added functionality to the preprocessor to run scripts

[Lucas Meneghel Rodrigues <lmr@redhat.com>]

On Wed, 2009-06-03 at 02:14 -0400, Michael Goldish wrote:
> ----- "Avi Kivity" <avi@redhat.com> wrote:
> 
> > David Huff wrote:
> > > This patch will run pre and post scripts
> > > defined in config file with the parameter pre_command
> > > and post_command post_command.
> > >
> > > Also exports all the prameters in preprocess for passing
> > > arguments to the script.
> > >  
> > > +   #execute any pre_commands
> > > +    pre_command = params.get("pre_command")
> > > +    if pre_command:
> > > +        # export environment vars
> > > +        for k in params.keys():
> > > +            kvm_log.info("Adding KVM_TEST_%s to Environment" %
> > (k))
> > > +            os.putenv("KVM_TEST_%s" % (k), str(params[k]))
> > > +        # execute command
> > > +        kvm_log.info("Executing command '%s'..." % pre_command)
> > > +        timeout = int(params.get("pre_commmand_timeout", "600"))
> > > +        (status, pid, output) = kvm_utils.run_bg("cd %s; %s" %
> > (test.bindir, pre_command),
> > > +                                                 None,
> > kvm_log.debug, "(pre_command) ", timeout=timeout)
> > > +        if status != 0:
> > > +            kvm_utils.safe_kill(pid, signal.SIGTERM)
> > > +            raise error.TestError, "Custom processing pre_command
> > failed"
> > >   
> > 
> > kvm_utils.run_bg should throw an exception instead of returning
> > status.

utils.system or utils.run will throw a error.CmdError exception on exit
code !=0. The representation and the printed format of CmdErrors contain
the exit code, the full string of the command and stuff like that.

And yes, I agree in general that we should throw exceptions when command
fails. If the failure is not bad enough to stop the whole test, we can
catch the exception and increment failure counters or something to keep
record of the failures. 

> - What if we're interested in the status for some reason? Its value
> may indicate what went wrong with the child process.
> - If we throw an exception we should add a parameter that controls
> whether an exception should be thrown (something like "ignore_status")
> because in some cases we don't want to throw an exception.
> - A run_bg call has 3 possible outcomes: success (status == 0), failure
> (status != 0) and timeout (process still running). If we throw an exception
> upon failure, what do we do upon timeout? Sometimes it's good (qemu should
> keep running unless something went wrong) and sometimes bad (pre_command
> should probably not time out). So we end up needing an ignore_timeout
> parameter as well.
> - What if we want the test to fail with an informative test-specific
> exception such as "something failed after migration"?
> 
> > But if status != 0, will there actually be a pid to kill?
> 
> If the timeout expires and the process is still running, status is None.
> 
> > -- 
> > error compiling committee.c: too many arguments to function
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies