Ayal Baron has posted comments on this change.

Change subject: More precise exception when multipath failed.
......................................................................


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

(2 inline comments)

....................................................
File vdsm/storage/devicemapper.py
Line 34:     devlinkPath = DMPATH_FORMAT % deviceMultipathName
Line 35:     try:
Line 36:         devStat = os.stat(devlinkPath)
Line 37:     except OSError as e:
Line 38:         raise OSError(e.errno, "%s: %s: %s" %
if you don't even keep the logic of raising ENODEV specifically (which was what 
you did do in the first patch set) then why catch here at all?
why is it suddenly ok to raise whatever is returned?
Line 39:                       (deviceMultipathName, e.strerror, e.filename))
Line 40:     else:
Line 41:         return "dm-%d" % os.minor(devStat.st_rdev)
Line 42: 


Line 75: 
Line 76: 
Line 77: def resolveDevName(devName):
Line 78:     try:
Line 79:         resolved = getSysfsPath(devName)
you're changing the interface here (returning /sys/block/devName instead of 
devName).  Why is that ok?
Line 80:     except OSError as e:
Line 81:         if e.errno == errno.ENOENT:
Line 82:             resolved = getDmId(devName)
Line 83:         else:


-- 
To view, visit http://gerrit.ovirt.org/17145
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1b87e8e91b838db2c8a98c9afbbc998e8f4c792a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Elad Ben Aharon <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to