Amador Pahim has posted comments on this change.

Change subject: Configure iSCSI iface.net_ifacename
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.ovirt.org/#/c/31534/3/vdsm/storage/iscsi.py
File vdsm/storage/iscsi.py:

Line 324:         self._conf = {}
Line 325:         self.name = name
Line 326: 
Line 327:         ni = netinfo.NetInfo()
Line 328:         network = tuple(ni.getBridgedNetworksAndVlansForIface(name))
> This returns multiple of networks (and vlans) that use the "name" interface
> This returns multiple of networks (and vlans) that use the "name" interface.

Returns (network,None) OR (network,vlan), only if device is bridged. "network" 
is the bridge name. As one device cannot be attached to more than one bridge at 
same time, seems safe to assume the first element will be the bridge name, when 
the bridge exists.

> Why do you take only the (random) first one?

It's not random... The first element, if exists, is the bridge name:

  870     def getBridgedNetworksAndVlansForIface(self, iface):$                 
      
  871         """ Returns tuples of (bridge, vlan) connected to nic/bond """$   
      
  872         for network, netdict in self.networks.iteritems():$               
      
  873             if netdict['bridged']:$                                       
      
  874                 for interface in netdict['ports']:$                       
      
  875                     if iface == interface:$                               
      
  876                         yield (network, None)$                            
      
  877                     elif interface.startswith(iface + '.'):$              
      
  878                         yield (network, interface.split('.', 1)[1])$      
      

> What is passed from Engine when iscsi is to use a vlan device?

The vlan device as "name". e.g. "eth1.10" is $name.

> If the storage network is a non-VM network, but bridged networks exist over 
> the interface, this function would not return the storage network at all.

If it's bridged, this function will use the bridge to connect to the storage. 
Isn't that what we are supposed to do?

This is what happens with this code applied, considering eth1:

  - Network "SAN1" (NonVM, Non-VLAN):
  iscsiadm -m iface -I eth1 -n iface.net_ifacename -v eth1 --op=update

  - Network "SAN1" (NonVM, VLAN 10):
  iscsiadm -m iface -I eth1.10 -n iface.net_ifacename -v eth1.10 --op=update

  - Network "SAN1" (VM, Non-VLAN):
  iscsiadm -m iface -I eth1 -n iface.net_ifacename -v SAN1 --op=update

  - Network "SAN1" (VM, VLAN 10):
  iscsiadm -m iface -I eth1.10 -n iface.net_ifacename -v SAN1 --op=update

  - Network "SAN1" (NonVM, VLAN 10) AND Network "SAN2" (NonVM, VLAN 11) :
  iscsiadm -m iface -I eth1.10 -n iface.net_ifacename -v eth1.10 --op=update
  iscsiadm -m iface -I eth1.11 -n iface.net_ifacename -v eth1.11 --op=update

  - Network "SAN1" (VM, VLAN 10) AND Network "SAN2" (VM, VLAN 11) :
  iscsiadm -m iface -I eth1.10 -n iface.net_ifacename -v SAN1 --op=update
  iscsiadm -m iface -I eth1.11 -n iface.net_ifacename -v SAN2 --op=update

  - Network "SAN1" (VM, VLAN 10) AND Network "SAN2" (NonVM, VLAN 11) :
  iscsiadm -m iface -I eth1.10 -n iface.net_ifacename -v SAN1 --op=update
  iscsiadm -m iface -I eth1.11 -n iface.net_ifacename -v eth1.11 --op=update


Do you see any corner case?

Tks.
Line 329: 
Line 330:         if network:
Line 331:             self._conf['iface.net_ifacename'] = network[0][0]
Line 332:         else:


-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim <[email protected]>
Gerrit-Reviewer: Amador Pahim <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[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

Reply via email to