Dan Kenigsberg has posted comments on this change. Change subject: Configure iSCSI iface.net_ifacename ......................................................................
Patch Set 12: (3 comments) http://gerrit.ovirt.org/#/c/31534/12/vdsm/storage/iscsi.py File vdsm/storage/iscsi.py: Line 87: def getSessionInfo(sessionID): Line 88: return supervdsm.getProxy().readSessionInfo(sessionID) Line 89: Line 90: Line 91: def getIscsiSessionPath(sessionId): on another patch: get a consistent spelling to sessionId/sessionID in this module. Line 92: return os.path.join("/sys", "class", "iscsi_session", Line 93: "session%d" % sessionId) Line 94: Line 95: Line 110: Line 111: pattern = os.path.join(_DEVPLATFORM_PATH, 'host*', 'session%s' % sessionID) Line 112: for path in glob.glob(pattern): Line 113: session_host = os.path.basename(os.path.split(path)[0]) Line 114: return os.path.join(_ISCSIHOST_PATH, session_host) > This would be more clear as: .. but please use glob.iglob to avoid possibly-long intermediate list. Line 115: Line 116: raise OSError(errno.ENOENT, "No iscsi_host for session [%s]" % sessionID) Line 117: Line 118: Line 348: initiatorName=None, netIfacename=None): Line 349: self._conf = {} Line 350: # Only new tcp interfaces are supported for now Line 351: self._conf['iface.transport_name'] = 'tcp' Line 352: self._conf['iface.net_ifacename'] = netIfacename or '' I don't understand this assignment. Does iscsiadm accepts setting iface.net_ifacename="" ? Even if so, it is very odd and may change in the future. It is much safer not to pass iface.net_ifacename if it was not requested by Engine. Line 353: Line 354: self._loaded = False Line 355: Line 356: self.name = name -- To view, visit http://gerrit.ovirt.org/31534 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3ebb2f272669b753700b57486d869b21dd16f2d6 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amador Pahim <apa...@redhat.com> Gerrit-Reviewer: Amador Pahim <apa...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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