Change in vdsm[master]: Sysctl to allow iSCSI multipath with multiple NICs in the sa...
danken at redhat.com
danken at redhat.com
Fri Sep 12 09:37:05 UTC 2014
Dan Kenigsberg has posted comments on this change.
Change subject: Sysctl to allow iSCSI multipath with multiple NICs in the same subnet
......................................................................
Patch Set 12: Code-Review-1
(2 comments)
This patch has a nasty side effect - it loosens rp-filter even for iscsi sessions that does not need it, which may be insecure if netInterface is a bridge.
How are we going to block/warn about this?
http://gerrit.ovirt.org/#/c/31529/12/vdsm/storage/iscsi.py
File vdsm/storage/iscsi.py:
Line 187: iscsiadm.node_update(iface.name, portalStr, target.iqn,
Line 188: key, value, hideValue=True)
Line 189:
Line 190: if iface.netIfacename:
Line 191: setIscsiIfaceSysctlRpfilter(iface.netIfacename, '2')
Please define a CONSTANT. '2' is not very clear.
Line 192:
Line 193: iscsiadm.node_login(iface.name, portalStr, target.iqn)
Line 194:
Line 195: iscsiadm.node_update(iface.name, portalStr, target.iqn,
Line 535: # FIXME : Should throw exception on error
Line 536: sessionID = int(sessionID)
Line 537: sessionInfo = getSessionInfo(sessionID)
Line 538:
Line 539: if sessionInfo.iface.netIfacename:
That's not good enough. You should iterate through all other sessions, and only if none of them is using the netIface, reset its value. A single network interface may be used by multiple sessions.
Here, too, a
_SYSCTL_RPFILTER_STRICT = '0'
would make things more readable.
Line 540: setIscsiIfaceSysctlRpfilter(sessionInfo.iface.netIfacename, '0')
Line 541:
Line 542: try:
Line 543: iscsiadm.session_logout(sessionID)
--
To view, visit http://gerrit.ovirt.org/31529
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf93d49317c76aece764e53e58e0ff28868f16b0
Gerrit-PatchSet: 12
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: 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