Nir Soffer has posted comments on this change.

Change subject: vdsm: Shuting down protocol detector fails
......................................................................


Patch Set 3:

(4 comments)

Nice, but can be much simpler.

http://gerrit.ovirt.org/#/c/29556/3/vdsm/protocoldetector.py
File vdsm/protocoldetector.py:

Line 93:         self.log.debug("Acceptor running")
Line 94:         self._required_size = max(h.REQUIRED_SIZE for h in 
self._handlers)
Line 95:         self.log.debug("Using required_size=%d", self._required_size)
Line 96:         self._next_cleanup = time.time() + self.CLEANUP_INTERVAL
Line 97:         keep_running = True
The keep_running changes are not needed.
Line 98:         try:
Line 99:             while keep_running:
Line 100:                 try:
Line 101:                     keep_running = self._process_events()


Line 95:         self.log.debug("Using required_size=%d", self._required_size)
Line 96:         self._next_cleanup = time.time() + self.CLEANUP_INTERVAL
Line 97:         keep_running = True
Line 98:         try:
Line 99:             while keep_running:
while True:
Line 100:                 try:
Line 101:                     keep_running = self._process_events()
Line 102:                 except Exception:
Line 103:                     self.log.exception("Unhandled exception")


Line 97:         keep_running = True
Line 98:         try:
Line 99:             while keep_running:
Line 100:                 try:
Line 101:                     keep_running = self._process_events()
Handle Stopped exception here:

    except Stopped:
        return
Line 102:                 except Exception:
Line 103:                     self.log.exception("Unhandled exception")
Line 104:         finally:
Line 105:             self._cleanup()


Line 169:                 raise
Line 170: 
Line 171:     def _cleanup_wakeup_pipe(self):
Line 172:         try:
Line 173:             return os.read(self._read_fd, 128) == '1'
This is not about cleaning up the pipe any more, but about receiving stop 
request. So:

    def check_wakeup_pipe(self):
        try:
            if os.read(self._read_fd, 1) == '1':
                raise Stopped()
        except ...
Line 174:         except OSError as e:
Line 175:             if e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK):
Line 176:                 raise
Line 177: 


-- 
To view, visit http://gerrit.ovirt.org/29556
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I82119a61835fe335f2aa5da29fb8d3f2b8ae33fc
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to