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

fromani at redhat.com fromani at redhat.com
Thu Mar 19 15:35:06 UTC 2015


Francesco Romani has posted comments on this change.

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


Patch Set 1:

(5 comments)

Surprisingly simple, but I need to think a bit more about the API (from the client perspective mostly)

https://gerrit.ovirt.org/#/c/38937/1/tests/jsonRpcTests.py
File tests/jsonRpcTests.py:

Line 59:         return None
Line 60: 
Line 61:     def double_response(self):
Line 62:         n = self.cif.create_notification('vdsm.double_response')
Line 63:         n.emit(**{CALL_ID: True})
related?
Line 64:         return 'sent'
Line 65: 
Line 66:     def register_server_address(self, server_address):
Line 67:         self.server_address = server_address


Line 192:                     pass
Line 193:                 else:
Line 194:                     def callback(client, event, params):
Line 195:                         self.assertEquals(event, 'vdsm.double_response')
Line 196:                         self.assertEquals(params, {CALL_ID: True})
same
Line 197: 
Line 198:                     client.registerEventCallback(callback)
Line 199:                     res = self._callTimeout(client, "double_response", [],
Line 200:                                             CALL_ID, timeout=CALL_TIMEOUT)


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

Line 867:         with self._statusLock:
Line 868:             notification = self.cif.create_notification(
Line 869:                 '|virt|VM_status|' + self.id
Line 870:             )
Line 871:             notification.emit(**{self.id: value})
this hides a lot IMHO. We need a simpler way to achieve the same result.
Line 872:             if self._lastStatus == vmstatus.DOWN:
Line 873:                 self.log.warning(
Line 874:                     'trying to set state to %s when already Down',
Line 875:                     value)


Line 1624:                     self._lastStatus in vmstatus.PAUSED_STATES):
Line 1625:                 notification = self.cif.create_notification(
Line 1626:                     '|virt|VM_status|' + self.id
Line 1627:                 )
Line 1628:                 notification.emit(**{self.id: 'Paused'})
Need to check but I'm quite confident we can get rid of this hunk. Not a big deal for now.
Line 1629:         finally:
Line 1630:             if not guestCpuLocked:
Line 1631:                 self._guestCpuLock.release()
Line 1632: 


Line 5027:             stats['monitorResponse'] = '-1'
Line 5028:             notification = self.cif.create_notification(
Line 5029:                 '|virt|VM_status|' + self.id
Line 5030:             )
Line 5031:             notification.emit(**{self.id: 'NotResponding'})
We'll probably need the same treatment inside _timeoutExperienced.

but _timeoutExperienced it is itself a callback, so we need a bit of care there.
Line 5032: 
Line 5033: 
Line 5034: class LiveMergeCleanupThread(threading.Thread):
Line 5035:     def __init__(self, vm, jobId, drive, doPivot):


-- 
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list