Change in vdsm[ovirt-3.6]: network.api: EL7's dhclient doesn't support -df so stop usin...

osvoboda at redhat.com osvoboda at redhat.com
Fri Sep 4 16:33:40 UTC 2015


Ondřej Svoboda has uploaded a new change for review.

Change subject: network.api: EL7's dhclient doesn't support -df so stop using it there
......................................................................

network.api: EL7's dhclient doesn't support -df so stop using it there

This is a quick fix only. A proper solution for the affected bug will
have to fall back to -lf (a copy of a lease file).

Change-Id: Ia096d42d24b00e7ef075f1a2dde7e3a951c0b81c
Bug-Url: https://bugzilla.redhat.com/1219429
Signed-off-by: Ondřej Svoboda <osvoboda at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/45713
Reviewed-by: Francesco Romani <fromani at redhat.com>
Tested-by: Martin Sivák <msivak at redhat.com>
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/network/api.py
M vdsm/network/configurators/__init__.py
M vdsm/network/configurators/dhclient.py
3 files changed, 26 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/54/45754/1

diff --git a/vdsm/network/api.py b/vdsm/network/api.py
index 5b3a905..5bd675a 100755
--- a/vdsm/network/api.py
+++ b/vdsm/network/api.py
@@ -36,7 +36,7 @@
 from vdsm import utils
 from vdsm import ipwrapper
 
-from .configurators import libvirt
+from .configurators import dhclient, libvirt
 from .errors import ConfigNetworkError
 from . import errors as ne
 from .models import Bond, Bridge, IPv4, IPv6, Nic, Vlan
@@ -608,6 +608,14 @@
     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.
     """
+    # On EL7 dhclient doesn't have a -df option (to read DUID from the port's
+    # lease file). We must detect if the option is available, by running
+    # dhclient manually. To unbreak a beta4 release we just won't use the -df
+    # option in that case. A proper fix is probably to fall back to -lf,
+    # passing to it a modified NIC lease file.
+    if not dhclient.supports_duid_file():
+        return
+
     for devices in (_netinfo.nics, _netinfo.bondings, _netinfo.vlans):
         port = devices.get(bridge.port.name)
         if port and port['dhcpv4']:
diff --git a/vdsm/network/configurators/__init__.py b/vdsm/network/configurators/__init__.py
index bca3eba..86e17a1 100644
--- a/vdsm/network/configurators/__init__.py
+++ b/vdsm/network/configurators/__init__.py
@@ -176,7 +176,7 @@
 
 def runDhclient(iface, family=4, default_route=False):
     dhclient = DhcpClient(iface.name, family, default_route, iface.duid_source)
-    rc = dhclient.start(iface.blockingdhcp)
+    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..783126b 100644
--- a/vdsm/network/configurators/dhclient.py
+++ b/vdsm/network/configurators/dhclient.py
@@ -28,10 +28,7 @@
 from vdsm import cmdutils
 from vdsm import ipwrapper
 from vdsm import netinfo
-from vdsm.utils import CommandPath
-from vdsm.utils import execCmd
-from vdsm.utils import pgrep
-from vdsm.utils import rmFile
+from vdsm.utils import CommandPath, execCmd, memoized, pgrep, rmFile
 
 DHCLIENT_CGROUP = 'vdsm-dhclient'
 LEASE_DIR = '/var/lib/dhclient'
@@ -68,13 +65,11 @@
         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
+        return execCmd(cmd)
 
     def start(self, blocking):
         if blocking:
-            rc, _, _ = self._dhclient()
-            return rc
+            return self._dhclient()
         else:
             t = threading.Thread(target=self._dhclient, name='vdsm-dhclient-%s'
                                  % self.iface)
@@ -137,3 +132,16 @@
             raise
     if pid_file is not None:
         rmFile(pid_file)
+
+
+ at memoized
+def supports_duid_file():
+    probe = DhcpClient('-invalid-option')  # dhclient doesn't have -h/--help
+    rc, out, err = probe.start(blocking=True)
+    if rc:  # -invalid-option should always fail, unlike --help if implemented
+        for line in err:
+            if '-df' in line:
+                return True
+        return False
+    else:
+        raise AssertionError("dhclient shouldn't succeed with invalid options")


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia096d42d24b00e7ef075f1a2dde7e3a951c0b81c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Ondřej Svoboda <osvoboda at redhat.com>


More information about the vdsm-patches mailing list