Change in vdsm[master]: net: run dhclient in its own process group.

Ido Barkan has uploaded a new change for review.

Change subject: net: run dhclient in its own process group.
......................................................................

net: run dhclient in its own process group.

This is done to prevent systemd from killing supervdsm dhclient child
processes during supervdsm stop (or restart). Forking dhclient in
its own process group prevents systemd from doing this.
Since dhclient is deliberately killed either directly by supervdsm or
indirectly by executing ifdown on a device and since executing a
process in a different process group is harmless, the separate
cgroup is now the default behaviour.
This patch also fixes https://bugzilla.redhat.com/show_bug
.cgi?id=1187244 in a less clumsy way than KillMode=process.

Change-Id: I82848a36b52cd8e9dec188d865ef86edc4bb7488
Signed-off-by: Ido Barkan <ibarkan(a)redhat.com&gt;
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1228322
---
M init/systemd/vdsm-network.service.in
M vdsm/network/configurators/dhclient.py
M vdsm/network/configurators/ifcfg.py
3 files changed, 20 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/05/43005/1

diff --git a/init/systemd/vdsm-network.service.in b/init/systemd/vdsm-network.service.in
index 722a2b4..3c992f3 100644
--- a/init/systemd/vdsm-network.service.in
+++ b/init/systemd/vdsm-network.service.in
@@ -9,7 +9,6 @@
 EnvironmentFile=-/etc/sysconfig/vdsm
 ExecStartPre=@BINDIR@/vdsm-tool --vvverbose --append --logfile=@VDSMLOGDIR(a)/upgrade.log
upgrade-unified-persistence
 ExecStart=@BINDIR@/vdsm-tool restore-nets
-KillMode=process
 RemainAfterExit=yes
 
 [Install]
diff --git a/vdsm/network/configurators/dhclient.py
b/vdsm/network/configurators/dhclient.py
index 67f7eba..d71b5ff 100644
--- a/vdsm/network/configurators/dhclient.py
+++ b/vdsm/network/configurators/dhclient.py
@@ -25,12 +25,15 @@
 import signal
 import threading
 
+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
+
+DHCLIENT_CGROUP = 'vdsm-dhclient'
 
 
 class DhcpClient(object):
@@ -39,7 +42,7 @@
     LEASE_FILE = os.path.join(LEASE_DIR, 'dhclient{0}--{1}.lease')
     DHCLIENT = CommandPath('dhclient', '/sbin/dhclient')
 
-    def __init__(self, iface, family=4):
+    def __init__(self, iface, family=4, cgroup=DHCLIENT_CGROUP):
         self.iface = iface
         self.family = family
         self.pidFile = self.PID_FILE % (family, self.iface)
@@ -47,14 +50,16 @@
             os.mkdir(self.LEASE_DIR)
         self.leaseFile = self.LEASE_FILE.format(
             '' if family == 4 else '6', self.iface)
+        self._cgroup = cgroup
 
     def _dhclient(self):
         # Ask dhclient to stop any dhclient running for the device
         if os.path.exists(os.path.join(netinfo.NET_PATH, self.iface)):
             kill_dhclient(self.iface, self.family)
-        rc, out, err = execCmd([self.DHCLIENT.cmd, '-%s' % self.family,
-                                '-1', '-pf', self.pidFile,
-                                '-lf', self.leaseFile, self.iface])
+        cmd = [self.DHCLIENT.cmd, '-%s' % self.family, '-1',
'-pf',
+               self.pidFile, '-lf', self.leaseFile, self.iface]
+        cmd = cmdutils.systemd_run(cmd, scope=True, slice=self._cgroup)
+        rc, out, err = execCmd(cmd)
         return rc, out, err
 
     def start(self, blocking):
diff --git a/vdsm/network/configurators/ifcfg.py b/vdsm/network/configurators/ifcfg.py
index e76be51..450a6d1 100644
--- a/vdsm/network/configurators/ifcfg.py
+++ b/vdsm/network/configurators/ifcfg.py
@@ -33,6 +33,7 @@
 from libvirt import libvirtError, VIR_ERR_NO_NETWORK
 
 from vdsm.config import config
+from vdsm import cmdutils
 from vdsm import constants
 from vdsm import ipwrapper
 from vdsm import netinfo
@@ -740,9 +741,14 @@
     return rc
 
 
-def _exec_ifup(iface_name):
+def _exec_ifup(iface_name, cgroup=dhclient.DHCLIENT_CGROUP):
     """Bring up an interface"""
-    rc, out, err = utils.execCmd([constants.EXT_IFUP, iface_name], raw=False)
+    cmd = [constants.EXT_IFUP, iface_name]
+
+    if cgroup is not None:
+        cmd = cmdutils.systemd_run(cmd, scope=True, slice=cgroup)
+
+    rc, out, err = utils.execCmd(cmd, raw=False)
 
     if rc != 0:
         # In /etc/sysconfig/network-scripts/ifup* the last line usually
@@ -750,16 +756,16 @@
         raise ConfigNetworkError(ERR_FAILED_IFUP, out[-1] if out else '')
 
 
-def _ifup(iface):
+def _ifup(iface, cgroup=dhclient.DHCLIENT_CGROUP):
     if not iface.blockingdhcp and (iface.ipv4.bootproto == 'dhcp' or
                                    iface.ipv6.dhcpv6):
         # wait for dhcp in another thread, so vdsm won't get stuck (BZ#498940)
         t = threading.Thread(target=_exec_ifup, name='ifup-waiting-on-dhcp',
-                             args=(iface.name,))
+                             args=(iface.name, cgroup))
         t.daemon = True
         t.start()
     else:
-        _exec_ifup(iface.name)
+        _exec_ifup(iface.name, cgroup)
 
 
 def _restore_default_bond_options(bond_name, desired_options):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I82848a36b52cd8e9dec188d865ef86edc4bb7488
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan(a)redhat.com&gt;