Dan Kenigsberg has posted comments on this change.
Change subject: Added VDSM interface for the virt-alignment-scan tool from
libguestfs
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File tests/alignmentScan_test.py
Line 46: cmd = "/bin/dd if=/dev/zero of=%s bs=4K count=1K" % imagepath
I think I wrote it before: split is evil (think imagepath with a space!).
Use a list literal instead.
....................................................
File vdsm/alignmentScan.py
Line 32: if not os.path.exists(image_path):
please do not check path existence, this may hang on blocked nfs mounts. let
virt-alignment-scan explode on missing path.
Line 37: raise ValueError("An error scanning the disk image or
guest:\n%s" % err)
why is that a ValueError? I think a specific VirtAlignError is more fitting.
Line 45: # (allows easier iteration)
else:
missing, with another raise of VirtAlignError
Line 47: out_list = [ScanOutput(*line.split(None, 3)) for line in out]
some of these fields makes more sense if converted to int.
--
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saša Tomić <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <[email protected]>
Gerrit-Reviewer: Saša Tomić <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/vdsm-patches
