Change in vdsm[master]: monitor: return whether the reported status is real
laravot at redhat.com
laravot at redhat.com
Sun Feb 22 13:26:49 UTC 2015
Liron Aravot has posted comments on this change.
Change subject: monitor: return whether the reported status is real
......................................................................
Patch Set 3:
(8 comments)
http://gerrit.ovirt.org/#/c/37952/3//COMMIT_MSG
Commit Message:
Line 12: by the engine (leads to domain status change).
Line 13:
Line 14: It was attempted to change the inital returned status in the past but it
Line 15: broke the host activation flow in the engine (see change I8e0df) and
Line 16: therfore the inital results can't be changed (backward comptability) -
> therfore -> therefore
Done
Line 17: therefore in this change i add a new info that indicates whether the
Line 18: reported status is real or not.
Line 19:
Line 20: Change-Id: I1fea518991a76ea0f9ff1ff5258afe95bca2f00d
Line 14: It was attempted to change the inital returned status in the past but it
Line 15: broke the host activation flow in the engine (see change I8e0df) and
Line 16: therfore the inital results can't be changed (backward comptability) -
Line 17: therefore in this change i add a new info that indicates whether the
Line 18: reported status is real or not.
> real -> actual
Done
Line 19:
Line 20: Change-Id: I1fea518991a76ea0f9ff1ff5258afe95bca2f00d
Line 21: Bug-Url: https://bugzilla.redhat.com/1183977
http://gerrit.ovirt.org/#/c/37952/3/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1776: #
Line 1777: # @actual: Indicates if the returned status is an actual monitoring result
Line 1778: # or initial result that means that the first monitor run was not
Line 1779: # yet completed
Line 1780: # (new in version 4.16.12)
> I think next version 4.17.something
is there a place in which it's documented? how can i determine the version?
Line 1781: #
Line 1782: # Since: 4.10.0
Line 1783: # XXX: Add an enum for return codes and their meanings
Line 1784: ##
Line 1784: ##
Line 1785: {'type': 'StorageDomainVitals',
Line 1786: 'data': {'code': 'int', 'delay': 'float', 'lastCheck': 'float',
Line 1787: 'valid': 'bool', 'version': 'int', 'acquired': 'bool',
Line 1788: 'real': 'bool'}}
> It would be nice to replace real with actual :-)
Done
Line 1789:
Line 1790: ##
Line 1791: # @PathStats:
Line 1792: #
http://gerrit.ovirt.org/#/c/37952/3/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 3612: 'valid': (domStatus.error is None),
Line 3613: 'version': domStatus.version,
Line 3614: # domStatus.hasHostId can also be None
Line 3615: 'acquired': domStatus.hasHostId is True,
Line 3616: 'real': domStatus.real
> It would be nice to replace real with actual here also.
Done
Line 3617: },
Line 3618:
Line 3619: 'disktotal': disktotal,
Line 3620: 'diskfree': diskfree,
http://gerrit.ovirt.org/#/c/37952/3/vdsm/storage/monitor.py
File vdsm/storage/monitor.py:
Line 171
Line 172
Line 173
Line 174
Line 175
> Use FrozenStatus(Status(actual=False)).
while i agree that this is uneeded, this should be changed in a separate change - this change isn't related to this one and doesn't affect this patch.
Line 170: self.domain = None
Line 171: self.sdUUID = sdUUID
Line 172: self.hostId = hostId
Line 173: self.interval = interval
Line 174: self.nextStatus = Status(actual=False)
> This is wrong - nextStatus *must* be actual.
that is fine- see line 225, newStatus is being initialized when _monitorDomain runs.
I haven't changed the behavior here from the previous code. In the old code before _monitorDomain ran and set firstChange = False, the nextStatus was considered as 'not actual' (because firstChange was false) - there's no need to change that part.
Line 175: self.status = FrozenStatus(self.nextStatus)
Line 176: self.isIsoDomain = None
Line 177: self.isoPrefix = None
Line 178: self.lastRefresh = time.time()
Line 264: # Notifiying status changes
Line 265:
Line 266: def _statusDidChange(self):
Line 267: return not self.status.actual \
Line 268: or self.status.valid != self.nextStatus.valid
> Please do not use \ for long lines, use:
Done
Line 269:
Line 270: @utils.cancelpoint
Line 271: def _notifyStatusChanges(self):
Line 272: self.log.info("Domain %s became %s", self.sdUUID,
--
To view, visit http://gerrit.ovirt.org/37952
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1fea518991a76ea0f9ff1ff5258afe95bca2f00d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Candace Sheremeta <cshereme at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer 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