Change in vdsm[ovirt-3.6]: net: ovs: remove initial nic ip, dont touch unchanged networks

phoracek at redhat.com phoracek at redhat.com
Thu Dec 3 23:31:04 UTC 2015


Hello Ido Barkan, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/49695

to review the following change.

Change subject: net: ovs: remove initial nic ip, dont touch unchanged networks
......................................................................

net: ovs: remove initial nic ip, dont touch unchanged networks

According to ifcfg configurator behavior, now we remove inital IP
configuration of nics attached to VDSM network.

Also there is an improvement in IP changes handling: If IP configuration
was not changed, don't recreate it.

Signed-off-by: Petr Horáček <phoracek at redhat.com>
Change-Id: Ie9682c75cfb79f75b64cb8cb587fc3ed01cf15c8
Reviewed-on: https://gerrit.ovirt.org/46692
Continuous-Integration: Jenkins CI
Reviewed-by: Ido Barkan <ibarkan at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Bug-Url: https://bugzilla.redhat.com/1234867
---
M vdsm_hooks/ovs/README
M vdsm_hooks/ovs/ovs_before_network_setup_ip.py
2 files changed, 41 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/95/49695/1

diff --git a/vdsm_hooks/ovs/README b/vdsm_hooks/ovs/README
index 8231e46..22c5cc7 100644
--- a/vdsm_hooks/ovs/README
+++ b/vdsm_hooks/ovs/README
@@ -70,9 +70,6 @@
   + We need to be able to set and get those values. It should be possible
     via 'mcast_snooping_enable' records, described here [1]
 - Implement implicitBonding
-- Remove IP configuration of nics attached to OVS networks:
-  + ifcfg removes IP addresses and stops dhclients running on NICs attached to
-    VDSM networks. We should do the same with OVS.
 - Defined multiple OVS Bridge instances:
   + Now custom network property 'ovs' is passing only True, we could change it
     to pass an integer and then create OVS network under 'ovsbr$INT'
diff --git a/vdsm_hooks/ovs/ovs_before_network_setup_ip.py b/vdsm_hooks/ovs/ovs_before_network_setup_ip.py
index e8f5bce..c29d517 100644
--- a/vdsm_hooks/ovs/ovs_before_network_setup_ip.py
+++ b/vdsm_hooks/ovs/ovs_before_network_setup_ip.py
@@ -18,6 +18,7 @@
 # Refer to the README and COPYING files for full details of the license
 #
 from collections import namedtuple
+import os
 import sys
 
 from vdsm import ipwrapper, sysctl
@@ -28,7 +29,7 @@
 
 # TODO: move required modules into vdsm/lib
 sys.path.append('/usr/share/vdsm')
-from network.configurators.dhclient import DhcpClient
+from network.configurators.dhclient import DhcpClient, kill_dhclient
 from network.configurators.iproute2 import Iproute2
 from network.models import NetDevice, IPv4, IPv6
 from network.sourceroute import DynamicSourceRoute
@@ -113,6 +114,13 @@
             ipwrapper.addrFlush(iface)
 
 
+def _drop_ip_config(iface):
+    """Remove IP configuration of a new nic controlled by VDSM"""
+    if os.path.exists(os.path.join('/sys/class/net', iface)):
+        kill_dhclient(iface, family=4)  # kill_dhclient flushes IP
+        kill_dhclient(iface, family=6)
+
+
 def configure_ip(nets, init_nets):
 
     def _gather_ip_config(attrs):
@@ -126,14 +134,39 @@
     ip_config_to_set = {}
     ip_config_to_remove = {}
 
-    for net, attrs in nets.items():
-        if net in init_nets:
-            init_ip_config = _gather_ip_config(init_nets[net])
-            ip_config_to_remove[init_ip_config.top_dev] = init_ip_config
-        if 'remove' not in attrs:
+    for net, attrs in nets.iteritems():
+        if 'remove' in attrs:  # if network was removed
+            # remove network's IP configuration (running dhclient)
+            ip_config = _gather_ip_config(init_nets[net])
+            ip_config_to_remove[ip_config.top_dev] = ip_config
+        else:
             ip_config = _gather_ip_config(attrs)
-            if ip_config.ipv4 or ip_config.ipv6:
-                ip_config_to_set[ip_config.top_dev] = ip_config
+            if net in init_nets:  # if network was edited
+                init_ip_config = _gather_ip_config(init_nets[net])
+
+                # drop IP of newly attached nics
+                if init_nets[net].get('nic') != attrs.get('nic') is not None:
+                    _drop_ip_config(attrs.get('nic'))
+
+                # if IP config is to be changed or network's top device was
+                # changed, remove initial IP configuration and set the new one
+                # if there is any
+                if (ip_config.ipv4 != init_ip_config.ipv4 or
+                    ip_config.ipv6 != init_ip_config.ipv6 or
+                        attrs.get('vlan') != init_nets[net].get('vlan')):
+                    ip_config_to_remove[
+                        init_ip_config.top_dev] = init_ip_config
+                    if ip_config.ipv4 or ip_config.ipv6:
+                        ip_config_to_set[ip_config.top_dev] = ip_config
+            else:  # if network was added
+                # drop IP of newly attached nics
+                nic = attrs.get('nic')
+                if nic is not None:
+                    _drop_ip_config(nic)
+
+                # set networks IP configuration if any
+                if ip_config.ipv4 or ip_config.ipv6:
+                    ip_config_to_set[ip_config.top_dev] = ip_config
 
     hooking.log('Remove IP configuration of: %s' % ip_config_to_remove)
     hooking.log('Set IP configuration: %s' % ip_config_to_set)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie9682c75cfb79f75b64cb8cb587fc3ed01cf15c8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
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>


More information about the vdsm-patches mailing list