Vinzenz Feenstra has posted comments on this change. Change subject: virt: Correct epoll unregistration usage in vmchannels ......................................................................
Patch Set 3: (2 comments) https://gerrit.ovirt.org/#/c/51521/3/vdsm/virt/vmchannels.py File vdsm/virt/vmchannels.py: Line 53: self._epoll.unregister(fileno) Line 54: except IOError as e: Line 55: # We do not care if it has been previously removed from epoll Line 56: # or is not tracked by epoll. The removal is exactly what we Line 57: # want to achieve. > Lets move the the check before the comment, and log debug debug message if I will move this comment behind the condition, however I won't add the log line. We have already enough that are normal things and debug messages were customers are complaining about that they are logged. They are normal things happening during the life time and I don't want that to happen again. Line 58: if e.errno != errno.ENOENT: Line 59: raise Line 60: Line 61: def _handle_event(self, fileno, event): Line 225: def unregister(self, fileno): Line 226: """ Unregister an exist file descriptor from the listener. """ Line 227: self.log.debug("Delete fileno %d from listener.", fileno) Line 228: with self._update_lock: Line 229: # Threadsafe, fileno will be closed by caller > Please explain in the comment why this must be done here and not insie _do_ ok Line 230: self._unregister_fd(fileno) -- To view, visit https://gerrit.ovirt.org/51521 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f7eab8318f41f653e0a24552c81bcd5b09d8690 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
