Change in vdsm[master]: net: ovs: remove initial nic ip, dont touch unchanged networks
phoracek at redhat.com
phoracek at redhat.com
Thu Oct 29 12:18:37 UTC 2015
Petr Horáček has posted comments on this change.
Change subject: net: ovs: remove initial nic ip, dont touch unchanged networks
......................................................................
Patch Set 6:
(9 comments)
https://gerrit.ovirt.org/#/c/46692/6/vdsm_hooks/ovs/ovs_before_network_setup_ip.py
File vdsm_hooks/ovs/ovs_before_network_setup_ip.py:
Line 89: _run_dhclient(iface, blockingdhcp, ipv6.defaultRoute, 6)
Line 90: iproute2._addSourceRoute(net_dev)
Line 91:
Line 92:
Line 93: def _remove_ip_config(iface, ipv4, ipv6, vdsm=True):
> such an awful arg name begs for a docstring
i forgot to remove this one, not needed
Line 94: net_dev = NetDevice(iface, iproute2, ipv4=ipv4, ipv6=ipv6)
Line 95: DynamicSourceRoute.addInterfaceTracking(net_dev)
Line 96: DhcpClient(iface).shutdown()
Line 97: iproute2._removeSourceRoute(net_dev, DynamicSourceRoute)
Line 100: ipwrapper.addrFlush(iface)
Line 101:
Line 102:
Line 103: def _drop_ip_config(iface):
Line 104: """Remove IP configuration of a new nic attached to VDSM"""
> nics are controlled by Vdsm, not attached to it.
Done
Line 105: if os.path.exists(os.path.join('/sys/class/net', iface)):
Line 106: kill_dhclient(iface, family=4) # kill_dhclient flushes IP
Line 107: kill_dhclient(iface, family=6)
Line 108:
Line 114: ipv4 = _get_ipv4_model(attrs)
Line 115: ipv6 = _get_ipv6_model(attrs)
Line 116: port = attrs.get('nic') or attrs.get('bonding')
Line 117: blocking_dhcp = 'blockingdhcp' in attrs
Line 118: return top_dev, ipv4, ipv6, port, blocking_dhcp
> Tuples are not bad by themselves, but I'd like to see a container object wi
Done
Line 119:
Line 120: ip_config_to_set = {}
Line 121: ip_config_to_remove = {}
Line 122:
Line 128: else:
Line 129: top_dev, ipv4, ipv6, port, blocking_dhcp = _gather_ip_config(attrs)
Line 130:
Line 131: if net in init_nets: # if network was edited
Line 132: init_top_dev, init_ipv4, init_ipv6, _, _ = \
> We can do without slashes.
Done
Line 133: _gather_ip_config(init_nets[net])
Line 134:
Line 135: # drop IP of newly attached nics
Line 136: if init_nets[net].get('nic') != attrs.get('nic') is not None:
Line 132: init_top_dev, init_ipv4, init_ipv6, _, _ = \
Line 133: _gather_ip_config(init_nets[net])
Line 134:
Line 135: # drop IP of newly attached nics
Line 136: if init_nets[net].get('nic') != attrs.get('nic') is not None:
> I failed to interpret this.
yourself or inside interpreter?
it means: (original nic is not the same as the new one) and (new nic is not None). = we changed nic, not just removed it.
In [1]: init_nets = {'net1': {'nic': 'dummy_1'}}
In [2]: init_nets['net1'].get('nic') != {'nic': 'dummy_1'}.get('nic') is not None
Out[2]: False
In [3]: init_nets['net1'].get('nic') != {'nic': None}.get('nic') is not None
Out[3]: False
In [4]: init_nets['net1'].get('nic') != {'nic': 'dummy_2'}.get('nic') is not None
Out[4]: True
Line 137: _drop_ip_config(attrs.get('nic'))
Line 138:
Line 139: # if IP config is to be changed or net top device was changed,
Line 140: # remove initial configuration and set the new one if any
Line 135: # drop IP of newly attached nics
Line 136: if init_nets[net].get('nic') != attrs.get('nic') is not None:
Line 137: _drop_ip_config(attrs.get('nic'))
Line 138:
Line 139: # if IP config is to be changed or net top device was changed,
> a network's top device?
Done
Line 140: # remove initial configuration and set the new one if any
Line 141: if (ipv4 != init_ipv4 or ipv6 != init_ipv6 or
Line 142: attrs.get('vlan') != init_nets[net].get('vlan')):
Line 143: ip_config_to_remove[init_top_dev] = ipv4, ipv6
Line 136: if init_nets[net].get('nic') != attrs.get('nic') is not None:
Line 137: _drop_ip_config(attrs.get('nic'))
Line 138:
Line 139: # if IP config is to be changed or net top device was changed,
Line 140: # remove initial configuration and set the new one if any
> Please be more verbose. Is it "any configuration"?
Done
Line 141: if (ipv4 != init_ipv4 or ipv6 != init_ipv6 or
Line 142: attrs.get('vlan') != init_nets[net].get('vlan')):
Line 143: ip_config_to_remove[init_top_dev] = ipv4, ipv6
Line 144: if ipv4 or ipv6:
Line 146: blocking_dhcp)
Line 147: else: # if network was added
Line 148: # drop IP of newly attached nics
Line 149: if attrs.get('nic') is not None:
Line 150: _drop_ip_config(attrs.get('nic'))
> You can attrs.get('nic') first, store it in a variable and then see if it i
Done
Line 151:
Line 152: # set networks IP configuration if any
Line 153: if ipv4 or ipv6:
Line 154: ip_config_to_set[top_dev] = (ipv4, ipv6, port,
Line 151:
Line 152: # set networks IP configuration if any
Line 153: if ipv4 or ipv6:
Line 154: ip_config_to_set[top_dev] = (ipv4, ipv6, port,
Line 155: blocking_dhcp)
> Again, tuples.
Done
Line 156:
Line 157: hooking.log('Remove IP configuration of: %s' % ip_config_to_remove)
Line 158: hooking.log('Set IP configuration: %s' % ip_config_to_set)
Line 159: for iface, (ipv4, ipv6) in ip_config_to_remove.iteritems():
--
To view, visit https://gerrit.ovirt.org/46692
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9682c75cfb79f75b64cb8cb587fc3ed01cf15c8
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list