Change in vdsm[master]: Revert "network: bridge inherits DHCP unique identifier from...

danken at redhat.com danken at redhat.com
Thu Sep 3 03:11:45 UTC 2015


Dan Kenigsberg has uploaded a new change for review.

Change subject: Revert "network: bridge inherits DHCP unique identifier from its DHCP-enabled port"
......................................................................

Revert "network: bridge inherits DHCP unique identifier from its DHCP-enabled port"

This reverts commit 2ccbb949f55a812d97bbc076e7b33ebd11cb602d
which fails with older dhclient that does not support -df

 dhclient[15151]: Usage: dhclient [-4|-6] [-SNTP1dvrx] [-nw] [-p <port>] [-D LL|LLT]

Change-Id: I087eb9886160767492fd3a8d000c7b8b0711c841
Signed-off-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/functional/networkTests.py
M vdsm/network/api.py
M vdsm/network/configurators/__init__.py
M vdsm/network/configurators/dhclient.py
M vdsm/network/configurators/ifcfg.py
M vdsm/network/models.py
6 files changed, 7 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/85/45685/1

diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 1873222..6612034 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -2113,58 +2113,6 @@
 
     @cleanupNet
     @RequireVethMod
-    def testDhcpReplaceNicWithBridge(self):
-        with veth.pair() as (left, right):
-            veth.setIP(left, IP_ADDRESS, IP_CIDR)
-            veth.setIP(left, IPv6_ADDRESS, IPv6_CIDR, 6)
-            veth.setLinkUp(left)
-            with dnsmasqDhcp(left):
-
-                # first, a network without a bridge should get a certain
-                # address
-
-                network = {NETWORK_NAME: {'nic': right, 'bridged': False,
-                                          'bootproto': 'dhcp',
-                                          'blockingdhcp': True}}
-                try:
-                    status, msg = self.setupNetworks(network, {}, NOCHK)
-                    self.assertEqual(status, SUCCESS, msg)
-                    self.assertNetworkExists(NETWORK_NAME)
-
-                    test_net = self.vdsm_net.netinfo.networks[NETWORK_NAME]
-                    self.assertEqual(test_net['dhcpv4'], True)
-                    devs = self.vdsm_net.netinfo.nics
-                    device_name = right
-
-                    self.assertIn(device_name, devs)
-                    net_attrs = devs[device_name]
-                    self.assertEqual(net_attrs['dhcpv4'], True)
-
-                    self.assertEqual(test_net['gateway'], IP_GATEWAY)
-                    ip_addr = test_net['addr']
-                    self.assertSourceRoutingConfiguration(device_name, ip_addr)
-
-                    # now, a bridged network should get the same address
-                    # (because dhclient should send the same dhcp-client-
-                    #  identifier)
-
-                    network[NETWORK_NAME]['bridged'] = True
-                    status, msg = self.setupNetworks(network, {}, NOCHK)
-                    self.assertEqual(status, SUCCESS, msg)
-                    test_net = self.vdsm_net.netinfo.networks[NETWORK_NAME]
-                    self.assertEqual(ip_addr, test_net['addr'])
-
-                    network = {NETWORK_NAME: {'remove': True}}
-                    status, msg = self.setupNetworks(network, {}, NOCHK)
-                    self.assertEqual(status, SUCCESS, msg)
-                    self.assertNetworkDoesntExist(NETWORK_NAME)
-
-                finally:
-                    dhcp.delete_dhclient_leases(right, True, False)
-                    dhcp.delete_dhclient_leases(NETWORK_NAME, True, False)
-
-    @cleanupNet
-    @RequireVethMod
     def testSetupNetworksReconfigureBridge(self):
         def setup_test_network(dhcp=True):
             network_params = {'nic': right, 'bridged': True}
diff --git a/vdsm/network/api.py b/vdsm/network/api.py
index 5b3a905..5c8bed5 100755
--- a/vdsm/network/api.py
+++ b/vdsm/network/api.py
@@ -166,9 +166,6 @@
                                      'bridge STP value.' % stp)
         topNetDev = Bridge(bridge, configurator, port=topNetDev, mtu=mtu,
                            stp=stp)
-        # inherit DUID from the port's existing DHCP lease (BZ#1219429)
-        if topNetDev.port and bootproto == 'dhcp':
-            _inherit_dhcp_unique_identifier(topNetDev, _netinfo)
     if topNetDev is None:
         raise ConfigNetworkError(ne.ERR_BAD_PARAMS, 'Network defined without '
                                  'devices.')
@@ -601,18 +598,6 @@
         if not set(nics).issubset(currentNicsSet):
             raise ConfigNetworkError(ne.ERR_BAD_NIC,
                                      "Unknown nics in: %r" % list(nics))
