Change in vdsm[master]: Make advanced process invocations part of execCmd

smizrahi at redhat.com smizrahi at redhat.com
Mon Nov 5 21:58:16 UTC 2012


Saggi Mizrahi has uploaded a new change for review.

Change subject: Make advanced process invocations part of execCmd
......................................................................

Make advanced process invocations part of execCmd

There are a bunch of places in the code where priority is set using ad
hoc command concatenation. This is hard to understand and error prone.
This patch adds this as a feature of execCmd so proper use is always
enforced.

- niceness can be set with the nice parameter
- ionice class and class data can also be set with the appropriate
  parameters
- setsid can be set as a boolean with setsid=True

Change-Id: I5abd61c03ce907ea94761f0209a4626669051eb1
Signed-off-by: Saggi Mizrahi <smizrahi at redhat.com>
---
M vdsm/constants.py.in
M vdsm/storage/misc.py
M vdsm/storage/safelease.py
M vdsm/storage/volume.py
M vdsm/sudoers.vdsm.in
5 files changed, 53 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/9042/1

diff --git a/vdsm/constants.py.in b/vdsm/constants.py.in
index 2e9eb99..dead7ed 100644
--- a/vdsm/constants.py.in
+++ b/vdsm/constants.py.in
@@ -150,8 +150,6 @@
 EXT_WGET = '@WGET_PATH@'
 EXT_WRITE_NET_CONFIG = '@VDSMDIR@/write-net-config'
 
-CMD_LOWPRIO = [EXT_NICE, '-n', '19', EXT_IONICE, '-c', '3']
-
 #
 # Storage constants
 #
diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py
index 3a1280b..4ab7253 100644
--- a/vdsm/storage/misc.py
+++ b/vdsm/storage/misc.py
@@ -181,12 +181,38 @@
                               logSkipName="Storage.Misc.excCmd")
 
 
+class IOCLASS:
+    REALTIME = 1
+    BEST_EFFORT = 2
+    IDLE = 3
+
+
+class NICENESS:
+    NORMAL = 0
+    LOW = 19
+
+
 @logskip("Storage.Misc.excCmd")
 def execCmd(command, sudo=False, cwd=None, data=None, raw=False, logErr=True,
-            printable=None, env=None, sync=True):
+            printable=None, env=None, sync=True, nice=None, ioclass=None,
+            ioclassdata=None, setsid=False):
     """
     Executes an external command, optionally via sudo.
     """
+    if ioclass is not None:
+        cmd = command
+        command = [constants.EXT_IONICE, '-c', str(ioclass)]
+        if ioclassdata is not None:
+            command.extend(("-n", str(ioclassdata)))
+
+        command = command + cmd
+
+    if nice is not None:
+        command = [constants.EXT_NICE, '-n', str(nice)] + command
+
+    if setsid:
+        command = [constants.EXT_SETSID] + command
+
     if sudo:
         command = [constants.EXT_SUDO, SUDO_NON_INTERACTIVE_FLAG] + command
 
@@ -246,11 +272,13 @@
     return str(ctime)
 
 
-def watchCmd(command, stop, cwd=None, data=None, recoveryCallback=None):
+def watchCmd(command, stop, cwd=None, data=None, recoveryCallback=None,
+             nice=None, ioclass=None):
     """
     Executes an external command, optionally via sudo with stop abilities.
     """
-    proc = execCmd(command, sudo=False, cwd=cwd, data=data, sync=False)
+    proc = execCmd(command, sudo=False, cwd=cwd, data=data, sync=False,
+                   nice=nice, ioclass=ioclass)
     if recoveryCallback:
         recoveryCallback(proc)
 
@@ -393,13 +421,13 @@
         if oflag:
             cmd.append("oflag=%s" % oflag)
 
-        cmd = constants.CMD_LOWPRIO + cmd
-
         if not stop:
-            (rc, out, err) = execCmd(cmd, sudo=False)
+            (rc, out, err) = execCmd(cmd, sudo=False, nice=NICENESS.LOW,
+                                     ioclass=IOCLASS.IDLE)
         else:
             (rc, out, err) = watchCmd(cmd, stop=stop,
-                                      recoveryCallback=recoveryCallback)
+                                      recoveryCallback=recoveryCallback,
+                                      nice=NICENESS.LOW, ioclass=IOCLASS.IDLE)
 
         if rc:
             raise se.MiscBlockWriteException(dst, offset, size)
