Federico Simoncelli has uploaded a new change for review.
Change subject: utils: add CommandStream class ......................................................................
utils: add CommandStream class
Change-Id: Ie015368bb9c5992e5c73a149277c59fc4ffbd570 Signed-off-by: Federico Simoncelli fsimonce@redhat.com --- M lib/vdsm/utils.py M tests/utilsTests.py 2 files changed, 188 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/33909/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index 244f0e2..4b0ccb7 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -313,6 +313,90 @@ timeout = max(0, endtime - time.time())
+class CommandStream(object): + def __init__(self, command, stdoutcb, stderrcb, cwd=None, + deathSignal=0): + self.command = command + self.returncode = None + + self.child = CPopen(self.command, cwd=cwd, close_fds=True, + deathSignal=deathSignal) + + self.epoll = select.epoll() + + self.iocb = { + self.child.stdout.fileno(): stdoutcb, + self.child.stderr.fileno(): stderrcb, + } + + for fd in self.iocb.keys(): + self.epoll.register(fd, select.EPOLLIN) + + def terminate(self): + self.child.terminate() + + def kill(self): + self.child.kill() + + def write(self, data): + self.child.stdin.write(data) + + def flush(self): + self.child.stdin.flush() + + def close(self): + self.child.stdin.close() + + def _epoll_input(self, fileno): + self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) + + def _epoll_event(self, fileno): + self.epoll.unregister(fileno) + del self.iocb[fileno] + + def _epoll_timeout(self, timeout): + fdevents = NoIntrPoll(self.epoll.poll, timeout) + + for fileno, event in fdevents: + if event & select.EPOLLIN: + self._epoll_input(fileno) + elif event & (select.EPOLLHUP | select.EPOLLERR): + self._epoll_event(fileno) + # Trying to collect the child status in case the + # file descriptor was closed because the process + # terminated. + self.returncode = self.child.poll() + + def wait(self, timeout=None): + if timeout is None: + epoll_remaining = -1 + else: + endtime = os.times()[4] + timeout + + while self.returncode is None: + if timeout is not None: + epoll_remaining = endtime - os.times()[4] + if epoll_remaining <= 0: + break + + if len(self.iocb): + self._epoll_timeout(epoll_remaining) + else: + # This is a busy-loop taken from issue5673, and + # python 3.4 still uses this implementation. + # A smarter solution would be using signalfd or + # sigtimedwait but they don't seem to mix well + # with multithreading (especially under heavy + # load: tens of children from tens of threads). + # Anyway we reach this only when both stdout and + # stderr are closed, which means that in most of + # the cases the child is about to die. + time.sleep(0.0005) + self.returncode = self.child.poll() + + return self.returncode + + class AsyncProc(object): """ AsyncProc is a funky class. It wraps a standard subprocess.Popen diff --git a/tests/utilsTests.py b/tests/utilsTests.py index 6e66c02..c279f04 100644 --- a/tests/utilsTests.py +++ b/tests/utilsTests.py @@ -22,8 +22,12 @@ import contextlib import errno import logging +import operator +import signal import sys import threading + +from contextlib import contextmanager
from testlib import VdsmTestCase as TestCaseBase from testlib import permutations, expandPermutations @@ -634,3 +638,103 @@
def test_empty(self): self.assertEquals(utils._list2cmdline([]), "") + + +class CommandStreamTests(TestCaseBase): + + @contextmanager + def assertElapsed(self, limit): + start = os.times()[4] + + yield + + elapsed = os.times()[4] - start + + if elapsed < limit: + raise AssertionError("Operation time: %s" % elapsed) + + def assertNoOutput(self, data): + raise AssertionError("Unexpected data: " + repr(data)) + + def test_output(self): + text = "Hello World" + received = bytearray() + + def recv_stdout(buffer): + # cannot use received += buffer with a variable + # defined in the parent function. + operator.iadd(received, buffer) + + p = utils.CommandStream(["echo", "-n", text], + recv_stdout, + self.assertNoOutput) + + retcode = p.wait() + + self.assertEqual(retcode, 0) + self.assertEqual(text, str(received)) + + def test_write(self): + text = "Hello World" + received = bytearray() + + def recv_stdout(buffer): + # cannot use received += buffer with a variable + # defined in the parent function. + operator.iadd(received, buffer) + + p = utils.CommandStream(["cat"], recv_stdout, + self.assertNoOutput) + + p.write(text) + p.flush() + p.close() + + retcode = p.wait() + + self.assertEqual(retcode, 0) + self.assertEqual(text, str(received)) + + def test_timeout(self): + p = utils.CommandStream(["sleep", "3"], + self.assertNoOutput, + self.assertNoOutput) + + with self.assertElapsed(2): + retcode = p.wait(2) + + self.assertEqual(retcode, None) + + retcode = p.wait() + self.assertEqual(retcode, 0) + + def test_terminate(self): + p = utils.CommandStream(["sleep", "2"], + self.assertNoOutput, + self.assertNoOutput) + + p.terminate() + + retcode = p.wait() + self.assertEqual(retcode, -signal.SIGTERM) + + def test_kill(self): + p = utils.CommandStream(["sleep", "2"], + self.assertNoOutput, + self.assertNoOutput) + + p.kill() + + retcode = p.wait() + self.assertEqual(retcode, -signal.SIGKILL) + + def test_early_close(self): + p = utils.CommandStream(["bash", "-c", + "exec 1>&-; exec 2>&-; exec sleep 2"], + self.assertNoOutput, + self.assertNoOutput) + + with self.assertElapsed(2): + retcode = p.wait() + + self.assertEqual(retcode, 0)
Federico Simoncelli has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/33909/1/tests/utilsTests.py File tests/utilsTests.py:
Line 736: Line 737: with self.assertElapsed(2): Line 738: retcode = p.wait() Line 739: Line 740: self.assertEqual(retcode, 0) Add test_early_close_timeout
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12827/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11879/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12670/ : SUCCESS
Federico Simoncelli has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/33909/2//COMMIT_MSG Commit Message:
Line 4: Commit: Federico Simoncelli fsimonce@redhat.com Line 5: CommitDate: 2014-11-13 12:37:07 +0000 Line 6: Line 7: utils: add CommandStream class Line 8: Still a draft. Commit message to come. Line 9: Change-Id: Ie015368bb9c5992e5c73a149277c59fc4ffbd570
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/13388/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/12598/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created_staging/164/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/13550/ : FAILURE
Francesco Romani has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/33909/2//COMMIT_MSG Commit Message:
Line 4: Commit: Federico Simoncelli fsimonce@redhat.com Line 5: CommitDate: 2014-11-13 12:37:07 +0000 Line 6: Line 7: utils: add CommandStream class Line 8:
Still a draft. Commit message to come.
Waiting for it because I'd like to know more about the rationale and the benefits of this change. Line 9: Change-Id: Ie015368bb9c5992e5c73a149277c59fc4ffbd570
Francesco Romani has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 2:
(9 comments)
preliminary review - easy parts first
http://gerrit.ovirt.org/#/c/33909/2/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 327: if endtime is not None: Line 328: timeout = max(0, endtime - time.time()) Line 329: Line 330: Line 331: class CommandStream(object): A docstring would be helpful, mostly to understand why and when use this class, comparing for example with AsyncProc Line 332: def __init__(self, command, stdoutcb, stderrcb, cwd=None, Line 333: deathSignal=0): Line 334: self.command = command Line 335: self.returncode = None
Line 330: Line 331: class CommandStream(object): Line 332: def __init__(self, command, stdoutcb, stderrcb, cwd=None, Line 333: deathSignal=0): Line 334: self.command = command all the members could be made _private Line 335: self.returncode = None Line 336: Line 337: self.child = CPopen(self.command, cwd=cwd, close_fds=True, Line 338: deathSignal=deathSignal)
Line 343: self.child.stdout.fileno(): stdoutcb, Line 344: self.child.stderr.fileno(): stderrcb, Line 345: } Line 346: Line 347: for fd in self.iocb.keys(): why not for fd in self.iocb ? Line 348: self.epoll.register(fd, select.EPOLLIN) Line 349: Line 350: def terminate(self): Line 351: self.child.terminate()
Line 381: # file descriptor was closed because the process Line 382: # terminated. Line 383: self.returncode = self.child.poll() Line 384: Line 385: def wait(self, timeout=None): despite your explanation (thanks for that) I still find the 'timeout' usage a bit confusing Line 386: if timeout is None: Line 387: epoll_remaining = -1 Line 388: else: Line 389: endtime = os.times()[4] + timeout
Line 385: def wait(self, timeout=None): Line 386: if timeout is None: Line 387: epoll_remaining = -1 Line 388: else: Line 389: endtime = os.times()[4] + timeout usage of os.times() could be surprising (I must admit it was like this for me), better to extract a tiny helper function for the sake of readability like mine http://gerrit.ovirt.org/#/c/35350/ - just consider it as example of what I mean Line 390: Line 391: while self.returncode is None: Line 392: if timeout is not None: Line 393: epoll_remaining = endtime - os.times()[4]
Line 393: epoll_remaining = endtime - os.times()[4] Line 394: if epoll_remaining <= 0: Line 395: break Line 396: Line 397: if len(self.iocb): why not if self.iocb ? Line 398: self._epoll_timeout(epoll_remaining) Line 399: else: Line 400: # This is a busy-loop taken from issue5673, and Line 401: # python 3.4 still uses this implementation.
http://gerrit.ovirt.org/#/c/33909/2/tests/utilsTests.py File tests/utilsTests.py:
Line 639: def test_empty(self): Line 640: self.assertEquals(utils._list2cmdline([]), "") Line 641: Line 642: Line 643: class CommandStreamTests(TestCaseBase): tests are nice, but for the sake of readability I'd suggest to put short summaries for each of them as docstrings. Line 644: Line 645: @contextmanager Line 646: def assertElapsed(self, expected, tolerance=0.5): Line 647: start = os.times()[4]
Line 663: Line 664: def recv_stdout(buffer): Line 665: # cannot use received += buffer with a variable Line 666: # defined in the parent function. Line 667: operator.iadd(received, buffer) what about the bytearray.extend() method? Line 668: Line 669: p = utils.CommandStream(["echo", "-n", text], Line 670: recv_stdout, Line 671: self.assertNoOutput)
Line 701: self.assertNoOutput, Line 702: self.assertNoOutput) Line 703: Line 704: with self.assertElapsed(2): Line 705: retcode = p.wait(2) this indeed suggests the wait() timeout parameter means "wait at least X time units" which may be misleading Line 706: Line 707: self.assertEqual(retcode, None) Line 708: Line 709: p.terminate()
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 2:
(12 comments)
I like the direction.
http://gerrit.ovirt.org/#/c/33909/2/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 327: if endtime is not None: Line 328: timeout = max(0, endtime - time.time()) Line 329: Line 330: Line 331: class CommandStream(object): The name is not very good, as this does not have a stream like interface. Maybe just "Command"?
And we should move this out of utils to the commands module. Line 332: def __init__(self, command, stdoutcb, stderrcb, cwd=None, Line 333: deathSignal=0): Line 334: self.command = command Line 335: self.returncode = None
Line 330: Line 331: class CommandStream(object): Line 332: def __init__(self, command, stdoutcb, stderrcb, cwd=None, Line 333: deathSignal=0): Line 334: self.command = command
all the members could be made _private
+1 - if not everyone will start to access these.
Do we need to keep self.command? Line 335: self.returncode = None Line 336: Line 337: self.child = CPopen(self.command, cwd=cwd, close_fds=True, Line 338: deathSignal=deathSignal)
Line 331: class CommandStream(object): Line 332: def __init__(self, command, stdoutcb, stderrcb, cwd=None, Line 333: deathSignal=0): Line 334: self.command = command Line 335: self.returncode = None Why do we need self.returncode? we can have a property returning self.child.returncode. Line 336: Line 337: self.child = CPopen(self.command, cwd=cwd, close_fds=True, Line 338: deathSignal=deathSignal) Line 339:
Line 336: Line 337: self.child = CPopen(self.command, cwd=cwd, close_fds=True, Line 338: deathSignal=deathSignal) Line 339: Line 340: self.epoll = select.epoll() I don't see a need for epoll. We are polling on 2 file descriptors, why do we need epoll for that? Line 341: Line 342: self.iocb = { Line 343: self.child.stdout.fileno(): stdoutcb, Line 344: self.child.stderr.fileno(): stderrcb,
Line 359: def flush(self): Line 360: self.child.stdin.flush() Line 361: Line 362: def close(self): Line 363: self.child.stdin.close() Both flush() and close() are not clear for this class. Accessing stdin directly is better. Line 364: Line 365: def _epoll_input(self, fileno): Line 366: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 367:
Line 361: Line 362: def close(self): Line 363: self.child.stdin.close() Line 364: Line 365: def _epoll_input(self, fileno): I don't understand this name - maybe _call_iocb()? Line 366: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 367: Line 368: def _epoll_event(self, fileno): Line 369: self.epoll.unregister(fileno)
Line 364: Line 365: def _epoll_input(self, fileno): Line 366: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 367: Line 368: def _epoll_event(self, fileno): This name is also strange - maybe _unregister_iocb()? Line 369: self.epoll.unregister(fileno) Line 370: del self.iocb[fileno] Line 371: Line 372: def _epoll_timeout(self, timeout):
Line 368: def _epoll_event(self, fileno): Line 369: self.epoll.unregister(fileno) Line 370: del self.iocb[fileno] Line 371: Line 372: def _epoll_timeout(self, timeout): This should be something like _wait_for_io() Line 373: fdevents = NoIntrPoll(self.epoll.poll, timeout) Line 374: Line 375: for fileno, event in fdevents: Line 376: if event & select.EPOLLIN:
Line 379: self._epoll_event(fileno) Line 380: # Trying to collect the child status in case the Line 381: # file descriptor was closed because the process Line 382: # terminated. Line 383: self.returncode = self.child.poll() We don't need to keep the return code, as the poll() update the child.returncode. Line 384: Line 385: def wait(self, timeout=None): Line 386: if timeout is None: Line 387: epoll_remaining = -1
Line 385: def wait(self, timeout=None): Line 386: if timeout is None: Line 387: epoll_remaining = -1 Line 388: else: Line 389: endtime = os.times()[4] + timeout
usage of os.times() could be surprising (I must admit it was like this for
+1 Line 390: Line 391: while self.returncode is None: Line 392: if timeout is not None: Line 393: epoll_remaining = endtime - os.times()[4]
Line 387: epoll_remaining = -1 Line 388: else: Line 389: endtime = os.times()[4] + timeout Line 390: Line 391: while self.returncode is None: self.child.returncode? Line 392: if timeout is not None: Line 393: epoll_remaining = endtime - os.times()[4] Line 394: if epoll_remaining <= 0: Line 395: break
Line 405: # load: tens of children from tens of threads). Line 406: # Anyway we reach this only when both stdout and Line 407: # stderr are closed, which means that in most of Line 408: # the cases the child is about to die. Line 409: time.sleep(0.0005) We should not do this.
The proper way to wait is to wait on a condition notified by SIGCHLD signal handler. See http://gerrit.ovirt.org/35055/
If you don't want to depend on it now, I think using 1 second wait between polls is good enough. Line 410: self.returncode = self.child.poll() Line 411: Line 412: return self.returncode Line 413:
Federico Simoncelli has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 2:
(2 comments)
http://gerrit.ovirt.org/#/c/33909/2/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 343: self.child.stdout.fileno(): stdoutcb, Line 344: self.child.stderr.fileno(): stderrcb, Line 345: } Line 346: Line 347: for fd in self.iocb.keys():
why not for fd in self.iocb ?
Done Line 348: self.epoll.register(fd, select.EPOLLIN) Line 349: Line 350: def terminate(self): Line 351: self.child.terminate()
Line 385: def wait(self, timeout=None): Line 386: if timeout is None: Line 387: epoll_remaining = -1 Line 388: else: Line 389: endtime = os.times()[4] + timeout
+1
Done Line 390: Line 391: while self.returncode is None: Line 392: if timeout is not None: Line 393: epoll_remaining = endtime - os.times()[4]
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 3:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/13568/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/14525/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/14357/ : SUCCESS
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 3:
(7 comments)
http://gerrit.ovirt.org/#/c/33909/3/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 331: Line 332: class CommandStream(object): Line 333: def __init__(self, command, stdoutcb, stderrcb): Line 334: self._command = command Line 335: self.epoll = select.epoll() Should be private. Line 336: Line 337: self.iocb = { Line 338: self._command.stdout.fileno(): stdoutcb, Line 339: self._command.stderr.fileno(): stderrcb,
Line 333: def __init__(self, command, stdoutcb, stderrcb): Line 334: self._command = command Line 335: self.epoll = select.epoll() Line 336: Line 337: self.iocb = { Same - private Line 338: self._command.stdout.fileno(): stdoutcb, Line 339: self._command.stderr.fileno(): stderrcb, Line 340: } Line 341:
Line 348: def _epoll_event(self, fileno): Line 349: self.epoll.unregister(fileno) Line 350: del self.iocb[fileno] Line 351: Line 352: def _epoll_timeout(self, timeout): Rename to _poll? Line 353: fdevents = NoIntrPoll(self.epoll.poll, timeout) Line 354: Line 355: for fileno, event in fdevents: Line 356: if event & select.EPOLLIN:
Line 363: return len(self.iocb) == 0 Line 364: Line 365: def receive(self, timeout=None): Line 366: if timeout is None: Line 367: epoll_remaining = -1 We can do this in the loop bellow instead. Line 368: else: Line 369: endtime = monotonic_time() + timeout Line 370: Line 371: while not self.closed:
Line 365: def receive(self, timeout=None): Line 366: if timeout is None: Line 367: epoll_remaining = -1 Line 368: else: Line 369: endtime = monotonic_time() + timeout I like to call this "deadline", in case you are not attached to "endtime". Line 370: Line 371: while not self.closed: Line 372: if timeout is not None: Line 373: epoll_remaining = endtime - monotonic_time()
Line 368: else: Line 369: endtime = monotonic_time() + timeout Line 370: Line 371: while not self.closed: Line 372: if timeout is not None: It is little ugly to check every time for timeout is None. Line 373: epoll_remaining = endtime - monotonic_time() Line 374: if epoll_remaining <= 0: Line 375: break Line 376:
Line 371: while not self.closed: Line 372: if timeout is not None: Line 373: epoll_remaining = endtime - monotonic_time() Line 374: if epoll_remaining <= 0: Line 375: break But since we do check, lets put the -1 here?
else: epoll_remaining = -1
We can simplify more if we use some big default timeout:
def receive(self, timeout=sys.maxint): now = monotonic_time() deadline = now + timeout while not self.closed and now < deadline: self._poll(deadline - now) now = monotonic_time() Line 376: Line 377: self._epoll_timeout(epoll_remaining) Line 378: Line 379:
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 3:
(18 comments)
http://gerrit.ovirt.org/#/c/33909/3//COMMIT_MSG Commit Message:
Line 4: Commit: Federico Simoncelli fsimonce@redhat.com Line 5: CommitDate: 2014-12-17 21:53:58 +0000 Line 6: Line 7: utils: add CommandStream class Line 8: Can we have some information on why this class is needed? Line 9: Change-Id: Ie015368bb9c5992e5c73a149277c59fc4ffbd570
http://gerrit.ovirt.org/#/c/33909/3/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 328: if endtime is not None: Line 329: timeout = max(0, endtime - time.time()) Line 330: Line 331: Line 332: class CommandStream(object): Looking at the tests, the only interesting public method is receive() - this does not look like a stream. How about naming it CommandReceiver or StreamReceiver? Line 333: def __init__(self, command, stdoutcb, stderrcb): Line 334: self._command = command Line 335: self.epoll = select.epoll() Line 336:
Line 335: self.epoll = select.epoll() Line 336: Line 337: self.iocb = { Line 338: self._command.stdout.fileno(): stdoutcb, Line 339: self._command.stderr.fileno(): stderrcb, Will this work when we redirect stderr to stdout (do we have same fileno for both files?) Line 340: } Line 341: Line 342: for fd in self.iocb: Line 343: self.epoll.register(fd, select.EPOLLIN)
Line 341: Line 342: for fd in self.iocb: Line 343: self.epoll.register(fd, select.EPOLLIN) Line 344: Line 345: def _epoll_input(self, fileno): I commented about this name in a previous version. Line 346: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 347: Line 348: def _epoll_event(self, fileno): Line 349: self.epoll.unregister(fileno)
Line 344: Line 345: def _epoll_input(self, fileno): Line 346: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 347: Line 348: def _epoll_event(self, fileno): I commented about this name in a previous version. Line 349: self.epoll.unregister(fileno) Line 350: del self.iocb[fileno] Line 351: Line 352: def _epoll_timeout(self, timeout):
http://gerrit.ovirt.org/#/c/33909/3/tests/utilsTests.py File tests/utilsTests.py:
Line 679: class CommandStreamTests(TestCaseBase): Line 680: Line 681: @contextmanager Line 682: def assertElapsed(self, expected, tolerance=0.5): Line 683: start = os.times()[4] Should use new utils.monotonic_time() Line 684: Line 685: yield Line 686: Line 687: elapsed = os.times()[4] - start
Line 686: Line 687: elapsed = os.times()[4] - start Line 688: Line 689: if (elapsed < expected - tolerance Line 690: or elapsed > expected + tolerance): How about:
if abs(elapsed - expected) > tolerance: raise ... Line 691: raise AssertionError("Operation time: %s" % elapsed) Line 692: Line 693: def assertNoOutput(self, data): Line 694: raise AssertionError("Unexpected data: " + repr(data))
Line 689: if (elapsed < expected - tolerance Line 690: or elapsed > expected + tolerance): Line 691: raise AssertionError("Operation time: %s" % elapsed) Line 692: Line 693: def assertNoOutput(self, data): This does not assert anything about output or data. Maybe call this assertUnexpectedCall? Line 694: raise AssertionError("Unexpected data: " + repr(data)) Line 695: Line 696: def _command(self, command): Line 697: return CPopen(command)
Line 690: or elapsed > expected + tolerance): Line 691: raise AssertionError("Operation time: %s" % elapsed) Line 692: Line 693: def assertNoOutput(self, data): Line 694: raise AssertionError("Unexpected data: " + repr(data)) And change this to "Unexpected call with data: %r" % data Line 695: Line 696: def _command(self, command): Line 697: return CPopen(command) Line 698:
Line 692: Line 693: def assertNoOutput(self, data): Line 694: raise AssertionError("Unexpected data: " + repr(data)) Line 695: Line 696: def _command(self, command): Rename to _startCommand? Line 697: return CPopen(command) Line 698: Line 699: def test_output(self): Line 700: text = "Hello World"
Line 695: Line 696: def _command(self, command): Line 697: return CPopen(command) Line 698: Line 699: def test_output(self): Rename to test_receive_stdout? Line 700: text = "Hello World" Line 701: received = bytearray() Line 702: Line 703: def recv_stdout(buffer):
Line 696: def _command(self, command): Line 697: return CPopen(command) Line 698: Line 699: def test_output(self): Line 700: text = "Hello World" We need another tests for real world output, maybe many lines (/usr/bin/yes?) Line 701: received = bytearray() Line 702: Line 703: def recv_stdout(buffer): Line 704: # cannot use received += buffer with a variable
Line 702: Line 703: def recv_stdout(buffer): Line 704: # cannot use received += buffer with a variable Line 705: # defined in the parent function. Line 706: operator.iadd(received, buffer) Too complicated.
Why not:
self.received = bytearray()
def recv_stdout(buffer): self.received += buffer Line 707: Line 708: c = self._command(["echo", "-n", text]) Line 709: p = utils.CommandStream(c, recv_stdout, self.assertNoOutput) Line 710:
Line 711: p.receive() Line 712: retcode = c.wait() Line 713: Line 714: self.assertEqual(retcode, 0) Line 715: self.assertEqual(text, str(received)) Works without converting the str. If you want to be more explicit, we can use text = bytes("Hello World") or b"Hello World". Line 716: Line 717: def test_write(self): Line 718: text = "Hello World" Line 719: received = bytearray()
Line 720: Line 721: def recv_stdout(buffer): Line 722: # cannot use received += buffer with a variable Line 723: # defined in the parent function. Line 724: operator.iadd(received, buffer) Same as above, but this time it is also duplicated :-) Line 725: Line 726: c = self._command(["cat"]) Line 727: p = utils.CommandStream(c, recv_stdout, self.assertNoOutput) Line 728:
Line 730: c.stdin.flush() Line 731: c.stdin.close() Line 732: Line 733: while not p.closed: Line 734: p.receive() Why do we need here a loop and we don't need one in test_output? Line 735: Line 736: retcode = c.wait() Line 737: Line 738: self.assertEqual(retcode, 0)
Line 741: def test_timeout(self): Line 742: c = self._command(["sleep", "5"]) Line 743: p = utils.CommandStream(c, self.assertNoOutput, self.assertNoOutput) Line 744: Line 745: with self.assertElapsed(2): If this assert fails, we never wait for the process, keeping a zombie until the test process exit. Line 746: p.receive(2) Line 747: Line 748: self.assertEqual(p.closed, False) Line 749:
Line 770: Line 771: with self.assertElapsed(0): Line 772: p.receive(2) Line 773: Line 774: self.assertEqual(c.wait(), -signal.SIGKILL) We need more tests:
- Receiving from both stdout and stderr - Redirecting stderr to stdout - Interacting with a command (shell?), writing commands to stdin, receiving output and errors from process - stdout closed while receiving, continue to receive from stderr - both stdout and stderr closed while receiving - read from stdout fails during receive - how the caller can handle this error? maybe we need an errorcb?
Federico Simoncelli has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 3:
(5 comments)
http://gerrit.ovirt.org/#/c/33909/3/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 335: self.epoll = select.epoll() Line 336: Line 337: self.iocb = { Line 338: self._command.stdout.fileno(): stdoutcb, Line 339: self._command.stderr.fileno(): stderrcb,
Will this work when we redirect stderr to stdout (do we have same fileno fo
Yes. Currently everything is squashed to the stderrcb, I'd better invert them and add a comment. Line 340: } Line 341: Line 342: for fd in self.iocb: Line 343: self.epoll.register(fd, select.EPOLLIN)
Line 341: Line 342: for fd in self.iocb: Line 343: self.epoll.register(fd, select.EPOLLIN) Line 344: Line 345: def _epoll_input(self, fileno):
I commented about this name in a previous version.
This method is handling the poll input event, the name seems consistent. Line 346: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 347: Line 348: def _epoll_event(self, fileno): Line 349: self.epoll.unregister(fileno)
Line 344: Line 345: def _epoll_input(self, fileno): Line 346: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 347: Line 348: def _epoll_event(self, fileno):
I commented about this name in a previous version.
This method is handling the poll events, the name seems consistent. Line 349: self.epoll.unregister(fileno) Line 350: del self.iocb[fileno] Line 351: Line 352: def _epoll_timeout(self, timeout):
Line 348: def _epoll_event(self, fileno): Line 349: self.epoll.unregister(fileno) Line 350: del self.iocb[fileno] Line 351: Line 352: def _epoll_timeout(self, timeout):
Rename to _poll?
This method is polling with timeout, the name seems consistent. Line 353: fdevents = NoIntrPoll(self.epoll.poll, timeout) Line 354: Line 355: for fileno, event in fdevents: Line 356: if event & select.EPOLLIN:
Line 363: return len(self.iocb) == 0 Line 364: Line 365: def receive(self, timeout=None): Line 366: if timeout is None: Line 367: epoll_remaining = -1
We can do this in the loop bellow instead.
If we do it in the loop it will be set at every cycle unless we rely on some implicit python optimization. Line 368: else: Line 369: endtime = monotonic_time() + timeout Line 370: Line 371: while not self.closed:
automation@ovirt.org has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 4:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15357/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/15188/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14400/ : FAILURE
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 3:
(2 comments)
http://gerrit.ovirt.org/#/c/33909/3/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 341: Line 342: for fd in self.iocb: Line 343: self.epoll.register(fd, select.EPOLLIN) Line 344: Line 345: def _epoll_input(self, fileno):
This method is handling the poll input event, the name seems consistent.
These names are consistent, but they are not good method names. They do not describe what the method does, and do not help readability.
The method that does polling is not the same thing as the method that handle certain event, so their names should not be "consistent".
Having epoll in the name adds no value, because we don't have several method of polling, one with epoll and one without another type. It would be better if we can replace epoll with poll without modifying these names.
I would expect to have names like:
- _poll - wait for events. This is typically done with a timeout, and we don't have another method for polling without a timeouts, so the _timeout suffix is not needed. - _handle_input - handle input from a watched fd - _handle_close - handle closed fd Line 346: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 347: Line 348: def _epoll_event(self, fileno): Line 349: self.epoll.unregister(fileno)
Line 363: return len(self.iocb) == 0 Line 364: Line 365: def receive(self, timeout=None): Line 366: if timeout is None: Line 367: epoll_remaining = -1
If we do it in the loop it will be set at every cycle unless we rely on som
I'm not worried about setting this each iteration. Line 368: else: Line 369: endtime = monotonic_time() + timeout Line 370: Line 371: while not self.closed:
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 4:
(2 comments)
http://gerrit.ovirt.org/#/c/33909/4/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 356: # entries in the dictionary) Line 357: self._iocb = { Line 358: self._command.stderr.fileno(): stderrcb, Line 359: self._command.stdout.fileno(): stdoutcb, Line 360: } This practically works, but I'm not sure the behavior is defined. It would be better to have a method to add a descriptor and invoke it in a well defined order, with the comment explaining the correct order. Line 361: Line 362: for fd in self._iocb: Line 363: self._poll.register(fd, select.EPOLLIN) Line 364:
Line 362: for fd in self._iocb: Line 363: self._poll.register(fd, select.EPOLLIN) Line 364: Line 365: def _poll_input(self, fileno): Line 366: self._iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) We don't set the descriptors to non-blocking mode, so this may block when handling an event.
We should set the descriptors to non-blocking when command is given, or maybe dup them and set the dup as non-blocking. Then we should handle EAGAIN here.
Another issue is handling errors - if os.read raises, the caller should get the exception and handle it. We can have an errorcb called when os.read() fails with the fd and the exception.
Or if we choose to let the caller of receive handle such errors, we should document the expected errors that received may raise.
Finally, if the caller wants to stop listening for events - how can you "close" this stream? Line 367: Line 368: def _poll_event(self, fileno): Line 369: self._poll.unregister(fileno) Line 370: del self._iocb[fileno]
Federico Simoncelli has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/33909/4/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 362: for fd in self._iocb: Line 363: self._poll.register(fd, select.EPOLLIN) Line 364: Line 365: def _poll_input(self, fileno): Line 366: self._iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE))
We don't set the descriptors to non-blocking mode, so this may block when h
If you have signal handlers that potentially take a long time to execute you have much more serious problems than CommandStream.
I don't plan to make this non-blocking out of the box (the consumer can do that on his own since CommandStream is fully composable with a Popen object).
Anyway it is obviously useful to document that receive may raise the same exceptions of read.
At the moment the only interesting use-case is to stop the command (and not stop listening for events) which is accomplished by the Popen method terminate/kill.
That will cause the process to exit and the file-descriptors to be closed. Line 367: Line 368: def _poll_event(self, fileno): Line 369: self._poll.unregister(fileno) Line 370: del self._iocb[fileno]
automation@ovirt.org has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 5:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16807/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/16979/ : SUCCESS
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/33909/4/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 362: for fd in self._iocb: Line 363: self._poll.register(fd, select.EPOLLIN) Line 364: Line 365: def _poll_input(self, fileno): Line 366: self._iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE))
If you have signal handlers that potentially take a long time to execute yo
It seems that fd spuriously reported as ready is an issue only with select and no such issue is documented for poll or epoll, so this usage should be safe.
Since we don't need currently to stop watching a running command, it is good enough to terminate the command. Line 367: Line 368: def _poll_event(self, fileno): Line 369: self._poll.unregister(fileno) Line 370: del self._iocb[fileno]
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 5:
Would you like to check my comments on the tests in version 3? https://gerrit.ovirt.org/#/c/33909/3..5/tests/utilsTests.py,unified
automation@ovirt.org has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 6:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 6:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16903/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 6:
Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17075/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 6:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16903/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17075/ : SUCCESS
automation@ovirt.org has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 7:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 7:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18265/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 7:
Build Started (2/2)
0 -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1495/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 7:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18265/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1495/ : 0
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 7:
(2 comments)
https://gerrit.ovirt.org/#/c/33909/7/tests/utilsTests.py File tests/utilsTests.py:
Line 770: Line 771: def _startCommand(self, command): Line 772: return cpopen.CPopen(command) Line 773: Line 774: @permutations((('stdout',), ('stderr',))) Lets put the command and cb spec here:
@permutations([ # command, recv_out, recv_err (["echo", "-n", "%s"], True, False), (["sh", "-c", 'echo -n "%s" >&2'], False, True), ]) def test_receive(self, cmd, recv_out, recv_err):
So the code is more generic, and we don't need error handling for unsupported output:
cmd[-1] = cmd[-1] % text
c = self._startCommand(cmd)
p = utils.CommandStream(c, recv_data if recv_out else self.assertUnexpectedCall, recv_data if recv_err else self.assertUnexpectedCall)
And later we can easily add more tests, for example, receiving multiple lines, large amount of data, no data, without changing the code. Line 775: def test_receive(self, output): Line 776: text = bytes("Hello World") Line 777: received = bytearray() Line 778:
Line 797: Line 798: self.assertEqual(retcode, 0) Line 799: self.assertEqual(text, received) Line 800: Line 801: @permutations((('stdout',), ('stderr',))) Same as for test_receive Line 802: def test_write(self, output): Line 803: text = "Hello World" Line 804: received = bytearray() Line 805:
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 7:
(4 comments)
https://gerrit.ovirt.org/#/c/33909/7/tests/utilsTests.py File tests/utilsTests.py:
Line 754: @expandPermutations Line 755: class CommandStreamTests(TestCaseBase): Line 756: Line 757: @contextlib.contextmanager Line 758: def assertElapsed(self, expected, tolerance=0.5): This seems like a general utility that should be in testlib. Line 759: start = utils.monotonic_time() Line 760: Line 761: yield Line 762:
Line 773: Line 774: @permutations((('stdout',), ('stderr',))) Line 775: def test_receive(self, output): Line 776: text = bytes("Hello World") Line 777: received = bytearray() We can use
received = [bytearray()]
or
self.received = bytearray() Line 778: Line 779: def recv_stdout(buffer): Line 780: # cannot use received += buffer with a variable Line 781: # defined in the parent function.
Line 775: def test_receive(self, output): Line 776: text = bytes("Hello World") Line 777: received = bytearray() Line 778: Line 779: def recv_stdout(buffer): This should be now recv_data - can come from stderr in the stderr permutation. Line 780: # cannot use received += buffer with a variable Line 781: # defined in the parent function. Line 782: operator.iadd(received, buffer) Line 783:
Line 778: Line 779: def recv_stdout(buffer): Line 780: # cannot use received += buffer with a variable Line 781: # defined in the parent function. Line 782: operator.iadd(received, buffer) But we can use received[0] += buffer, or self.received += bufffer Line 783: Line 784: if output == 'stdout': Line 785: c = self._startCommand(["echo", "-n", text]) Line 786: p = utils.CommandStream(c, recv_stdout, self.assertUnexpectedCall)
automation@ovirt.org has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 8:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 8:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18440/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 8:
Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1671/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 8: Code-Review-1 Verified-1
Build Unstable
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18440/ : UNSTABLE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1671/ : SUCCESS
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 8: Code-Review+1
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 8:
(2 comments)
https://gerrit.ovirt.org/#/c/33909/8/tests/utilsTests.py File tests/utilsTests.py:
Line 777: Line 778: c = self._startCommand(cmd) Line 779: p = utils.CommandStream(c, Line 780: recv_data if recv_out else self.assertUnexpectedCall, Line 781: recv_data if recv_err else self.assertUnexpectedCall) pep8 does not like the indentation here:
tests/utilsTests.py:780:13: E128 continuation line under-indented for visual indent tests/utilsTests.py:781:13: E128 continuation line under-indented for visual indent Line 782: Line 783: while not p.closed: Line 784: p.receive() Line 785:
Line 803: Line 804: c = self._startCommand(cmd) Line 805: p = utils.CommandStream(c, Line 806: recv_data if recv_out else self.assertUnexpectedCall, Line 807: recv_data if recv_err else self.assertUnexpectedCall) pep8 does not like the indentation here:
tests/utilsTests.py:806:13: E128 continuation line under-indented for visual indent tests/utilsTests.py:807:13: E128 continuation line under-indented for visual indent Line 808: Line 809: c.stdin.write(text) Line 810: c.stdin.flush() Line 811: c.stdin.close()
automation@ovirt.org has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 9:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 9: Code-Review+1
Adam Litke has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 9: Code-Review+2
Looks good.
automation@ovirt.org has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 10:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Adam Litke has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 10: Code-Review+2
Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 10: Code-Review+1
Federico Simoncelli has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 10: Verified+1
Dan Kenigsberg has submitted this change and it was merged.
Change subject: utils: add CommandStream class ......................................................................
utils: add CommandStream class
This patch adds CommandStream a generic class to process the output of external commands (both stdout and stderr) as soon as available. For this purpose CommandStream provides the ability to define callbacks that will be executed when data is available from the external command.
This class is the foundation for long-running commands that are able to provide their progress during execution: their output must be processed as soon as possible to maintain and report status updates.
Change-Id: Ie015368bb9c5992e5c73a149277c59fc4ffbd570 Signed-off-by: Federico Simoncelli fsimonce@redhat.com Reviewed-on: https://gerrit.ovirt.org/33909 Reviewed-by: Adam Litke alitke@redhat.com Reviewed-by: Nir Soffer nsoffer@redhat.com --- M lib/vdsm/utils.py M tests/testlib.py M tests/utilsTests.py 3 files changed, 171 insertions(+), 0 deletions(-)
Approvals: Nir Soffer: Looks good to me, but someone else must approve Adam Litke: Looks good to me, approved Federico Simoncelli: Verified
automation@ovirt.org has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 11:
* Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org