On Tue, Apr 13, 2010 at 1:24 AM, pradeep <psuri...@linux.vnet.ibm.com> wrote:
>
> Lucas
>
> Any comments??

No pradeep, Sudhir's comments are very reasonable, please implement
the changes he suggested and re-send the patch, I am going to do some
testing on it on my local systems and then commit it upstream when
done with testing.

>
> --sp
>
> pradeep wrote:
>
>> sudhir kumar wrote:
>>
>>
>>> On Fri, Apr 9, 2010 at 2:40 PM, pradeep <psuri...@linux.vnet.ibm.com> wrote:
>>>
>>>
>>>> Hi Lucas
>>>>
>>>> Thanks for your comments.
>>>> Please find the patch, with suggested changes.
>>>>
>>>> Thanks
>>>> Pradeep
>>>>
>>>>
>>>>
>>>> Signed-off-by: Pradeep Kumar Surisetty <psuri...@linux.vnet.ibm.com>
>>>> ---
>>>> diff -uprN autotest-old/client/tests/kvm/tests/balloon_check.py
>>>> autotest/client/tests/kvm/tests/balloon_check.py
>>>> --- autotest-old/client/tests/kvm/tests/balloon_check.py        1969-12-31
>>>> 19:00:00.000000000 -0500
>>>> +++ autotest/client/tests/kvm/tests/balloon_check.py    2010-04-09
>>>> 12:33:34.000000000 -0400
>>>> @@ -0,0 +1,47 @@
>>>> +import re, string, logging, random, time
>>>> +from autotest_lib.client.common_lib import error
>>>> +import kvm_test_utils, kvm_utils
>>>> +
>>>> +def run_balloon_check(test, params, env):
>>>> +    """
>>>> +    Check Memory ballooning:
>>>> +    1) Boot a guest
>>>> +    2) Increase and decrease the memory of guest using balloon command 
>>>> from
>>>> monitor
>>>>
>>>>
>>> Better replace this description by "Change the guest memory between X
>>> and Y values"
>>> Also instead of using 0.6 and 0.95 below, better use two variables and
>>> take their value from config file. This will give the user a
>>> flexibility to narrow or widen the ballooning range.
>>>
>>>
>> Thanks for your suggestions. I dont  think, user should need flexibility
>> here.
>> If ballooning doest work for one set of value, it will not work for any
>> other.
>> And here, we are choosing between 60 to 95% of actual values, which is
>> reasonable.
>>
>>
>>>
>>>
>>>> +    3) check memory info
>>>> +
>>>> +   �...@param test: kvm test object
>>>> +   �...@param params: Dictionary with the test parameters
>>>> +   �...@param env: Dictionary with test environment.
>>>> +    """
>>>> +
>>>> +    vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
>>>> +    session = kvm_test_utils.wait_for_login(vm)
>>>> +    fail = 0
>>>> +
>>>> +    # Check memory size
>>>> +    logging.info("Memory size check")
>>>> +    expected_mem = int(params.get("mem"))
>>>> +    actual_mem = vm.get_memory_size()
>>>> +    if actual_mem != expected_mem:
>>>> +        logging.error("Memory size mismatch:")
>>>> +        logging.error("Assigned to VM: %s" % expected_mem)
>>>> +        logging.error("Reported by OS: %s" % actual_mem)
>>>> +
>>>> +    #change memory to random size between 60% to 95% of actual memory
>>>> +    percent = random.uniform(0.6, 0.95)
>>>> +    new_mem = int(percent*expected_mem)
>>>> +    vm.send_monitor_cmd("balloon %s" %new_mem)
>>>>
>>>>
>>> You may want to check if the command passed/failed. Older versions
>>> might not support ballooning.
>>>
>>>
>>
>> sure,  i will make changes here.
>>
>>>
>>>
>>>> +    time.sleep(20)
>>>>
>>>>
>>> why 20 second sleep and why the magic number?
>>>
>>>
>> As soon as balloon command is passed, it takes some time for memory
>> ballooing.
>> If we check "info balloon" as soon as we run ballooning, it will give
>> weird values.
>> I just choose it as, 20sec and its not huge time from testing perspective.
>>
>>>
>>>
>>>> +    status, output = vm.send_monitor_cmd("info balloon")
>>>>
>>>>
>>> You might want to put this check before changing the memory.
>>>
>>>
>>>
>> sure, will make changes here too.
>>
>>>> +    if status != 0:
>>>> +        logging.error("qemu monitor command failed: info balloon")
>>>> +
>>>> +    balloon_cmd_mem = int(re.findall("\d+",output)[0])
>>>>
>>>>
>>> A better variable name I can think of is "ballooned_mem"
>>>
>>>
>>
>> will  change it...
>>
>>>
>>>
>>>> +    if balloon_cmd_mem != new_mem:
>>>> +        logging.error("memory ballooning failed while changing memory to
>>>> %s" %balloon_cmd_mem)
>>>> +       fail += 1
>>>> +
>>>> +    #Checking for test result
>>>> +    if fail != 0:
>>>>
>>>>
>>> In case you are running multiple iterations and the 2nd iteration
>>> fails you will always miss this condition.
>>>
>>>
>>
>>
>>>
>>>
>>>> +        raise error.TestFail("Memory ballooning test failed ")
>>>> +    session.close()
>>>> diff -uprN autotest-old/client/tests/kvm/tests_base.cfg.sample
>>>> autotest/client/tests/kvm/tests_base.cfg.sample
>>>> --- autotest-old/client/tests/kvm/tests_base.cfg.sample 2010-04-09
>>>> 12:32:50.000000000 -0400
>>>> +++ autotest/client/tests/kvm/tests_base.cfg.sample     2010-04-09
>>>> 12:53:27.000000000 -0400
>>>> @@ -185,6 +185,10 @@ variants:
>>>>                 drift_threshold = 10
>>>>                 drift_threshold_single = 3
>>>>
>>>> +    - balloon_check:  install setup unattended_install boot
>>>> +        type = balloon_check
>>>> +        extra_params += " -balloon virtio"
>>>> +
>>>>     - stress_boot:  install setup unattended_install
>>>>         type = stress_boot
>>>>         max_vms = 5
>>>> ---
>>>>
>>>>
>>> Rest all looks good!!!!
>>>
>>>
>>>> _______________________________________________
>>>> Autotest mailing list
>>>> autot...@test.kernel.org
>>>> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>> _______________________________________________
>> Autotest mailing list
>> autot...@test.kernel.org
>> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>>
>
> _______________________________________________
> Autotest mailing list
> autot...@test.kernel.org
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>



-- 
Lucas
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to