Dan Kenigsberg has posted comments on this change. Change subject: Configure iSCSI iface.net_ifacename ......................................................................
Patch Set 13: Code-Review-1 (3 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't you use it here? 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. the bigger problem is the *wish* to update - I am not at all sure that _resolveIscsiIface() should ever modify and existing iscsi interface. iterateIscsiInterfaces() should yield existing interfaces with their complete info - can net_ifacename be extracted from iscsiadm? 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 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 this patch. it only confuses the reviewers. Line 350: Line 351: self._loaded = False Line 352: Line 353: 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amador Pahim <apa...@redhat.com> Gerrit-Reviewer: Amador Pahim <apa...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@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