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 <[email protected]> Gerrit-Reviewer: Amador Pahim <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
