Change in vdsm[master]: vdsm: Try to reconnect on vmchannel after errors

danken at redhat.com danken at redhat.com
Tue Apr 16 20:19:30 UTC 2013


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 <vfeenstr at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Gal Hammer <ghammer at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Jason Dillaman <jdillama at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list