Piotr Kliczewski has posted comments on this change. Change subject: virt: Trigger event on guest agent status changes ......................................................................
Patch Set 2: (2 comments) https://gerrit.ovirt.org/#/c/41108/2/vdsm/virt/guestagent.py File vdsm/virt/guestagent.py: Line 133: self._channelListener = channelListener Line 134: self._messageState = MessageState.NORMAL Line 135: Line 136: @property Line 137: def guestStatus(self): > I know the varialbe was there already, but since now you're building an int Will change. Line 138: return self._guestStatus Line 139: Line 140: @guestStatus.setter Line 141: def guestStatus(self, value): https://gerrit.ovirt.org/#/c/41108/2/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 354: self._vmJobs = None Line 355: Line 356: def _agent_status_changed(self): Line 357: vm_status = self._getVmStatus() Line 358: if self.lastStatus != vm_status: > you're comparing different set of values the comparison is valid. I understand that you would like to set guest status as lastStatus but let's do it that later and refactor status related logic completely. Line 359: self._send_status_event(vm_status['status']) Line 360: Line 361: def _get_lastStatus(self): Line 362: # note that we don't use _statusLock here. One of the reasons is the -- To view, visit https://gerrit.ovirt.org/41108 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaceca8e42720df50d5eb1e20670b6ed733db8b50 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
