Change in vdsm[master]: Configure iSCSI iface.net_ifacename

danken at redhat.com danken at redhat.com
Fri Sep 12 11:19:18 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: Configure iSCSI iface.net_ifacename
......................................................................


Patch Set 10: Code-Review-1

(4 comments)

http://gerrit.ovirt.org/#/c/31534/10/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 155: 
Line 156:     for iface in iscsi.iterateIscsiInterfaces():
Line 157:         if iface.name == ifaceName:
Line 158:             iface.netIfacename = netIfacename
Line 159:             iface.update()
I think that update() explodes with netIfacename==None; please verify.
Line 160:             return iface
Line 161: 
Line 162:     iface = iscsi.IscsiInterface(ifaceName, initiatorName=initiatorName,
Line 163:                                  netIfacename=netIfacename)


http://gerrit.ovirt.org/#/c/31534/10/vdsm/storage/iscsi.py
File vdsm/storage/iscsi.py:

Line 95:     return os.path.join("/sys", "class", "iscsi_connection",
Line 96:                         "connection%d:0" % sessionId)
Line 97: 
Line 98: 
Line 99: def getIscsiHostPath(sessionId):
Please add a docstring, specifying clearly that you return None in case sessionId's host is not found.
Line 100:     hosts = []
Line 101:     platform = os.path.join("/sys", "devices", "platform")
Line 102:     iscsi_host = os.path.join("/sys", "class", "iscsi_host")
Line 103: 


Line 104:     for subdir in os.listdir(platform):
Line 105:         if subdir.startswith("host"):
Line 106:             hosts.append(subdir)
Line 107: 
Line 108:     for host in hosts:
there's no need in the "hosts" list - just place  the following block under the

 if subdir.startswith("host"):

condition.
Line 109:         host_path = os.path.join(platform, "host%s" % host)
Line 110:         if "session%s" % sessionId in os.listdir(host_path):
Line 111:             return os.path.join(iscsi_host, host)
Line 112: 


Line 106:             hosts.append(subdir)
Line 107: 
Line 108:     for host in hosts:
Line 109:         host_path = os.path.join(platform, "host%s" % host)
Line 110:         if "session%s" % sessionId in os.listdir(host_path):
it would be quicker and cleaner to do

 if os.path.exists(os.path.join(host_path, "session%s" % sessionId))
Line 111:             return os.path.join(iscsi_host, host)
Line 112: 
Line 113: 
Line 114: def readSessionInfo(sessionID):


-- 
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: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim <apahim at redhat.com>
Gerrit-Reviewer: Amador Pahim <apahim at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipchuk at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list