diff --git a/vdsm/storage/safelease.py b/vdsm/storage/safelease.py
index 551dda5..4d706b0 100644
--- a/vdsm/storage/safelease.py
+++ b/vdsm/storage/safelease.py
@@ -99,11 +99,12 @@
                 str(self._leaseFile), str(leaseTimeMs), str(ioOpTimeoutMs),
                 str(self._leaseFailRetry)])
 
-            cmd = [constants.EXT_SETSID, constants.EXT_IONICE, '-c1', '-n0',
-                constants.EXT_SU, misc.IOUSER, '-s', constants.EXT_SH, '-c',
-                acquireLockCommand]
+            cmd = [constants.EXT_SU, misc.IOUSER, '-s', constants.EXT_SH, '-c',
+                   acquireLockCommand]
             (rc, out, err) = misc.execCmd(cmd, cwd=self.lockUtilPath,
-                    sudo=True)
+                                          sudo=True,
+                                          ioclass=misc.IOCLASS.REALTIME,
+                                          ioclassdata=0, setsid=True)
             if rc != 0:
                 raise se.AcquireLockFailure(self._sdUUID, rc, out, err)
             self.log.debug("Clustered lock acquired successfully")
diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py
index b97e6f3..fada654 100644
--- a/vdsm/storage/volume.py
+++ b/vdsm/storage/volume.py
@@ -981,9 +981,9 @@
     log.debug('(qemuRebase): REBASE %s (fmt=%s) on top of %s (%s) START' %
               (src, srcFormat, backingFile, backingFormat))
 
-    cmd = constants.CMD_LOWPRIO + [constants.EXT_QEMUIMG, "rebase",
-                                   "-t", "none", "-f", srcFormat,
-                                   "-b", backingFile, "-F", backingFormat]
+    cmd = [constants.EXT_QEMUIMG, "rebase",
+           "-t", "none", "-f", srcFormat,
+           "-b", backingFile, "-F", backingFormat]
     if unsafe:
         cmd += ["-u"]
     cmd += [src]
@@ -992,7 +992,9 @@
     if rollback:
         recoveryCallback = baseAsyncTasksRollback
     (rc, out, err) = misc.watchCmd(cmd, stop=stop, cwd=cwd,
-                                   recoveryCallback=recoveryCallback)
+                                   recoveryCallback=recoveryCallback,
+                                   ioclass=misc.IOCLASS.IDLE,
+                                   nice=misc.NICENESS.LOW)
 
     log.debug('(qemuRebase): REBASE %s DONE' % (src))
     return (rc, out, err)
@@ -1014,11 +1016,13 @@
                                  stop=stop, size=size,
                                  recoveryCallback=baseAsyncTasksRollback)
     else:
-        cmd = constants.CMD_LOWPRIO + [constants.EXT_QEMUIMG, "convert",
-                                       "-t", "none", "-f", src_fmt, src,
-                                       "-O", dst_fmt, dst]
+        cmd = [constants.EXT_QEMUIMG, "convert",
+               "-t", "none", "-f", src_fmt, src,
+               "-O", dst_fmt, dst]
         (rc, out, err) = misc.watchCmd(cmd, stop=stop,
-                                       recoveryCallback=baseAsyncTasksRollback)
+                                       recoveryCallback=baseAsyncTasksRollback,
+                                       ioclass=misc.IOCLASS.IDLE,
+                                       nice=misc.NICENESS.LOW)
 
     log.debug('(qemuConvert): COPY %s to %s DONE' % (src, dst))
     return (rc, out, err)
diff --git a/vdsm/sudoers.vdsm.in b/vdsm/sudoers.vdsm.in
index ab99e8e..688fe16 100644
--- a/vdsm/sudoers.vdsm.in
+++ b/vdsm/sudoers.vdsm.in
@@ -40,7 +40,7 @@
     @CP_PATH@ * /var/log/vdsm/backup/* *, \
     @MULTIPATH_PATH@, \
     @BLOCKDEV_PATH@ --getsize64 *, \
-    @SETSID_PATH@ @IONICE_PATH@ -c? -n? @SU_PATH@ vdsm -s /bin/sh -c /usr/libexec/vdsm/spmprotect.sh*, \
+    @SETSID_PATH@ @IONICE_PATH@ -c ? -n ? @SU_PATH@ vdsm -s /bin/sh -c /usr/libexec/vdsm/spmprotect.sh*, \
     @SERVICE_PATH@ vdsmd *, \
     @REBOOT_PATH@ -f, \
     @PYTHON@ @VDSMDIR@/supervdsmServer.py* [a-z0-9\\-]* [0-9]*


--
To view, visit http://gerrit.ovirt.org/9042
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5abd61c03ce907ea94761f0209a4626669051eb1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi at redhat.com>


More information about the vdsm-patches mailing list