Michael Goldish wrote:
> Looks good to me. See some comments below.
>
> ----- "David Huff" <dhuff@xxxxxxxxxx> 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.
>
> Why not do this for post_command as well?
I didn't do post_command b/c I figured that they would already be
exported in the pre_command, however I guess that there can be the case
where there is no pre and only post and I guess exporting twice will not
hurt anything.... I will add this to the patch.
>
>> ---
>> client/tests/kvm_runtest_2/kvm_preprocessing.py | 31
>> +++++++++++++++++++++-
>> 1 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py
>> b/client/tests/kvm_runtest_2/kvm_preprocessing.py
>> index c9eb35d..02df615 100644
>> --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py
>> +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py
>> @@ -135,8 +135,7 @@ def postprocess_vm(test, params, env, name):
>> "Waiting for VM to kill itself..."):
>> kvm_log.debug("'kill_vm' specified; killing VM...")
>> vm.destroy(gracefully = params.get("kill_vm_gracefully") ==
>> "yes")
>> -
>> -
>> +
>
> I hate to be petty, but we usually keep two blank lines between top
> level functions.
>
> Also, you have some trailing whitespace there...
good catch, I will take care of this
> It wouldn't hurt to make this timeout user-configurable with a default
> value of 600 or so:
>
> timeout = int(params.get("pre_commmand_timeout", "600"))
> (status, pid, output) = kvm_utils.run_bg(..., timeout=timeout)
>
> We can also do that in a separate patch.
>
I'll go ahead and add this while I rework the patch...
-D
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
