Change in vdsm[master]: v2v: Add execCmd() that accepts stdin/out/err
Francesco Romani has posted comments on this change. Change subject: v2v: Add execCmd() that accepts stdin/out/err .. Patch Set 2: (2 comments) https://gerrit.ovirt.org/#/c/62092/2//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2016-08-08 14:23:25 +0200 Line 4: Commit: Tomáš Golembiovský Line 5: CommitDate: 2016-08-09 16:21:43 +0200 Line 6: Line 7: v2v: Add execCmd() that accepts stdin/out/err > Tried that. It's too long. :) If we're not too strict about the 50 characte so let's bend the rules ;) v2v: add helper that redirects stdin/out/err Line 8: Line 9: There is no way to pass stdin, stdout and stderr streams to Popen via Line 10: execCmd. This will be useful in subsequent changes introducing logging Line 11: of virt-v2v output. Line 7: v2v: Add execCmd() that accepts stdin/out/err Line 8: Line 9: There is no way to pass stdin, stdout and stderr streams to Popen via Line 10: execCmd. This will be useful in subsequent changes introducing logging Line 11: of virt-v2v output. > Not really. IMO the only "feature" of execCmd we duplicate is the call to c even better! :) Line 12: Line 13: Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Line 14: Bug-Url: https://bugzilla.redhat.com/1350465 -- To view, visit https://gerrit.ovirt.org/62092 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: v2v: Add execCmd() that accepts stdin/out/err
Tomas Golembiovsky has posted comments on this change. Change subject: v2v: Add execCmd() that accepts stdin/out/err .. Patch Set 2: (2 comments) https://gerrit.ovirt.org/#/c/62092/2//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2016-08-08 14:23:25 +0200 Line 4: Commit: Tomáš Golembiovský Line 5: CommitDate: 2016-08-09 16:21:43 +0200 Line 6: Line 7: v2v: Add execCmd() that accepts stdin/out/err > what about: v2v: add simple_exec_cmd that... Tried that. It's too long. :) If we're not too strict about the 50 character limit on the short description I can change that. Line 8: Line 9: There is no way to pass stdin, stdout and stderr streams to Popen via Line 10: execCmd. This will be useful in subsequent changes introducing logging Line 11: of virt-v2v output. Line 7: v2v: Add execCmd() that accepts stdin/out/err Line 8: Line 9: There is no way to pass stdin, stdout and stderr streams to Popen via Line 10: execCmd. This will be useful in subsequent changes introducing logging Line 11: of virt-v2v output. > In the end we are duplicating execCmd. I don't have a better alternative, s Not really. IMO the only "feature" of execCmd we duplicate is the call to cmdutils.wrap_command(). Line 12: Line 13: Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Line 14: Bug-Url: https://bugzilla.redhat.com/1350465 -- To view, visit https://gerrit.ovirt.org/62092 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: v2v: Add execCmd() that accepts stdin/out/err
Shahar Havivi has posted comments on this change. Change subject: v2v: Add execCmd() that accepts stdin/out/err .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/62092 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: v2v: Add execCmd() that accepts stdin/out/err
Francesco Romani has posted comments on this change. Change subject: v2v: Add execCmd() that accepts stdin/out/err .. Patch Set 1: Code-Review+2 ok, good enough for me -- To view, visit https://gerrit.ovirt.org/62092 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: v2v: Add execCmd() that accepts stdin/out/err
gerrit-hooks has posted comments on this change. Change subject: v2v: Add execCmd() that accepts stdin/out/err .. Patch Set 1: * #1350465::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1350465::OK, public bug * Check Product::#1350465::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62092 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: v2v: Add execCmd() that accepts stdin/out/err
Tomas Golembiovsky has uploaded a new change for review. Change subject: v2v: Add execCmd() that accepts stdin/out/err .. v2v: Add execCmd() that accepts stdin/out/err There is no way to pass stdin, stdout and stderr streams to Popen via execCmd. This will be useful in subsequent changes introducing logging of virt-v2v output. Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Bug-Url: https://bugzilla.redhat.com/1350465 Signed-off-by: Tomáš Golembiovský --- M lib/vdsm/v2v.py M tests/v2vTests.py 2 files changed, 29 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/92/62092/1 diff --git a/lib/vdsm/v2v.py b/lib/vdsm/v2v.py index c6a9943..860768e 100644 --- a/lib/vdsm/v2v.py +++ b/lib/vdsm/v2v.py @@ -42,9 +42,10 @@ from vdsm.commands import execCmd from vdsm.common import zombiereaper +from vdsm.compat import CPopen from vdsm.constants import P_VDSM_RUN, EXT_KVM_2_OVIRT from vdsm.define import errCode, doneCode -from vdsm import libvirtconnection, response, concurrent +from vdsm import cmdutils, concurrent, libvirtconnection, response from vdsm.utils import traceback, CommandPath, NICENESS, IOCLASS try: @@ -1139,3 +1140,18 @@ else: net['type'] = 'interface' vm['networks'].append(net) + + +def _simple_exec_cmd(command, env=None, nice=None, ioclass=None, + stdin=None, stdout=None, stderr=None): + +command = cmdutils.wrap_command(command, with_ioclass=ioclass, +ioclassdata=None, with_nice=nice, +with_setsid=False, with_sudo=False, +reset_cpu_affinity=True) + +logging.debug(cmdutils.command_log_line(command, cwd=None)) + +p = CPopen(command, close_fds=True, cwd=None, env=env, + stdin=stdin, stdout=stdout, stderr=stderr) +return p diff --git a/tests/v2vTests.py b/tests/v2vTests.py index b7bfc54..3978035 100644 --- a/tests/v2vTests.py +++ b/tests/v2vTests.py @@ -20,6 +20,7 @@ from collections import namedtuple from contextlib import contextmanager from StringIO import StringIO +import subprocess import tarfile import zipfile import uuid @@ -559,6 +560,17 @@ self.assertEqual(job.status, v2v.STATUS.DONE) +def testSimpleExecCmd(self): +p = v2v._simple_exec_cmd(['cat'], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE) +msg = "test\ntest" +p.stdin.write(msg) +p.stdin.close() +p.wait() +out = p.stdout.read() +self.assertEqual(out, msg) + class MockVirConnectTests(TestCaseBase): def setUp(self): -- To view, visit https://gerrit.ovirt.org/62092 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
