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

nsoffer at redhat.com nsoffer at redhat.com
Wed Sep 17 19:25:39 UTC 2014


Nir Soffer has posted comments on this change.

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


Patch Set 13:

(3 comments)

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

Line 106:         - iSCSI host path - e.g. '/sys/class/iscsi_host/host17/'
Line 107:     """
Line 108: 
Line 109:     pattern = '/sys/devices/platform/host*/session%s' % sessionID
Line 110:     for path in glob.glob(pattern):
> better use iglob
It does not matter in this case, there will be only one value - otherwise this code is not correct.
Line 111:         host = os.path.basename(os.path.dirname(path))
Line 112:         return '/sys/class/iscsi_host/' + host
Line 113: 
Line 114:     raise OSError(errno.ENOENT, "No iscsi_host for session [%s]" % sessionID)


Line 152:         username = None
Line 153:     if password in ["<NULL>", "(null)"]:
Line 154:         password = None
Line 155: 
Line 156:     if netdev in ["<NULL>", "(null)"]:
> tuple instead of list ('<Null>', '(null)')
Better to keep this code as is in this patch, in another patch we can convert all uses of lists that can use tuples.

But basically it does not matter - there is no practical difference between using lists or tuples for this use case.
Line 157:         netdev = None
Line 158: 
Line 159:     iface = IscsiInterface(iface, netIfacename=netdev)
Line 160:     portal = IscsiPortal(ip, port)


Line 401:         self._conf = conf
Line 402: 
Line 403:     def __repr__(self):
Line 404:         return "<IscsiInterface name='%s' transport='%s' netIfacename='%s'>" \
Line 405:             % (self.name, self.transport, self.netIfacename)
> name is the iscsi iface name. Considering the configuration file, it is the
Considering the other names in the class and the names of the configuration values, netIfacename should be renamed to networkInterfaceName or netIfaceName.

I think this class should use exactly the same names used in the iscsi configuration file, since this it is used to query and modify this file, but this should be done in a separate patch.
Line 406: 
Line 407: 
Line 408: def iterateIscsiInterfaces():
Line 409:     names = iscsiadm.iface_list()


-- 
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: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim <apahim at redhat.com>
Gerrit-Reviewer: Amador Pahim <apahim at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap 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