Change in vdsm[master]: vdsm: Adding guest agent API versioning support
vfeenstr at redhat.com
vfeenstr at redhat.com
Wed Jan 15 09:41:40 UTC 2014
Vinzenz Feenstra has posted comments on this change.
Change subject: vdsm: Adding guest agent API versioning support
......................................................................
Patch Set 6:
(4 comments)
http://gerrit.ovirt.org/#/c/17004/6/vdsm/guestIF.py
File vdsm/guestIF.py:
Line 26: import supervdsm
Line 27: import unicodedata
Line 28:
Line 29: _MAX_SUPPORTED_API_VERSION = 0
Line 30: _MIN_SUPPORTED_API_VERSION = 0
> Hi,
Because with this patchset there's no new version, 0 is the baseline for now. There's a new patch in the pipe which will increase MAX to 1. Simply 0 means all what was there before. Since there's no versioning so far, this is simply version 0.
See here: http://gerrit.ovirt.org/#/c/23258/1/vdsm/guestIF.py
Line 31: _API_DISABLED_VALUE = -1
Line 32:
Line 33: __REPLACEMENT_CHAR = u'\ufffd'
Line 34: __RESTRICTED_CHARS = set(range(8 + 1)). \
Line 138: Args:
Line 139: version - the api version value to validate
Line 140:
Line 141: Returns:
Line 142: A valid api version or 0 which means the lowest possible version of
> Can return -1 (disable) at this stage too right?
no, it should not
Line 143: the API.
Line 144: """
Line 145: try:
Line 146: apiVersion = int(version)
Line 146: apiVersion = int(version)
Line 147: except ValueError:
Line 148: self.log.warning("Received invalid version value: %s", version)
Line 149: apiVersion = _MIN_SUPPORTED_API_VERSION
Line 150: return min(apiVersion, _MIN_SUPPORTED_API_VERSION)
> Question: apiversion could be a value > 0? If yes, not sure if we should us
you're right, It actually MUST be a value >= 0. This should have been max and not min so that at a very minimum it is _MIN_SUPPORTED_API_VERSION
Line 151:
Line 152: def _setAPIVersion(self, version):
Line 153: """ Sets the API version if it was changed
Line 154: Args:
Line 154: Args:
Line 155: version - the api version to set
Line 156:
Line 157: Note: If the value of `version` is not in between
Line 158: _MIN_SUPPORTED_API_VERSION and _MAX_SUPPORTED_API_VERSION the value
> between? but both init as 0, not sure if I understood it.
Well that's the documentation, it doesn't matter that there's no version 1 yet, however this will come with the above mentioned patchset.
Well this is the documentation, the constants can change over time.
Line 159: will be adjusted to have the highest possible API version supported.
Line 160:
Line 161: This should not be required and the protocol does not rely on this,
Line 162: this is simply a data santization fallback.
--
To view, visit http://gerrit.ovirt.org/17004
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9095b528c2c910f12d5f170088a458bf11c71910
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet at redhat.com>
Gerrit-Reviewer: Roy Golan <rgolan at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list