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 <[email protected]> --- 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 <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
