Dan Kenigsberg has posted comments on this change.

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


Patch Set 16: (3 inline comments)

few questions.

....................................................
File vdsm/guestIF.py
Line 202:         else:
Line 203:             self.log.error('Unknown message type %s', message)
Line 204: 
Line 205:     def stop(self):
Line 206:         if not self._stopped:
why is this "if" needed? Since it is raceful, there is still chance for the 
following code to be run multiple times in parallel.

it's no blocker, I'm just wondering about the motivation.
Line 207:             self._stopped = True
Line 208:             try:
Line 209:                 self._channelListener.unregister(self._sock.fileno())
Line 210:             except socket.error as e:


Line 335:                 else:
Line 336:                     self._handleData(data)
Line 337:         except socket.error as err:
Line 338:             if err.errno is not errno.EWOULDBLOCK:
Line 339:                 result = False
the original code is as problematic - but could you tell me why we are ignoring 
all but one socket.error ?
Line 340:         return result
Line 341: 
Line 342:     def _parseLine(self, line):
Line 343:         args = json.loads(line.decode('utf8'))


....................................................
File vdsm/vmChannels.py
Line 72:             obj = self._channels.pop(fileno)
Line 73:             try:
Line 74:                 fileno = obj['create_cb'](obj['opaque'])
Line 75:             except:
Line 76:                 self.log.exception("An error occurred in the setup 
callback "
setup->create
Line 77:                                    "fileno: %d.", fileno)
Line 78:             else:
Line 79:                 self._unconnected[fileno] = obj
Line 80: 


--
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: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[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: Jason Dillaman <[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