Yaniv Bronhaim has uploaded a new change for review.
Change subject: Using startCmd func for running async execution
......................................................................
Using startCmd func for running async execution
Splitting execCmd and startCmd. execCmd still uses deathSignal and
asyncProc which can't run with py3.
Change-Id: Id38507a04a124918f3865cf011bbf5adc7bc31d5
Signed-off-by: Yaniv Bronhaim <ybron...@redhat.com>
---
M lib/vdsm/commands.py
M lib/vdsm/v2v.py
M tests/utilsTests.py
M vdsm/storage/imageSharing.py
4 files changed, 74 insertions(+), 53 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/37/60237/1
diff --git a/lib/vdsm/commands.py b/lib/vdsm/commands.py
index bffec83..6b734c7 100644
--- a/lib/vdsm/commands.py
+++ b/lib/vdsm/commands.py
@@ -41,15 +41,59 @@
def execCmd(command, sudo=False, cwd=None, data=None, raw=False,
- printable=None, env=None, sync=True, nice=None, ioclass=None,
+ printable=None, env=None, nice=None, ioclass=None,
ioclassdata=None, setsid=False, execCmdLogger=logging.root,
- deathSignal=0, resetCpuAffinity=True):
+ resetCpuAffinity=True):
+ """
+ Duplication of startCmd method without the use of deathSignal and sync
+ parameters. Using simple_exec_cmd will execute the command by popen
+ in sync to the call and return outputs.
+
+ This is partial work to keep the usage of asyncProc in v2v and
+ imageSharing.
+ """
+ command = cmdutils.wrap_command(command, with_ioclass=ioclass,
+ ioclassdata=ioclassdata, with_nice=nice,
+ with_setsid=setsid, with_sudo=sudo,
+ reset_cpu_affinity=resetCpuAffinity)
+
+ # Unsubscriptable objects (e.g. generators) need conversion
+ if not callable(getattr(command, '__getitem__', None)):
+ command = tuple(command)
+
+ if not printable:
+ printable = command
+
+ execCmdLogger.debug(cmdutils.command_log_line(printable, cwd=cwd))
+
+ p = CPopen(command, close_fds=True, cwd=cwd, env=env)
+
+ with terminating(p):
+ (out, err) = p.communicate(data)
+
+ if out is None:
+ # Prevent splitlines() from barfing later on
+ out = ""
+
+ execCmdLogger.debug(cmdutils.retcode_log_line(p.returncode, err=err))
+
+ if not raw:
+ out = out.splitlines(False)
+ err = err.splitlines(False)
+
+ return p.returncode, out, err
+
+
+def startCmd(command, sudo=False, cwd=None, data=None, raw=False,
+ printable=None, env=None, nice=None, ioclass=None,
+ ioclassdata=None, setsid=False, execCmdLogger=logging.root,
+ deathSignal=0, 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
+ a temporary thread, spawn a sub-process, and have the thread
finish, the new subprocess would die immediately.
"""
@@ -67,30 +111,13 @@
execCmdLogger.debug(cmdutils.command_log_line(printable, cwd=cwd))
- p = CPopen(command, close_fds=True, cwd=cwd, env=env,
- deathSignal=deathSignal)
- if not sync:
- p = AsyncProc(p)
- if data is not None:
- p.stdin.write(data)
- p.stdin.flush()
+ p = AsyncProc(CPopen(command, close_fds=True, cwd=cwd, env=env,
+ deathSignal=deathSignal))
+ if data is not None:
+ p.stdin.write(data)
+ p.stdin.flush()
- return p
-
- with terminating(p):
- (out, err) = p.communicate(data)
-
- if out is None:
- # Prevent splitlines() from barfing later on
- out = ""
-
- execCmdLogger.debug(cmdutils.retcode_log_line(p.returncode, err=err))
-
- if not raw:
- out = out.splitlines(False)
- err = err.splitlines(False)
-
- return p.returncode, out, err
+ return p
class AsyncProc(object):
@@ -341,9 +368,8 @@
"""
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)
+ proc = startCmd(command, cwd=cwd, data=data, nice=nice, ioclass=ioclass,
+ execCmdLogger=execCmdLogger, deathSignal=deathSignal)
if not proc.wait(cond=stop):
proc.kill()
diff --git a/lib/vdsm/v2v.py b/lib/vdsm/v2v.py
index defbe14..24c0415 100644
--- a/lib/vdsm/v2v.py
+++ b/lib/vdsm/v2v.py
@@ -40,7 +40,7 @@
import libvirt
-from vdsm.commands import execCmd
+from vdsm.commands import startCmd
from vdsm.constants import P_VDSM_RUN, EXT_KVM_2_OVIRT
from vdsm.define import errCode, doneCode
from vdsm import libvirtconnection, response, concurrent
@@ -389,12 +389,11 @@
raise NotImplementedError("Subclass must implement this")
def _start_helper(self):
- return execCmd(self._command(),
- sync=False,
- deathSignal=signal.SIGTERM,
- nice=NICENESS.HIGH,
- ioclass=IOCLASS.IDLE,
- env=self._environment())
+ return startCmd(self._command(),
+ deathSignal=signal.SIGTERM,
+ nice=NICENESS.HIGH,
+ ioclass=IOCLASS.IDLE,
+ env=self._environment())
def _get_disk_format(self):
fmt = self._vminfo.get('format', 'raw').lower()
diff --git a/tests/utilsTests.py b/tests/utilsTests.py
index f503502..c145bba 100644
--- a/tests/utilsTests.py
+++ b/tests/utilsTests.py
@@ -92,7 +92,7 @@
class TerminatingTests(TestCaseBase):
def setUp(self):
- self.proc = commands.execCmd([EXT_SLEEP, "2"], sync=False)
+ self.proc = commands.startCmd([EXT_SLEEP, "2"])
self.proc_path = "/proc/%d" % self.proc.pid
self.kill_proc = self.proc.kill
self.assertTrue(os.path.exists(self.proc_path))
@@ -226,7 +226,7 @@
@MonkeyPatch(cmdutils, "_USING_CPU_AFFINITY", False)
def test_without_affinity(self):
args = ["sleep", "3"]
- sproc = commands.execCmd(args, sync=False)
+ sproc = commands.startCmd(args)
stats = utils.pidStat(sproc.pid)
pid = int(stats.pid)
# procName comes in the format of (procname)
@@ -241,8 +241,7 @@
def test(self):
sleepProcs = []
for i in range(3):
- sleepProcs.append(commands.execCmd([EXT_SLEEP, "3"], sync=False,
- sudo=False))
+ sleepProcs.append(commands.startCmd([EXT_SLEEP, "3"], sudo=False))
pids = utils.pgrep(EXT_SLEEP)
for proc in sleepProcs:
@@ -257,7 +256,7 @@
class GetCmdArgsTests(TestCaseBase):
def test(self):
args = [EXT_SLEEP, "4"]
- sproc = commands.execCmd(args, sync=False)
+ sproc = commands.startCmd(args)
try:
cmd_args = utils.getCmdArgs(sproc.pid)
# let's ignore optional taskset at the beginning
@@ -269,7 +268,7 @@
def testZombie(self):
args = [EXT_SLEEP, "0"]
- sproc = commands.execCmd(args, sync=False)
+ sproc = commands.startCmd(args)
sproc.kill()
try:
test = lambda: self.assertEquals(utils.getCmdArgs(sproc.pid),
@@ -354,7 +353,7 @@
class AsyncProcessOperationTests(TestCaseBase):
def _echo(self, text):
- proc = commands.execCmd(["echo", "-n", "test"], sync=False)
+ proc = commands.startCmd(["echo", "-n", "test"])
def parse(rc, out, err):
return out
@@ -362,11 +361,11 @@
return utils.AsyncProcessOperation(proc, parse)
def _sleep(self, t):
- proc = commands.execCmd(["sleep", str(t)], sync=False)
+ proc = commands.startCmd(["sleep", str(t)])
return utils.AsyncProcessOperation(proc)
def _fail(self, t):
- proc = commands.execCmd(["sleep", str(t)], sync=False)
+ proc = commands.startCmd(["sleep", str(t)])
def parse(rc, out, err):
raise Exception("TEST!!!")
@@ -634,7 +633,7 @@
@MonkeyPatch(cmdutils, '_USING_CPU_AFFINITY', False)
def testResetAffinityByDefault(self):
try:
- proc = commands.execCmd((EXT_SLEEP, '30s'), sync=False)
+ proc = commands.startCmd((EXT_SLEEP, '30s'))
self.assertEquals(taskset.get(proc.pid),
taskset.get(os.getpid()))
@@ -648,7 +647,7 @@
self.assertEquals(taskset.get(os.getpid()), self.CPU_SET)
try:
- proc = commands.execCmd((EXT_SLEEP, '30s'), sync=False)
+ proc = commands.startCmd((EXT_SLEEP, '30s'))
self.assertEquals(taskset.get(proc.pid), online_cpus())
finally:
@@ -661,8 +660,7 @@
self.assertEquals(taskset.get(os.getpid()), self.CPU_SET)
try:
- proc = commands.execCmd((EXT_SLEEP, '30s'),
- sync=False,
+ proc = commands.startCmd((EXT_SLEEP, '30s'),
resetCpuAffinity=False)
self.assertEquals(taskset.get(proc.pid), self.CPU_SET)
diff --git a/vdsm/storage/imageSharing.py b/vdsm/storage/imageSharing.py
index b8c3240..18cbb37 100644
--- a/vdsm/storage/imageSharing.py
+++ b/vdsm/storage/imageSharing.py
@@ -77,8 +77,7 @@
totalSize = getLengthFromArgs(methodArgs)
fileObj = methodArgs['fileObj']
cmd = [constants.EXT_DD, "of=%s" % dstImgPath, "bs=%s" % constants.MEGAB]
- p = commands.execCmd(cmd, sudo=False, sync=False,
- deathSignal=signal.SIGKILL)
+ p = commands.startCmd(cmd, sudo=False, deathSignal=signal.SIGKILL)
try:
_copyData(fileObj, p.stdin, totalSize)
p.stdin.close()
@@ -103,8 +102,7 @@
cmd = [constants.EXT_DD, "if=%s" % dstImgPath, "bs=%s" % constants.MEGAB,
"count=%s" % (total_size / constants.MEGAB + 1)]
- p = commands.execCmd(cmd, sync=False,
- deathSignal=signal.SIGKILL)
+ p = commands.startCmd(cmd, deathSignal=signal.SIGKILL)
p.blocking = True
try:
_copyData(p.stdout, fileObj, bytes_left)
--
To view, visit https://gerrit.ovirt.org/60237
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id38507a04a124918f3865cf011bbf5adc7bc31d5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