-
-
-def _inherit_dhcp_unique_identifier(bridge, _netinfo):
-    """
-    If there is dhclient already running on a bridge's port we have to use the
-    same DHCP unique identifier (DUID) in order to get the same IP address.
-    """
-    for devices in (_netinfo.nics, _netinfo.bondings, _netinfo.vlans):
-        port = devices.get(bridge.port.name)
-        if port and port['dhcpv4']:
-            bridge.duid_source = bridge.port.name
-            break
 
 
 def _handleBondings(bondings, configurator, in_rollback):
diff --git a/vdsm/network/configurators/__init__.py b/vdsm/network/configurators/__init__.py
index bca3eba..8cadf1d 100644
--- a/vdsm/network/configurators/__init__.py
+++ b/vdsm/network/configurators/__init__.py
@@ -175,7 +175,7 @@
 
 
 def runDhclient(iface, family=4, default_route=False):
-    dhclient = DhcpClient(iface.name, family, default_route, iface.duid_source)
+    dhclient = DhcpClient(iface.name, family, default_route)
     rc = dhclient.start(iface.blockingdhcp)
     if iface.blockingdhcp and rc:
         raise ConfigNetworkError(ERR_FAILED_IFUP, 'dhclient%s failed' % family)
diff --git a/vdsm/network/configurators/dhclient.py b/vdsm/network/configurators/dhclient.py
index 181c302..88edd1b 100644
--- a/vdsm/network/configurators/dhclient.py
+++ b/vdsm/network/configurators/dhclient.py
@@ -34,25 +34,23 @@
 from vdsm.utils import rmFile
 
 DHCLIENT_CGROUP = 'vdsm-dhclient'
-LEASE_DIR = '/var/lib/dhclient'
-LEASE_FILE = os.path.join(LEASE_DIR, 'dhclient{0}--{1}.lease')
 
 
 class DhcpClient(object):
     PID_FILE = '/var/run/dhclient%s-%s.pid'
+    LEASE_DIR = '/var/lib/dhclient'
+    LEASE_FILE = os.path.join(LEASE_DIR, 'dhclient{0}--{1}.lease')
     DHCLIENT = CommandPath('dhclient', '/sbin/dhclient')
 
-    def __init__(self, iface, family=4, default_route=False, duid_source=None,
+    def __init__(self, iface, family=4, default_route=False,
                  cgroup=DHCLIENT_CGROUP):
         self.iface = iface
         self.family = family
         self.default_route = default_route
-        self.duid_source_file = None if duid_source is None else (
-            LEASE_FILE.format('' if family == 4 else '6', duid_source))
         self.pidFile = self.PID_FILE % (family, self.iface)
-        if not os.path.exists(LEASE_DIR):
-            os.mkdir(LEASE_DIR)
-        self.leaseFile = LEASE_FILE.format(
+        if not os.path.exists(self.LEASE_DIR):
+            os.mkdir(self.LEASE_DIR)
+        self.leaseFile = self.LEASE_FILE.format(
             '' if family == 4 else '6', self.iface)
         self._cgroup = cgroup
 
@@ -65,8 +63,6 @@
         if not self.default_route:
             # Instruct Fedora/EL's dhclient-script not to set gateway on iface
             cmd += ['-e', 'DEFROUTE=no']
-        if self.duid_source_file:
-            cmd += ['-df', self.duid_source_file]
         cmd = cmdutils.systemd_run(cmd, scope=True, slice=self._cgroup)
         rc, out, err = execCmd(cmd)
         return rc, out, err
diff --git a/vdsm/network/configurators/ifcfg.py b/vdsm/network/configurators/ifcfg.py
index e1d3e94..94878d0 100644
--- a/vdsm/network/configurators/ifcfg.py
+++ b/vdsm/network/configurators/ifcfg.py
@@ -552,10 +552,6 @@
         if bridge.stp is not None:
             conf += 'STP=%s\n' % ('on' if bridge.stp else 'off')
         conf += 'ONBOOT=yes\n'
-        if bridge.duid_source:
-            duid_source_file = dhclient.LEASE_FILE.format(
-                '', bridge.duid_source)
-            conf += 'DHCLIENTARGS="-df %s"\n' % duid_source_file
 
         if 'custom' in opts and 'bridge_opts' in opts['custom']:
             opts['bridging_opts'] = opts['custom']['bridge_opts']
diff --git a/vdsm/network/models.py b/vdsm/network/models.py
index a0012d2..64cae6e 100644
--- a/vdsm/network/models.py
+++ b/vdsm/network/models.py
@@ -39,7 +39,6 @@
         self.mtu = mtu
         self.configurator = configurator
         self.master = None
-        self.duid_source = None
 
     def __iter__(self):
         raise NotImplementedError


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I087eb9886160767492fd3a8d000c7b8b0711c841
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list