Vinzenz Feenstra has posted comments on this change.

Change subject: vdsm: Try to reconnect on vmchannel after errors
......................................................................


Patch Set 5: (5 inline comments)

....................................................
File vdsm/guestIF.py
Line 92:     def _prepare_socket(self):
Line 93:         supervdsm.getProxy().prepareVmChannel(self._socketName)
Line 94: 
Line 95:     @staticmethod
Line 96:     def _setup_socket(self):
Done
Line 97:         if hasattr(self, '_sock'):
Line 98:             self._sock.close()
Line 99:         self._sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
Line 100:         self._sock.setblocking(0)


Line 323:                 # The connection is broken when recv returns no data
Line 324:                 # therefore we're going to set ourself to stopped 
state
Line 325:                 if len(data) == 0:
Line 326:                     self._stopped = True
Line 327:                     break
Actually this is just preventing an endless loop. If this scenario occurs the 
handling of the EPOLLERR and EPOLLHUP events will actually handle this.

However I will return True/False from here and in case there was such a failure 
it will return False.
Line 328:                 self._handleData(data)
Line 329:         except socket.error as err:
Line 330:             if err.errno == errno.EWOULDBLOCK:
Line 331:                 pass


....................................................
File vdsm/vmChannels.py
Line 40:         self._epoll = select.epoll()
Line 41:         self._channels = {}
Line 42:         self._unconnected = {}
Line 43:         self._update_lock = threading.Lock()
Line 44:         self._reconnect_cooldown = {}
Done
Line 45:         self._add_channels = {}
Line 46:         self._del_channels = []
Line 47:         self._timeout = None
Line 48: 


Line 49:     def _handle_event(self, fileno, event):
Line 50:         """ Handle an epoll event occurred on a specific file 
descriptor. """
Line 51:         if (event & (select.EPOLLHUP | select.EPOLLERR)):
Line 52:             self.log.error("Received EPOLLHUP on fileno %d", fileno)
Line 53:             self._prepare_reconnect(fileno)
One thing is to get no data during reading from that socket the other thing is 
that an error happened during the poll and not during reading.
This should be checked and handled as well. 

Handling the no data in guestIF is just preventing to run into an endless loop.
Line 54:         elif (event & select.EPOLLIN):
Line 55:             obj = self._channels[fileno]
Line 56:             obj['reconnects'] = 0
Line 57:             try:


Line 186:         """ Set the timeout value (in seconds) for all channels. """
Line 187:         self.log.info("Setting channels' timeout to %d seconds.", 
seconds)
Line 188:         self._timeout = seconds
Line 189: 
Line 190:     def register(self, connect_callback, setup_callback, 
read_callback,
Done
Line 191:                  timeout_callback, opaque):
Line 192:         """ Register a new file descriptor to the listener. """
Line 193:         fileno = setup_callback()
Line 194:         self.log.debug("Add fileno %d to listener's channels.", 
fileno)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b9b9418e39558d45c97e9545bc9ecc4935f004e
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Gal Hammer <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Peter V. Saveliev <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to