Petr Horáček has posted comments on this change.

Change subject: ovs: flush IP config from attached ifaces
......................................................................


Patch Set 4:

(6 comments)

https://gerrit.ovirt.org/#/c/59958/4/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

PS4, Line 205: flush
> Better be consistent and use 'remove'.
I wanted to emphasize, that it is not gentle removal like in case of 
dhclient.stop() but flush of the whole static/dynamic ip config.


PS4, Line 205: nics
> but it is not only nics, perhaps ifaces will fit better here.
Done


Line 203: 
Line 204: 
Line 205: def _flush_ip_config_from_used_nics(netinfo_bonds, netinfo_nics, 
attrs):
Line 206: 
Line 207:     def flush_nic_ip(nic):
> It will be better moving it to the module scope as private, there is no nee
Done
Line 208:         nic_info = netinfo_nics[nic]
Line 209:         if nic_info['ipv4address']:
Line 210:             _flush_ip_config(nic, family=4)
Line 211:         if nic_info['ipv6address']:


PS4, Line 209:         if nic_info['ipv4address']:
             :             _flush_ip_config(nic, family=4)
             :         if nic_info['ipv6address']:
             :             _flush_ip_config(nic, family=6)
> Why not just flush all?
Done


Line 214:     nic = attrs.get('nic')
Line 215:     bond = attrs.get('bonding')
Line 216:     if nic:
Line 217:         flush_nic_ip(nic)
Line 218:     elif bond:
> If there is a bond and there are IP settings on its slaves, we have a corru
Before setupNetworks you have two nics, in first stage of setup you used them 
for a bond and used the bond in a network.

Now we want to flush initial ip config from nics, how is that corrupted?
Line 219:         bond_info = netinfo_bonds[bond]
Line 220:         nics = bond_info['slaves']
Line 221:         for nic in nics:
Line 222:             flush_nic_ip(nic)


https://gerrit.ovirt.org/#/c/59958/5/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

Line 226:     dhclient.kill(iface, family)  # kills dhclient and flushes IP
Line 227: 
Line 228: 
Line 229: def _set_dhcp_config(iface, attrs):
Line 230:     # TODO: DHCPv6
> What about static IP/s?
by # flushes IP i mean flushes both IP obtained from dhcp and static one.
Line 231:     blocking_dhcp = attrs.get('blockingdhcp', False)
Line 232:     duid_source = attrs.get('bonding') or attrs.get('nic')
Line 233: 
Line 234:     ipv4 = address.IPv4(*_ipv4_conf_params(attrs))


-- 
To view, visit https://gerrit.ovirt.org/59958
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I66e0be6ea8583fae1814bb8fc776dd243932b192
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phora...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Edward Haas <edwa...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phora...@redhat.com>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Reply via email to