Ayal Baron has posted comments on this change. Change subject: multipath.getMPDevNamesIter() return /dev/mapper. ......................................................................
Patch Set 1: (3 comments) .................................................... File vdsm/storage/devicemapper.py Line 30: DMPATH_PREFIX = "/dev/mapper/" Line 31: Line 32: Line 33: def getDmId(deviceMultipathName): Line 34: devlinkPath = os.path.join(DMPATH_PREFIX, deviceMultipathName) 1 Line 35: try: Line 36: devStat = os.stat(devlinkPath) Line 37: except OSError: Line 38: raise OSError(errno.ENODEV, "Could not find dm device named `%s`" % .................................................... File vdsm/storage/hsm.py Line 3113: Warning: Internal use only. Line 3114: """ Line 3115: supervdsm.getProxy().appropriateDevice(guid, thiefId) Line 3116: supervdsm.getProxy().udevTrigger(guid) Line 3117: devPath = os.path.join(devicemapper.DMPATH_PREFIX, guid) 2 Line 3118: utils.retry(partial(fileUtils.validateQemuReadable, devPath), Line 3119: expectedException=OSError, Line 3120: timeout=QEMU_READABLE_TIMEOUT) Line 3121: .................................................... File vdsm/storage/multipath.py Line 380: Line 381: Line 382: def getMPDevNamesIter(): Line 383: for _, name in getMPDevsIter(): Line 384: yield os.path.join(devicemapper.DMPATH_PREFIX, name) So? In this patch alone you have 3 places (I marked them in the patch) which call os.path.join(devicemapper.DMPATH_PREFIX, VARIABLE), I see no justification for code duplication here. Note that this module is *multipath.py* so regardless of lvm, this is and will remain multipath. Line 385: Line 386: Line 387: def getMPDevsIter(): Line 388: """ -- To view, visit http://gerrit.ovirt.org/17967 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ec54147205992ac130684d01e73cd7aceccad48 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo <ewars...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Eduardo <ewars...@redhat.com> Gerrit-Reviewer: Elad Ben Aharon <eladba1...@gmail.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches