Michal Skrivanek has posted comments on this change. Change subject: events: sending stats with vm status change event ......................................................................
Patch Set 2: (3 comments) https://gerrit.ovirt.org/#/c/40204/2/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 368: return status Line 369: Line 370: def _set_lastStatus(self, value): Line 371: with self._statusLock: Line 372: self._send_status_event(value) would be better if we send it only on a status change, not all the time. I'd suspect sometimes we may set the same status twice Line 373: if self._lastStatus == vmstatus.DOWN: Line 374: self.log.warning( Line 375: 'trying to set state to %s when already Down', Line 376: value) Line 386: Line 387: def _send_status_event(self, value): Line 388: if self.lastStatus == vmstatus.DOWN: Line 389: stats = self._getExitedVmStats() Line 390: else: this is a nice improvement. Still we should drop few fields - this still does contain applist I think. Also net and disk stats. Let's talk about the content outside of the review Line 391: stats = self._getConfigVmStats() Line 392: stats.update(self._getRunningVmStats()) Line 393: stats.update(self._getVmStatus()) Line 394: Line 4635: self.log.warning('monitor become unresponsive' Line 4636: ' (command timeout, age=%s)', Line 4637: statsAge) Line 4638: stats['monitorResponse'] = '-1' Line 4639: self._send_status_event('NotResponding') This is aproblematic one...vdsm doesn't have a NotResponding status, it's only derived in engine when monitorResponse==-1. Since this is a new code maybe we can tweak the code and introduce this state on engine side Line 4640: Line 4641: def updateNumaInfo(self): Line 4642: self._numaInfo = numaUtils.getVmNumaNodeRuntimeInfo(self) Line 4643: -- To view, visit https://gerrit.ovirt.org/40204 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61a1bdea997ec24342bbb25b3baeb3e7f8313321 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Dima Kuznetsov <dkuzn...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <mskri...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
