Change in vdsm[master]: draft for new asyncProc

nsoffer at redhat.com nsoffer at redhat.com
Sun Dec 6 13:02:14 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: draft for new asyncProc
......................................................................


Patch Set 1:

(7 comments)

https://gerrit.ovirt.org/#/c/49441/1/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 339:             raise
Line 340:         break
Line 341: 
Line 342: 
Line 343: class AsyncProc(object):
I would call it simply Popen - it should behave like Popen in python3, and used only when running on Python 2.

We should not try to provide features not provided by Pyhton 3 like we did with AsyncProc.
Line 344:     def __init__(self, process):
Line 345:         self._process = process
Line 346:         self.stdin = self._process.stdin
Line 347:         self.stdout = StringIO()


Line 345:         self._process = process
Line 346:         self.stdin = self._process.stdin
Line 347:         self.stdout = StringIO()
Line 348:         self.stderr = StringIO()
Line 349:         self._stream = CommandStream(self._process, self._outdb, self._errcb)
We can register self.stdout.write and self.stderr.write instead of keeping self._outcb and self._errcb.
Line 350: 
Line 351:     @property
Line 352:     def returncode(self):
Line 353:         return self._process.returncode


Line 349:         self._stream = CommandStream(self._process, self._outdb, self._errcb)
Line 350: 
Line 351:     @property
Line 352:     def returncode(self):
Line 353:         return self._process.returncode
Needs also poll(), delegating to underlying process.
Line 354: 
Line 355:     @property
Line 356:     def pid(self):
Line 357:         return self._process.pid


Line 356:     def pid(self):
Line 357:         return self._process.pid
Line 358: 
Line 359:     def wait(self, timeout=None):
Line 360:         self._process.wait()
We need to implement timeout:

    if timeout is None:
        self._process.wait()
    else:
        deadline = utils.monotonic_time() + timeout
        self._wait_until(deadline)
Line 361: 
Line 362:     def kill(self):
Line 363:         self._process.kill()
Line 364: 


Line 363:         self._process.kill()
Line 364: 
Line 365:     def communicate(self, input=None, timeout=None):
Line 366:         if input:
Line 367:             self.stdin.write(input)
Should flush stdin and close it, see current code doing this.
Line 368:         self._stream.receive(timeout)
Line 369:         if self._process.returncode is None:
Line 370:             raise RuntimeError("Timed out")
Line 371: 


Line 366:         if input:
Line 367:             self.stdin.write(input)
Line 368:         self._stream.receive(timeout)
Line 369:         if self._process.returncode is None:
Line 370:             raise RuntimeError("Timed out")
Should raise same exception raised in Python 3, to make same code work in 2 and 3. 

This currently is wrong, since receive can return *before* the timeout has passed. You can check stream.closed to understand if receive timed out, or the file descriptors were closed. If the stream was closed, we should wait for the process to exit with the remainning timeout.

The best way to implement this is to work with deadlines:

    deadline = utils.monotonic_time() + timeout
    ...
    self._stream.receive(timeout)
    self._wait_until(deadline)

And add:

    def _wait_until(self, deadline):    
        while self._process.poll() is None:
            time.sleep(1)
            if utils.monotonic_time() >= deadline:
                raise TimeoutExpired(...)

This will also help with wait(timeout).
Line 371: 
Line 372:         return (self._process.returncode,
Line 373:                 self.stdout.getvalue(),
Line 374:                 self.stderr.getvalue())


Line 370:             raise RuntimeError("Timed out")
Line 371: 
Line 372:         return (self._process.returncode,
Line 373:                 self.stdout.getvalue(),
Line 374:                 self.stderr.getvalue())
This should return out, err - see Popen docs.
Line 375: 
Line 376:     def _outcb(self, data):
Line 377:         self.stdout.write(data)
Line 378: 


-- 
To view, visit https://gerrit.ovirt.org/49441
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I14dc1b2520706894cdd897659513c68e738500ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list