Other than that, the rest of the patchset looks good. I've noticed
this patch does not apply cleanly, so in v4, please rebase it.

Thanks a bunch for your work!

On Fri, Jul 27, 2012 at 11:53 AM, Lucas Meneghel Rodrigues
<[email protected]> wrote:
> I don't like the idea of introducing tristate on start_vm. It's a very
> proeminent parameter, so it might cause some breakage.
>
> I'd feel more comfortable with introducing a parameter like
> pause_after_start_vm, that can assume values 'yes' or 'no', what do
> you think?
>
> Other than that, there's a small detail here:
>
> On Fri, Jul 27, 2012 at 3:03 AM, Yu Mingfei <[email protected]> wrote:
>> This patch adds paused state in vm's preprocess.
>>
>> Signed-off-by: Yu Mingfei <[email protected]>
>> ---
>>  client/virt/virt_env_process.py |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/client/virt/virt_env_process.py
>> b/client/virt/virt_env_process.py
>> index 306be76..0737e3d 100644
>> --- a/client/virt/virt_env_process.py
>> +++ b/client/virt/virt_env_process.py
>> @@ -80,7 +80,7 @@ def preprocess_vm(test, params, env, name):
>>          logging.debug("Param 'migration_mode' specified, starting VM in "
>>                        "incoming migration mode")
>>          start_vm = True
>> -    elif params.get("start_vm") == "yes":
>> +    elif params.get("start_vm") != "no":
>>          # need to deal with libvirt VM differently than qemu
>>          if vm_type == 'libvirt':
>>              if not vm.is_alive():
>> @@ -104,6 +104,8 @@ def preprocess_vm(test, params, env, name):
>>              vm.create(name, params, test.bindir,
>>                        migration_mode=params.get("migration_mode"),
>>                        migration_fd=params.get("migration_fd"))
>> +        if params.get("start_vm") == "paused":
>> +            vm.pause()
>
> In case this is a kvm vm, it's already supposed to be started up with
> -S, therefore, paused. It's better to check if the vm is not paused,
> then if it's not, pause it.
>
>>      else:
>>          # Don't start the VM, just update its params
>>          vm.params = params
>> --
>> 1.7.1
>>
>>
>> --
>> Best Regards
>> Yu Mingfei
>>
>> _______________________________________________
>> Autotest-kernel mailing list
>> [email protected]
>> https://www.redhat.com/mailman/listinfo/autotest-kernel
>
>
>
> --
> Lucas



-- 
Lucas

_______________________________________________
Autotest-kernel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/autotest-kernel

Reply via email to