Re: [Autotest] [PATCH 12/19] KVM test: Add new module kvm_test_utils.py

Re: [Autotest] [PATCH 12/19] KVM test: Add new module kvm_test_utils.py

[Michael Goldish]

----- "Uri Lublin" <uril@redhat.com> wrote:

> On 09/14/2009 08:26 AM, Yolkfull Chow wrote:
> > On Wed, Sep 09, 2009 at 09:12:05PM +0300, Michael Goldish wrote:
> >> This module is meant to reduce code size by performing common test
> procedures.
> >> Generally, code here should look like test code.
> 
> >> +def wait_for_login(vm, nic_index=0, timeout=240):
> >> +    """
> >> +    Try logging into a VM repeatedly.  Stop on success or when
> timeout expires.
> >> +
> >> +    @param vm: VM object.
> >> +    @param nic_index: Index of NIC to access in the VM.
> >> +    @param timeout: Time to wait before giving up.
> >> +    @return: A shell session object.
> >> +    """
> >> +    logging.info("Waiting for guest to be up...")
> >> +    session = kvm_utils.wait_for(lambda:
> vm.remote_login(nic_index=nic_index),
> >> +                                 timeout, 0, 2)
> >> +    if not session:
> >> +        raise error.TestFail("Could not log into guest")
> >
> > Hi Michael, I think we should also add a parameter 'vm_name' for
> > wait_for_login(). On the assumption that we boot more than one VMs,
> it's
> > hard to know which guest failed to login according to message
> above.
> > What do you think? :-)
> >
> 
> The VM object ("vm" parameter) "knows" its own name.
> It is a good idea to add that name to log/error messages, since we do
> want to 
> run different tests (VMs) in parallel (although the logs should be
> also saved in 
> different directories/files).
> 
> Regards,
>      Uri.

Currently it would be useful for tests that use multiple VMs, not for
multiple tests running in parallel, because all tests call their main VM
'vm1'.  That can be changed though.  In any case I agree that it's a
good idea.

Re: [Autotest] [PATCH 12/19] KVM test: Add new module kvm_test_utils.py

[Yolkfull Chow]

On Mon, Sep 14, 2009 at 10:58:01AM +0300, Uri Lublin wrote:
> On 09/14/2009 08:26 AM, Yolkfull Chow wrote:
>> On Wed, Sep 09, 2009 at 09:12:05PM +0300, Michael Goldish wrote:
>>> This module is meant to reduce code size by performing common test procedures.
>>> Generally, code here should look like test code.
>
>>> +def wait_for_login(vm, nic_index=0, timeout=240):
>>> +    """
>>> +    Try logging into a VM repeatedly.  Stop on success or when timeout expires.
>>> +
>>> +    @param vm: VM object.
>>> +    @param nic_index: Index of NIC to access in the VM.
>>> +    @param timeout: Time to wait before giving up.
>>> +    @return: A shell session object.
>>> +    """
>>> +    logging.info("Waiting for guest to be up...")
>>> +    session = kvm_utils.wait_for(lambda: vm.remote_login(nic_index=nic_index),
>>> +                                 timeout, 0, 2)
>>> +    if not session:
>>> +        raise error.TestFail("Could not log into guest")
>>
>> Hi Michael, I think we should also add a parameter 'vm_name' for
>> wait_for_login(). On the assumption that we boot more than one VMs, it's
>> hard to know which guest failed to login according to message above.
>> What do you think? :-)
>>
>
> The VM object ("vm" parameter) "knows" its own name.
> It is a good idea to add that name to log/error messages, since we do 
> want to run different tests (VMs) in parallel (although the logs should 
> be also saved in different directories/files).
>

Yes, I did ignore that we could use 'vm.name'
instead of adding a parameter for wait_for_login(). Therefore those
log/error messages could be added something like this:

if not session:
    raise error.TestFail("Could not log into guest %s" vm.name)

Thanks for pointing out that. :-)

> Regards,
>     Uri.

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

Re: [Autotest] [PATCH 12/19] KVM test: Add new module kvm_test_utils.py

[Uri Lublin]

On 09/14/2009 08:26 AM, Yolkfull Chow wrote:
> On Wed, Sep 09, 2009 at 09:12:05PM +0300, Michael Goldish wrote:
>> This module is meant to reduce code size by performing common test procedures.
>> Generally, code here should look like test code.

>> +def wait_for_login(vm, nic_index=0, timeout=240):
>> +    """
>> +    Try logging into a VM repeatedly.  Stop on success or when timeout expires.
>> +
>> +    @param vm: VM object.
>> +    @param nic_index: Index of NIC to access in the VM.
>> +    @param timeout: Time to wait before giving up.
>> +    @return: A shell session object.
>> +    """
>> +    logging.info("Waiting for guest to be up...")
>> +    session = kvm_utils.wait_for(lambda: vm.remote_login(nic_index=nic_index),
>> +                                 timeout, 0, 2)
>> +    if not session:
>> +        raise error.TestFail("Could not log into guest")
>
> Hi Michael, I think we should also add a parameter 'vm_name' for
> wait_for_login(). On the assumption that we boot more than one VMs, it's
> hard to know which guest failed to login according to message above.
> What do you think? :-)
>

