Vinzenz Feenstra has uploaded a new change for review. Change subject: virt: Correct epoll unregistration usage in vmchannels ......................................................................
virt: Correct epoll unregistration usage in vmchannels Previously we have been unregistering fds from epoll but not in the correct way. Which has led to log messages which seemed to have no good explanation as of why they occur. That has been changed to 'fix' it by letting only epoll unregister those file descriptors themselves when they are closed. This however has been wrong - the correct procedure to use epoll is to unregister the file descriptor first and then close the socket. In the vmchannel case there are two scenarios where this applies: - On an error reported by epoll - When we want to unregister the channel from vmchannels This patch fixes those two scenarios. Change-Id: I9f7eab8318f41f653e0a24552c81bcd5b09d8690 Bug-Url: https://bugzilla.redhat.com/1226911 Signed-off-by: Vinzenz Feenstra <[email protected]> --- M vdsm/virt/vmchannels.py 1 file changed, 12 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/51521/1 diff --git a/vdsm/virt/vmchannels.py b/vdsm/virt/vmchannels.py index 45f18bd..5f1d488 100644 --- a/vdsm/virt/vmchannels.py +++ b/vdsm/virt/vmchannels.py @@ -18,6 +18,7 @@ # Refer to the README and COPYING files for full details of the license # +import errno import threading import time import select @@ -47,6 +48,13 @@ self._del_channels = [] self._timeout = None + def _unregister_fd(self, fileno): + try: + self._epoll.unregister(fileno) + except IOError as e: + if e.errno != errno.ENOENT: + raise + def _handle_event(self, fileno, event): """ Handle an epoll event occurred on a specific file descriptor. """ reconnect = False @@ -54,9 +62,6 @@ self.log.debug("Received %.08X on fileno %d", event, fileno) if fileno in self._channels: reconnect = True - else: - self.log.debug("Received %.08X. On fd removed by epoll.", - event) elif (event & select.EPOLLIN): obj = self._channels.get(fileno, None) if obj: @@ -77,6 +82,8 @@ self._prepare_reconnect(fileno) def _prepare_reconnect(self, fileno): + # fileno will be closed by create_cb + self._unregister_fd(fileno) obj = self._channels.pop(fileno) obj['timeout_seen'] = False try: @@ -213,4 +220,6 @@ """ Unregister an exist file descriptor from the listener. """ self.log.debug("Delete fileno %d from listener.", fileno) with self._update_lock: + # Threadsafe, fileno will be closed by caller + self._unregister_fd(fileno) self._del_channels.append(fileno) -- To view, visit https://gerrit.ovirt.org/51521 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9f7eab8318f41f653e0a24552c81bcd5b09d8690 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
