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@gmail.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saša Tomić tomic80@gmail.com