The VM object ("vm" parameter) "knows" its own name.
It is a good idea to add that name to log/error messages, since we do want to 
run different tests (VMs) in parallel (although the logs should be also saved in 
different directories/files).

Regards,
     Uri.

Re: [Autotest] [PATCH 12/19] KVM test: Add new module kvm_test_utils.py

[Yolkfull Chow]

On Wed, Sep 09, 2009 at 09:12:05PM +0300, Michael Goldish wrote:
> This module is meant to reduce code size by performing common test procedures.
> Generally, code here should look like test code.
> More specifically:
>     - Functions in this module should raise exceptions if things go wrong
>       (unlike functions in kvm_utils.py and kvm_vm.py which report failure via
>       their returned values).
>     - Functions in this module may use logging.info(), in addition to
>       logging.debug() and logging.error(), to log messages the user may be
>       interested in (unlike kvm_utils.py and kvm_vm.py which use
>       logging.debug() for everything that isn't an error).
>     - Functions in this module typically use functions and classes from
>       lower-level modules (e.g. kvm_utils.py, kvm_vm.py, kvm_subprocess.py).
>     - Functions in this module should not be used by lower-level modules.
>     - Functions in this module should be used in the right context.
>       For example, a function should not be used where it may display
>       misleading or inaccurate info or debug messages.
> 
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm/kvm_test_utils.py |   61 ++++++++++++++++++++++++++++++++++++
>  1 files changed, 61 insertions(+), 0 deletions(-)
>  create mode 100644 client/tests/kvm/kvm_test_utils.py
> 
> diff --git a/client/tests/kvm/kvm_test_utils.py b/client/tests/kvm/kvm_test_utils.py
> new file mode 100644
> index 0000000..39e92b9
> --- /dev/null
> +++ b/client/tests/kvm/kvm_test_utils.py
> @@ -0,0 +1,61 @@
> +import time, os, logging, re, commands
> +from autotest_lib.client.common_lib import utils, error
> +import kvm_utils, kvm_vm, kvm_subprocess
> +
> +"""
> +High-level KVM test utility functions.
> +
> +This module is meant to reduce code size by performing common test procedures.
> +Generally, code here should look like test code.
> +More specifically:
> +    - Functions in this module should raise exceptions if things go wrong
> +      (unlike functions in kvm_utils.py and kvm_vm.py which report failure via
> +      their returned values).
> +    - Functions in this module may use logging.info(), in addition to
> +      logging.debug() and logging.error(), to log messages the user may be
> +      interested in (unlike kvm_utils.py and kvm_vm.py which use
> +      logging.debug() for anything that isn't an error).
> +    - Functions in this module typically use functions and classes from
> +      lower-level modules (e.g. kvm_utils.py, kvm_vm.py, kvm_subprocess.py).
> +    - Functions in this module should not be used by lower-level modules.
> +    - Functions in this module should be used in the right context.
> +      For example, a function should not be used where it may display
> +      misleading or inaccurate info or debug messages.
> +
> +@copyright: 2008-2009 Red Hat Inc.
> +"""
> +
> +
> +def get_living_vm(env, vm_name):
> +    """
> +    Get a VM object from the environment and make sure it's alive.
> +
> +    @param env: Dictionary with test environment.
> +    @param vm_name: Name of the desired VM object.
> +    @return: A VM object.
> +    """
> +    vm = kvm_utils.env_get_vm(env, vm_name)
> +    if not vm:
> +        raise error.TestError("VM '%s' not found in environment" % vm_name)
> +    if not vm.is_alive():
> +        raise error.TestError("VM '%s' seems to be dead; test requires a "
> +                              "living VM" % vm_name)
> +    return vm
> +
> +
> +def wait_for_login(vm, nic_index=0, timeout=240):
> +    """
> +    Try logging into a VM repeatedly.  Stop on success or when timeout expires.
> +
> +    @param vm: VM object.
> +    @param nic_index: Index of NIC to access in the VM.
> +    @param timeout: Time to wait before giving up.
> +    @return: A shell session object.
> +    """
> +    logging.info("Waiting for guest to be up...")
> +    session = kvm_utils.wait_for(lambda: vm.remote_login(nic_index=nic_index),
> +                                 timeout, 0, 2)
> +    if not session:
> +        raise error.TestFail("Could not log into guest")

Hi Michael, I think we should also add a parameter 'vm_name' for
wait_for_login(). On the assumption that we boot more than one VMs, it's
hard to know which guest failed to login according to message above.
What do you think? :-)

> +    logging.info("Logged in")
> +    return session
> -- 
> 1.5.4.1
> 
> _______________________________________________
> Autotest mailing list
> Autotest@test.kernel.org
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest