Dan Kenigsberg 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 connect() method
did not swallow this failure. The caller of connect() should decided whether it is fine
with the fact that connect() did not succeed.
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 guestAgent?
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 <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes