Change in vdsm[master]: events: vm status notifications

piotr.kliczewski at gmail.com piotr.kliczewski at gmail.com
Fri Jun 12 10:59:36 UTC 2015


Piotr Kliczewski has posted comments on this change.

Change subject: events: vm status notifications
......................................................................


Patch Set 30:

(4 comments)

https://gerrit.ovirt.org/#/c/38937/30/tests/vmApiTests.py
File tests/vmApiTests.py:

Line 67: 
Line 68: 
Line 69: class TestVmStats(TestSchemaCompliancyBase):
Line 70: 
Line 71:     @MonkeyPatch(vm.Vm, 'send_status_event', lambda x: None)
> it is probably easier and less error prone to monkeypatch vmfakelib.Vm once
I would keep it as it is. We may want to test content of a status in the future.
Line 72:     def testDownStats(self):
Line 73:         with fake.VM() as testvm:
Line 74:             testvm.setDownStatus(define.ERROR, vmexitreason.GENERIC_ERROR)
Line 75:             self.assertVmStatsSchemaCompliancy('ExitedVmStats',


https://gerrit.ovirt.org/#/c/38937/30/vdsm/clientIF.py
File vdsm/clientIF.py:

Line 604:                 v._onWatchdogEvent(action)
Line 605:             else:
Line 606:                 v.log.warning('unknown eventid %s args %s', eventid, args)
Line 607: 
Line 608:             v.send_status_event()
> I'd rather take the timestamp here (maybe at the beginning of the send_stat
For events we set it just before sending.
Line 609:         except:


https://gerrit.ovirt.org/#/c/38937/30/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 390:                 'hash': str(hash((self._domain.devices_hash,
Line 391:                                   self.guestAgent.diskMappingHash))),
Line 392:             }
Line 393: 
Line 394:             if vm_status == vmstatus.DOWN:
> I don't like this duplication, we do less or more the same work on getExite
We talked about whether it would be better to use getExitedVmStatus and it seems that the method provides a bit more information than we want to send.
Line 395:                 if 'exitCode' in self.conf:
Line 396:                     stats['exitCode'] = self.conf['exitCode']
Line 397:                 if 'exitMessage' in self.conf:
Line 398:                     stats['exitMessage'] = self.conf['exitMessage']


Line 396:                     stats['exitCode'] = self.conf['exitCode']
Line 397:                 if 'exitMessage' in self.conf:
Line 398:                     stats['exitMessage'] = self.conf['exitMessage']
Line 399:                 if 'exitReason' in self.conf:
Line 400:                     stats['exitReason'] = self.conf['exitReason']
> Apparently, there *is* such a race - and it exists regardless of the notifi
Done
Line 401: 
Line 402:             self._notify('VM_status', stats)
Line 403: 
Line 404:     def _notify(self, operation, params):


-- 
To view, visit https://gerrit.ovirt.org/38937
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I746299f9f1e2f49831a05072f19af1d242796276
Gerrit-PatchSet: 30
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list