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 <dan...@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/86/45686/1 diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 39d8dab..e811955 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -2108,58 +2108,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/45686 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I087eb9886160767492fd3a8d000c7b8b0711c841 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.6 Gerrit-Owner: Dan Kenigsberg <dan...@redhat.com> _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches