Change in vdsm[master]: Remove usage of deathSignal

ybronhei at redhat.com ybronhei at redhat.com
Thu Nov 5 09:46:17 UTC 2015


Yaniv Bronhaim has uploaded a new change for review.

Change subject: Remove usage of deathSignal
......................................................................

Remove usage of deathSignal

deathSignal is an additional logic that python-cpopen addeds Popen
interface that will allow the initiator of subprocess to send automatic
signal to the subprocess on crash. Using systemd does it for us and kill
all subprocess in the same cgroup of the initiator, therefore we do not
require to have this additional logic anymore.

Change-Id: Ice7fc30ec90adf8055eb35a8c67c07f514030265
Signed-off-by: Yaniv Bronhaim <ybronhei at redhat.com>
---
M lib/vdsm/qemuimg.py
M lib/vdsm/utils.py
M lib/vdsm/virtsparsify.py
M vdsm/API.py
M vdsm/storage/blockSD.py
M vdsm/storage/curlImgWrap.py
M vdsm/storage/imageSharing.py
M vdsm/storage/misc.py
M vdsm/storage/storage_mailbox.py
M vdsm/v2v.py
10 files changed, 21 insertions(+), 45 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/48121/1

diff --git a/lib/vdsm/qemuimg.py b/lib/vdsm/qemuimg.py
index bce9973..68bb125 100644
--- a/lib/vdsm/qemuimg.py
+++ b/lib/vdsm/qemuimg.py
@@ -22,7 +22,6 @@
 import logging
 import os
 import re
-import signal
 
 from cpopen import CPopen
 
@@ -94,7 +93,7 @@
         cmd.extend(("-f", format))
 
     cmd.append(image)
-    rc, out, err = utils.execCmd(cmd, deathSignal=signal.SIGKILL)
+    rc, out, err = utils.execCmd(cmd)
 
     if rc != 0:
         raise QImgError(rc, out, err)
@@ -142,7 +141,7 @@
     if size is not None:
         cmd.append(str(size))
 
-    rc, out, err = utils.execCmd(cmd, cwd=cwdPath, deathSignal=signal.SIGKILL)
+    rc, out, err = utils.execCmd(cmd, cwd=cwdPath)
 
     if rc != 0:
         raise QImgError(rc, out, err)
@@ -155,7 +154,7 @@
         cmd.extend(("-f", format))
 
     cmd.append(image)
-    rc, out, err = utils.execCmd(cmd, deathSignal=signal.SIGKILL)
+    rc, out, err = utils.execCmd(cmd)
 
     # FIXME: handle different error codes and raise errors accordingly
     if rc != 0:
@@ -223,7 +222,7 @@
         cmd = cmdutils.wrap_command(cmd, with_nice=utils.NICENESS.HIGH,
                                     with_ioclass=utils.IOCLASS.IDLE)
         _log.debug(cmdutils.command_log_line(cmd, cwd=cwd))
-        self._command = CPopen(cmd, cwd=cwd, deathSignal=signal.SIGKILL)
+        self._command = CPopen(cmd, cwd=cwd)
         self._stream = utils.CommandStream(
             self._command, self._recvstdout, self._recvstderr)
 
@@ -295,7 +294,7 @@
         cmd.extend(("-f", format))
 
     cmd.extend((image, str(newSize)))
-    rc, out, err = utils.execCmd(cmd, deathSignal=signal.SIGKILL)
+    rc, out, err = utils.execCmd(cmd)
 
     if rc != 0:
         raise QImgError(rc, out, err)
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index f93e1ab..7f3f240 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -622,14 +622,9 @@
 def execCmd(command, sudo=False, cwd=None, data=None, raw=False,
             printable=None, env=None, sync=True, nice=None, ioclass=None,
             ioclassdata=None, setsid=False, execCmdLogger=logging.root,
-            deathSignal=0, childUmask=None, resetCpuAffinity=True):
+            childUmask=None, resetCpuAffinity=True):
     """
     Executes an external command, optionally via sudo.
-
-    IMPORTANT NOTE: the new process would receive `deathSignal` when the
-    controlling thread dies, which may not be what you intended: if you create
-    a temporary thread, spawn a sync=False sub-process, and have the thread
-    finish, the new subprocess would die immediately.
     """
 
     # Moving import here so that import utils over python3 will work. Once
@@ -651,7 +646,7 @@
     execCmdLogger.debug(cmdutils.command_log_line(printable, cwd=cwd))
 
     p = CPopen(command, close_fds=True, cwd=cwd, env=env,
-               deathSignal=deathSignal, childUmask=childUmask)
+               childUmask=childUmask)
     if not sync:
         p = AsyncProc(p)
         if data is not None:
