Saša Tomić has posted comments on this change.
Change subject: Added VDSM interface for the virt-alignment-scan tool from libguestfs
......................................................................
Patch Set 1: (9 inline comments)
I updated the source code in my branch. Should I:
(1) submit it as another commit + git-review, OR
(2) do a git rebase, and merge the changes with the previous commit?
....................................................
Commit Message
Line 16: Change-Id: I2d2c146a0fa3101317720f2e9c373e0d21b5cfdf
Done
....................................................
File vdsm/alignmentScan.py
Line 25: def runScanArgs(args):
It's a DRY function, it's useful for testing, and can also be useful if we have
more than one calling function. For example, beside scanImage we could have scanURI.
Line 31: image_path = image_path.strip()
Done
Line 33: raise ValueError("Image path does not exist")
Running the tool takes several minutes on my machine, and it *must* fail if the image does
not exist. This is simply an optimization.
Line 35: retcode, out, err = runScanArgs(args)
I do some checking of the return code, but I raise an exception only if the checking
failed. Please verify that this is OK.
Line 42: return {'status':{ 'message':out_list,
'code':1},
Done
....................................................
File vdsm/alignmentScan_test.py
Line 1: #!/usr/bin/python
The problem with this unit test is that it takes too long, 2-3 (or more) minutes to
finish. It's the virt-alignment-scan that is so slow. I was actually considering that,
but I gave up since testing is done during every rpm build.
It would be good to have "long" and "medium" and "quick"
tests in the project.
Line 46: execCmd(cmd.split())
I agree in general. However, I control the paths here. If you believe other implementation
would be better here, please tell me.
Line 50: img = "/tmp/aligncheck_bad.img"
Done
--
To view, visit
http://gerrit.ovirt.org/2916
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d2c146a0fa3101317720f2e9c373e0d21b5cfdf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saša Tomić <tomic80(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Saša Tomić <tomic80(a)gmail.com>