Change in vdsm[master]: monitor: return whether the reported status is real

laravot at redhat.com laravot at redhat.com
Sun Feb 22 09:59:43 UTC 2015


Liron Aravot has posted comments on this change.

Change subject: monitor: return whether the reported status is real
......................................................................


Patch Set 2:

(8 comments)

http://gerrit.ovirt.org/#/c/37952/2//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: monitor: return whether the reported status is real
Line 8: 
Line 9: When the domain monitoring results are reported after the domain
Line 10: monitor wass started the first monitoring run may haven't been yet
> wass started the -> was started. The
Done
Line 11: completed - which causes the returned status to be determined as OK
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


http://gerrit.ovirt.org/#/c/37952/2/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:

Line 1776: #
Line 1777: # @real:       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'm not sure about "real". How about "checked", "actual", or "initializing"
let's go with actual
Line 1781: #
Line 1782: # Since: 4.10.0
Line 1783: # XXX: Add an enum for return codes and their meanings
Line 1784: ##


http://gerrit.ovirt.org/#/c/37952/2/vdsm/storage/monitor.py
File vdsm/storage/monitor.py:

Line 35
Line 36
Line 37
Line 38
Line 39
> real must be added to __slots__
Done


Line 170
Line 171
Line 172
Line 173
Line 174
> This one is real now...
mistakenly left out due to a change that i made and reverted, returning.


Line 171
Line 172
Line 173
Line 174
Line 175
> And this one is also real.
this line can keep using nextStatus as it did till today.


Line 38:         "vgMdHasEnoughFreeSpace", "vgMdFreeBelowThreashold", "hasHostId",
Line 39:         "isoPrefix", "version",
Line 40:     )
Line 41: 
Line 42:     def __init__(self, realStatus=True):
> Lets keep the same name:
Done
Line 43:         self.error = None
Line 44:         self.checkTime = time.time()
Line 45:         self.valid = True
Line 46:         self.readDelay = 0


Line 58:         # synchronous operation (e.g.: connectStoragePool, getInfo)
Line 59:         # we cannot risk to stop and wait for the iso domain to
Line 60:         # report its prefix (it might be unreachable).
Line 61:         self.isoPrefix = None
Line 62:         self.real = realStatus
> It is common to initialize the instance variables according to the order of
Done
Line 63:         self.version = -1
Line 64: 
Line 65: 
Line 66: class FrozenStatus(Status):


Line 263: 
Line 264:     # Notifiying status changes
Line 265: 
Line 266:     def _statusDidChange(self):
Line 267:         return self.status.real != self.nextStatus.real \
> real should be initialized to False, and it is enough to check for not self
i'll change it, though it may be clearer have the two status checks matching.
Line 268:                or self.status.valid != self.nextStatus.valid
Line 269: 
Line 270:     @utils.cancelpoint
Line 271:     def _notifyStatusChanges(self):


-- 
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: 2
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