@@ -680,13 +675,12 @@
 
 
 def watchCmd(command, stop, cwd=None, data=None, nice=None, ioclass=None,
-             execCmdLogger=logging.root, deathSignal=signal.SIGKILL):
+             execCmdLogger=logging.root):
     """
     Executes an external command, optionally via sudo with stop abilities.
     """
     proc = execCmd(command, cwd=cwd, data=data, sync=False,
-                   nice=nice, ioclass=ioclass, execCmdLogger=execCmdLogger,
-                   deathSignal=deathSignal)
+                   nice=nice, ioclass=ioclass, execCmdLogger=execCmdLogger)
 
     if not proc.wait(cond=stop):
         proc.kill()
diff --git a/lib/vdsm/virtsparsify.py b/lib/vdsm/virtsparsify.py
index 1399167..e85af58 100644
--- a/lib/vdsm/virtsparsify.py
+++ b/lib/vdsm/virtsparsify.py
@@ -19,7 +19,6 @@
 #
 
 from __future__ import absolute_import
-import signal
 
 from . import utils
 
@@ -58,7 +57,7 @@
 
     cmd.extend((src_vol, dst_vol))
 
-    rc, _, err = utils.execCmd(cmd, deathSignal=signal.SIGKILL)
+    rc, _, err = utils.execCmd(cmd)
 
     if rc != 0:
         raise Error(rc, err)
diff --git a/vdsm/API.py b/vdsm/API.py
index 9cb58a4..7399483 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -22,7 +22,6 @@
 
 from contextlib import contextmanager
 import os
-import signal
 import six
 import sys
 import time
@@ -1200,12 +1199,8 @@
            action can be one of (status, on, off, reboot)."""
 
         def fence(script, inp):
-            # non-status actions are sent asyncronously. deathSignal is set to
-            # make sure that no stray fencing scripts are left behind if Vdsm
-            # crashes.
             rc, out, err = utils.execCmd(
-                [script], deathSignal=signal.SIGTERM,
-                data=inp)
+                [script], data=inp)
             self.log.debug('rc %s inp %s out %s err %s', rc,
                            hidePasswd(inp), out, err)
             return rc, out, err
diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index 2621f74..7bbf55d 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -217,7 +217,7 @@
            "of=%s" % lvm.lvPath(sdUUID, volUUID), "bs=%s" % BS,
            "count=%s" % count]
     p = misc.execCmd(cmd, sync=False, nice=utils.NICENESS.HIGH,
-                     ioclass=utils.IOCLASS.IDLE, deathSignal=signal.SIGKILL)
+                     ioclass=utils.IOCLASS.IDLE)
     return p
 
 
@@ -1201,8 +1201,7 @@
 
         masterfsdev = lvm.lvPath(self.sdUUID, MASTERLV)
         cmd = [constants.EXT_FSCK, "-p", masterfsdev]
-        (rc, out, err) = misc.execCmd(cmd, sudo=True,
-                                      deathSignal=signal.SIGKILL)
+        (rc, out, err) = misc.execCmd(cmd, sudo=True)
         # fsck exit codes
         # 0    - No errors
         # 1    - File system errors corrected
@@ -1227,7 +1226,7 @@
         # If there is a journal already tune2fs will do nothing, indicating
         # this condition only with exit code. However, we do not really care.
         cmd = [constants.EXT_TUNE2FS, "-j", masterfsdev]
-        misc.execCmd(cmd, sudo=True, deathSignal=signal.SIGKILL)
+        misc.execCmd(cmd, sudo=True)
 
         masterMount = mount.Mount(masterfsdev, masterDir)
 
@@ -1391,7 +1390,7 @@
     Create a special file system to store VM data
     """
     cmd = [constants.EXT_MKFS, "-q", "-j", "-E", "nodiscard", dev]
-    rc = misc.execCmd(cmd, sudo=True, deathSignal=signal.SIGKILL)[0]
+    rc = misc.execCmd(cmd, sudo=True)[0]
     if rc != 0:
         raise se.MkfsError(dev)
 
