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 <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Elad Ben Aharon <eladba1990(a)gmail.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server