KSM overcommit test review - 1st pass (issue183068)

Hi Jiri, thank you very much for your work! I have comments to make regarding to coding style as a first pass review. While reading them, please keep in mind autotest's coding standards:

http://autotest.kernel.org/browser/trunk/CODING_STYLE

Also, I have noticed lots of trailing spaces on lines. I recommend that every time you are done working with the code, you use the very useful script utils/reindent.py (see top level autotest directory) that will remove all trailing spaces on your code. While I wrote this first pass review, I have applied it on the resultant files.

http://codereview.appspot.com/183068

Here is the full text of the review

http://codereview.appspot.com/183068/diff/1/3
File client/tests/kvm/tests/ksm_overcommit.py (right):

http://codereview.appspot.com/183068/diff/1/3#newcode5
client/tests/kvm/tests/ksm_overcommit.py:5: import random, string, math, os Here we have the import of standard modules after the autotest and custom kvm ones. All standard module imports should be grouped together.

http://codereview.appspot.com/183068/diff/1/3#newcode19
client/tests/kvm/tests/ksm_overcommit.py:19: def parse_meminfo(rowName):
There are functions on client/bin/base_utils.py that already parse meminfo, we should consider using them instead of custom functions.

http://codereview.appspot.com/183068/diff/1/3#newcode29
client/tests/kvm/tests/ksm_overcommit.py:29:
Please don't use a single empty space between functions, as the autotest coding standard asks for 2:

http://autotest.kernel.org/browser/trunk/CODING_STYLE

I will omit the comment for further occurrences of single line spacing to avoid cluttering the review, but please fix them all.

http://codereview.appspot.com/183068/diff/1/3#newcode42
client/tests/kvm/tests/ksm_overcommit.py:42: "VM: %s" % vm.name) Apparently a raise was forgotten on the line above. Also, the best way to do multi-line statements is using implicit continuation in parenthesis, in general you should refrain from using \ to continue sentences, ie:

Instead of

print "Fancy multi-line statement"+\
"that should be avoided."

You should do

print ("Fancy multi-line statement"
"the preferred way.")

I will omit the comment for further occurrences of ending lines with slash to avoid cluttering the review, but please fix them all.

http://codereview.appspot.com/183068/diff/1/3#newcode88
client/tests/kvm/tests/ksm_overcommit.py:88:
Mixing up the body of the main function with declaration of the auxiliary functions didn't work up well. Please make a clear separation between the body of the function run_ksm_overcommit and the declaration of auxiliary functions.

http://codereview.appspot.com/183068/diff/1/3#newcode600
client/tests/kvm/tests/ksm_overcommit.py:600:
Please don't leave up 4 lines of spacing on the code.

http://codereview.appspot.com/183068/diff/1/2
File client/tests/kvm/unattended/allocator.py (right):

http://codereview.appspot.com/183068/diff/1/2#newcode11
client/tests/kvm/unattended/allocator.py:11:
1) The above imports should be done according to the autotest coding standard, grouping them into a single line.

2) datetime.timedelta is not being used on the below code, as far as I can see

3) Instead of using datetime.now() use datetime.datetime.now(), I know it's not exactly pretty but it's the preferred way when writing autotest code.

http://codereview.appspot.com/183068/diff/1/2#newcode13
client/tests/kvm/unattended/allocator.py:13: KVM test definitions. This summary of the program does not inform what the program is up for. I would suggest something along the lines "Auxiliary script used for memory allocation on guests"

http://codereview.appspot.com/183068/diff/1/2#newcode16
client/tests/kvm/unattended/allocator.py:16: Jiri Zupka <[email protected]> Use @author, and your e-mail in parenthesis
@author Jiri Zupka ([email protected])

http://codereview.appspot.com/183068/diff/1/2#newcode205
client/tests/kvm/unattended/allocator.py:205: print "PASS: Start" Why is that print statement for? It seems out of place

Lucas