diff --git a/vdsm/storage/curlImgWrap.py b/vdsm/storage/curlImgWrap.py
index 6b40906..2f63311 100644
--- a/vdsm/storage/curlImgWrap.py
+++ b/vdsm/storage/curlImgWrap.py
@@ -18,8 +18,6 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
-import signal
-
 from vdsm import utils, constants
 
 _curl = utils.CommandPath("curl",
@@ -65,7 +63,7 @@
     cmd = [constants.EXT_CURL_IMG_WRAP, "--download"]
     cmd.extend(_headersToOptions(headers) + [path, url])
 
-    rc, out, err = utils.execCmd(cmd, deathSignal=signal.SIGKILL)
+    rc, out, err = utils.execCmd(cmd)
 
     if rc != 0:
         raise CurlError(rc, out, err)
@@ -75,7 +73,7 @@
     cmd = [constants.EXT_CURL_IMG_WRAP, "--upload"]
     cmd.extend(_headersToOptions(headers) + [path, url])
 
-    rc, out, err = utils.execCmd(cmd, deathSignal=signal.SIGKILL)
+    rc, out, err = utils.execCmd(cmd)
 
     if rc != 0:
         raise CurlError(rc, out, err)
diff --git a/vdsm/storage/imageSharing.py b/vdsm/storage/imageSharing.py
index a5112c8..c47ec1b 100644
--- a/vdsm/storage/imageSharing.py
+++ b/vdsm/storage/imageSharing.py
@@ -18,7 +18,6 @@
 #
 
 import logging
-import signal
 
 import curlImgWrap
 from vdsm import constants
@@ -78,8 +77,7 @@
     totalSize = getLengthFromArgs(methodArgs)
     fileObj = methodArgs['fileObj']
     cmd = [constants.EXT_DD, "of=%s" % dstImgPath, "bs=%s" % constants.MEGAB]
-    p = utils.execCmd(cmd, sudo=False, sync=False,
-                      deathSignal=signal.SIGKILL)
+    p = utils.execCmd(cmd, sudo=False, sync=False)
     try:
         _copyData(fileObj, p.stdin, totalSize)
         p.stdin.close()
@@ -104,8 +102,7 @@
     cmd = [constants.EXT_DD, "if=%s" % dstImgPath, "bs=%s" % constants.MEGAB,
            "count=%s" % (total_size / constants.MEGAB + 1)]
 
-    p = utils.execCmd(cmd, sync=False,
-                      deathSignal=signal.SIGKILL)
+    p = utils.execCmd(cmd, sync=False)
     p.blocking = True
     try:
         _copyData(p.stdout, fileObj, bytes_left)
diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py
index d35c7a6..4e2692e 100644
--- a/vdsm/storage/misc.py
+++ b/vdsm/storage/misc.py
@@ -39,7 +39,6 @@
 import Queue
 import random
 import re
-import signal
 import string
 import struct
 import threading
@@ -339,8 +338,7 @@
 
         if not stop:
             (rc, out, err) = execCmd(cmd, nice=utils.NICENESS.HIGH,
-                                     ioclass=utils.IOCLASS.IDLE,
-                                     deathSignal=signal.SIGKILL)
+                                     ioclass=utils.IOCLASS.IDLE)
         else:
             (rc, out, err) = watchCmd(cmd, stop=stop,
                                       nice=utils.NICENESS.HIGH,
diff --git a/vdsm/storage/storage_mailbox.py b/vdsm/storage/storage_mailbox.py
index e2a6708..6109961 100644
--- a/vdsm/storage/storage_mailbox.py
+++ b/vdsm/storage/storage_mailbox.py
@@ -24,7 +24,6 @@
 import threading
 import Queue
 import struct
-import signal
 import logging
 
 import uuid
@@ -80,7 +79,6 @@
 
 
 def _mboxExecCmd(*args, **kwargs):
-    kwargs['deathSignal'] = signal.SIGKILL
     return misc.execCmd(*args, **kwargs)
 
 
diff --git a/vdsm/v2v.py b/vdsm/v2v.py
index 997979e..6697953 100644
--- a/vdsm/v2v.py
+++ b/vdsm/v2v.py
@@ -30,7 +30,6 @@
 import logging
 import os
 import re
-import signal
 import threading
 import xml.etree.ElementTree as ET
 
@@ -426,7 +425,7 @@
 
         # This is the way we run qemu-img convert jobs. virt-v2v is invoking
         # qemu-img convert to perform the migration.
-        self._proc = execCmd(cmd, sync=False, deathSignal=signal.SIGTERM,
+        self._proc = execCmd(cmd, sync=False,
                              nice=NICENESS.HIGH, ioclass=IOCLASS.IDLE,
                              env=self._execution_environments())
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ice7fc30ec90adf8055eb35a8c67c07f514030265
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>


More information about the vdsm-patches mailing list