Change in vdsm[master]: clientIF: Check image alignment using virt-alignment-scan

danken at redhat.com danken at redhat.com
Wed May 15 10:34:55 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: clientIF: Check image alignment using virt-alignment-scan
......................................................................


Patch Set 6: I would prefer that you didn't submit this

(2 inline comments)

....................................................
File vdsm/alignmentScan.py
Line 64:                         "virt-alignment-scan: %d" % rc)
Line 65:     outList = []
Line 66:     for line in out:
Line 67:         outList.append(ScanOutput(*line.split(None, 3)))
Line 68:     return outList
I really like the existing api of this module. Why do you keep changing it? A boolean scanResult is soon nice, please keep it.


....................................................
File vdsm/clientIF.py
Line 363:         volPath = self.prepareVolumePath(drive)
Line 364:         try:
Line 365:             out = alignmentScan.scanImage(volPath)
Line 366:         except alignmentScan.VirtAlignError as e:
Line 367:             self.log.warning("Error in virt-alignment-scan: %s", e)
Why are you swallowing the error here?
Line 368:         else:
Line 369:             for line in xrange(len(out)):
Line 370:                 aligning[out[line].partitionName] = (
Line 371:                     out[line].alignmentScanResult == "ok")


--
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list