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