Dan Kenigsberg has posted comments on this change.
Change subject: hsm: Check image alignment using virt-alignment-scan
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File vdsm/alignmentScan.py
Line 48: rc, out, err = runScanArgs('--add', image_path)
Line 49: if rc == 1:
Line 50: raise VirtAlignError("An error scanning the disk image "
Line 51: "or guest:\n%s" % err)
Line 52: elif rc not in [0, 2, 3]:
I actually found the original comments useful.
Line 53: raise VirtAlignError("Unexpected return code from "
Line 54: "virt-alignment-scan: %d" % rc)
Line 55: outList = []
Line 56: for line in out:
Line 53: raise VirtAlignError("Unexpected return code from "
Line 54: "virt-alignment-scan: %d" % rc)
Line 55: outList = []
Line 56: for line in out:
Line 57: outList.append(ScanOutput(*line.split(None, 3)))
here, too, I prefer to keep the nice comments, and particularly, the fact that
scan_result is a boolean.
....................................................
File vdsm/storage/hsm.py
Line 1715: :type spUUID: UUID
Line 1716: :param sdUUID: The UUID of the storage domain that contains
the image.
Line 1717: :type sdUUID: UUID
Line 1718: :param imgUUID: The UUID of the image.
Line 1719: :type imgUUID: UUID
could you please document the returned type, too.
Line 1720: """
Line 1721: img = self.prepareImage(sdUUID, spUUID, imgUUID)
Line 1722: try:
Line 1723: out = alignmentScan.scanImage(img['path'])
Line 1724: finally:
Line 1725: self.teardownImage(sdUUID, spUUID, imgUUID)
Line 1726:
Line 1727: aligning = {}
Line 1728: for line in out:
out is a namedtuple. let's use the field name.
Line 1729: aligning[out[line][0]] = (out[line][3] == "ok")
Line 1730: return aligning
Line 1731:
Line 1732: @public
--
To view, visit http://gerrit.ovirt.org/12003
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia925f5f138948acca623f6379b7b811474a43ffe
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
