Ido Barkan has posted comments on this change. Change subject: netinfo: make getNicsVlanAndBondingForNetwork more readable, drop asserts ......................................................................
Patch Set 5: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/45095/5/lib/vdsm/netinfo.py File lib/vdsm/netinfo.py: Line 916: Line 917: # If the network is bridged, it is assumed that at most one of its Line 918: # ports is connected to physical NIC(s) possibly through a VLAN and/or Line 919: # bonding device. Other ports (if present) are virtual NICs that belong Line 920: # to VMs (but they are also reported by the function). the fact that this function is reporting virtual (vnet) devices is just bad, and should be fixed in a later patch. Line 921: for device in ports: Line 922: vlan_info = self.vlans.get(device) Line 923: if vlan_info: Line 924: vlan = device Line 922: vlan_info = self.vlans.get(device) Line 923: if vlan_info: Line 924: vlan = device Line 925: vlan_id = vlan_info['vlanid'] Line 926: device = vlan_info['iface'] it still does the confusing thing of changing the iterator during iteration (now it is device, before it was port). Can you fix this? Line 927: bond_info = self.bondings.get(device) Line 928: if bond_info: Line 929: bond = device Line 930: nics += bond_info['slaves'] -- To view, visit https://gerrit.ovirt.org/45095 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8535dbd29314c97e4519b160a47c9ea6489b5a3 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
