Piotr Kliczewski 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' During testing of my changes I have never received the same status twice. I agree that I do not test all possible scenarios because I am not aware of all of them. We can add check to compare and send if different but if we think that the same status could be set twice in my opinion it is a bug. 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 do I checked the code and I believe that applist is added during _getGuestStats() and this was removed. I have decided to prepare status separately so we can freely modify. I am open to any suggestions what need to be send. 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 NotResponding is part of status enum on the engine side. I think this one and 'Paused' are problematic statuses for versioning. 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
