Change in vdsm[master]: Sysctl to allow iSCSI multipath with multiple NICs in the sa...

apahim at redhat.com apahim at redhat.com
Wed Nov 5 02:55:14 UTC 2014


Amador Pahim has posted comments on this change.

Change subject: Sysctl to allow iSCSI multipath with multiple NICs in the same subnet
......................................................................


Patch Set 18:

(7 comments)

http://gerrit.ovirt.org/#/c/31529/18/vdsm/storage/iscsi.py
File vdsm/storage/iscsi.py:

Line 185:                     key = "node.session." + key
Line 186:                     iscsiadm.node_update(iface.name, portalStr, target.iqn,
Line 187:                                          key, value, hideValue=True)
Line 188: 
Line 189:             setDevLooseRpFilter(iface.netIfaceName, target.portal.hostname)
> Lets simplify and send the session to this function:
I don't have a session here, before creating the session. We can simplify with:

   setDevLooseRpFilter(iface, target)

But that's not what the to next two lines are doing. 

Done.
Line 190: 
Line 191:             iscsiadm.node_login(iface.name, portalStr, target.iqn)
Line 192: 
Line 193:             iscsiadm.node_update(iface.name, portalStr, target.iqn,


Line 535:     try:
Line 536:         iscsiadm.session_logout(sessionID)
Line 537:     except iscsiadm.IscsiError as e:
Line 538:         return e[0]
Line 539: 
> and simply by sending sessionInfo - the function will get what it needs fro
Done
Line 540:     setDevStrictRpFilter(netIfaceName, hostname)
Line 541: 
Line 542:     return 0
Line 543: 


Line 548:     return [session for session in iterateIscsiSessions()
Line 549:             if session.iface.netIfaceName == netIfaceName]
Line 550: 
Line 551: 
Line 552: def setDevLooseRpFilter(netIfaceName, hostname):
> This is name is misleading - this will set loose rp filter only if needed.
Done
Line 553:     """
Line 554:     Set rp_filter to loose mode if the device in use by the iSCSI session is
Line 555:     not the device used by OS to reach the target IP.
Line 556: 


Line 557:     This is needed to allow multiple iSCSI connections in a multiple NIC per
Line 558:     subnet configuration.
Line 559: 
Line 560:     Arguments:
Line 561:         netIfaceName
> hostname missing
Done
Line 562:     """
Line 563: 
Line 564:     if netIfaceName != getRouteDeviceTo(hostname):
Line 565:         supervdsm.getProxy().set_rp_filter_loose(netIfaceName)


Line 563: 
Line 564:     if netIfaceName != getRouteDeviceTo(hostname):
Line 565:         supervdsm.getProxy().set_rp_filter_loose(netIfaceName)
Line 566:         log.debug("Setting loose mode rp_filter for device [%s]." %
Line 567:                   netIfaceName)
> Should be log.info - we made a significant change in the machine configurat
Done
Line 568: 
Line 569: 
Line 570: def setDevStrictRpFilter(netIfaceName, hostname):
Line 571:     """


Line 576:     This is needed to avoid the security breach where an untrusted VM can DoS
Line 577:     the host by sending it packets with spoofed random sources.
Line 578: 
Line 579:     Arguments:
Line 580:         sessionID
> Please update to reflect reality
Done
Line 581:     """
Line 582: 
Line 583:     sessions = findSessions(netIfaceName)
Line 584: 


Line 584: 
Line 585:     if netIfaceName != getRouteDeviceTo(hostname) and len(sessions) == 0:
Line 586:         supervdsm.getProxy().set_rp_filter_strict(netIfaceName)
Line 587:         log.debug("Setting strict mode rp_filter for device [%s]." %
Line 588:                   netIfaceName)
> Should be log.info - we made a significant change in the machine configurat
Done


-- 
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: 18
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: Yoav Kleinberger <ykleinbe at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: jian wang <wjian84 at gmail.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list