Vinzenz Feenstra has posted comments on this change. Change subject: Create GuestAgent instance in __init__ and connect later ......................................................................
Patch Set 7: (2 comments) http://gerrit.ovirt.org/#/c/26142/7/vdsm/virt/guestagent.py File vdsm/virt/guestagent.py: Line 136: self._messageState = MessageState.NORMAL Line 137: Line 138: def connect(self): Line 139: try: Line 140: self._prepare_socket() > this patch is a clear improvement; however, it would be nicer if the new co ok, done. But not happy with the fact that i have there a 'except Exception' in vm.py Line 141: except: Line 142: self.log.error("Failed to prepare vmchannel", exc_info=True) Line 143: else: Line 144: self._channelListener.register( http://gerrit.ovirt.org/#/c/26142/7/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4297: # Terminate the VM's creation thread. Line 4298: self._incomingMigrationFinished.set() Line 4299: if self._vmStats: Line 4300: self._vmStats.stop() Line 4301: self.guestAgent.stop() > I did not check - does this is bound to succeed even on non-connected guest Well errno.EBADF is caught and handled gracefully However it is possible that there can be other exceptions which are re-raised or not caught Which would be no different than it is today for that matter, except that we would avoid here the self.guestAgent == None cases Line 4302: if self._dom: Line 4303: try: Line 4304: self._dom.destroyFlags( Line 4305: libvirt.VIR_DOMAIN_DESTROY_GRACEFUL) -- To view, visit http://gerrit.ovirt.org/26142 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82f7397b01bff48a3c635eee9912cc67cf722b13 Gerrit-PatchSet: 7 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: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
