Change in vdsm[master]: events: vm status notifications
piotr.kliczewski at gmail.com
piotr.kliczewski at gmail.com
Fri Mar 20 07:52:56 UTC 2015
Piotr Kliczewski has posted comments on this change.
Change subject: events: vm status notifications
......................................................................
Patch Set 1:
(5 comments)
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?
I wanted to be sure that this structure works nicely. Not really related to the change.
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
As above
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.
As replied to other comment I will simplify it.
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 bi
As I understood from our conversation we do not update vm state to 'Paused' but check for it when calling _get_lastStatus so if we want to send I think this is good place to do so.
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.
Done
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