Change in vdsm[master]: Configure iSCSI iface.net_ifacename
apahim at redhat.com
apahim at redhat.com
Thu Sep 18 17:09:37 UTC 2014
Amador Pahim has posted comments on this change.
Change subject: Configure iSCSI iface.net_ifacename
......................................................................
Patch Set 13:
(6 comments)
http://gerrit.ovirt.org/#/c/31534/13/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 149:
Line 150: def _resolveIscsiIface(ifaceName, initiatorName, netIfacename):
Line 151: if not ifaceName:
Line 152: iface = iscsi.IscsiInterface('default')
Line 153: iface.netIfacename = netIfacename
> in iscsi.py you modified the initiator to accept netIfacename. so why don'
"default" means not using iscsi bond feature. So, no netIfacename will arrive from engine in this case.
Line 154: return iface
Line 155:
Line 156: for iface in iscsi.iterateIscsiInterfaces():
Line 157: if iface.name == ifaceName:
Line 156: for iface in iscsi.iterateIscsiInterfaces():
Line 157: if iface.name == ifaceName:
Line 158: iface.netIfacename = netIfacename
Line 159: if netIfacename:
Line 160: iface.update()
> this is a no-op, since nothing has change iface._conf.
I'm trying here to existing iface with modified netIfacename will be updated. But, yes, you're right. I will use a better approach.
Line 161: return iface
Line 162:
Line 163: iface = iscsi.IscsiInterface(ifaceName, initiatorName=initiatorName,
Line 164: netIfacename=netIfacename)
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):
> It does not matter in this case, there will be only one value - otherwise t
I will use iglob anyway. I've got 2 requests for it and I do like to make the reviewers happy.
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)"]:
> Better to keep this code as is in this patch, in another patch we can conve
Ok, keeping for now.
Line 157: netdev = None
Line 158:
Line 159: iface = IscsiInterface(iface, netIfacename=netdev)
Line 160: portal = IscsiPortal(ip, port)
Line 345: def __init__(self, name, hardwareAddress=None, ipAddress=None,
Line 346: initiatorName=None, netIfacename=None):
Line 347: self._conf = {}
Line 348: # Only new tcp interfaces are supported for now
Line 349: self._conf['iface.transport_name'] = 'tcp'
> there is no functional change in these 3 lines - no need to touch them in t
Done
Line 350:
Line 351: self._loaded = False
Line 352:
Line 353: self.name = name
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)
> Considering the other names in the class and the names of the configuration
Done
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