1:47 p.m.
Adam Litke has uploaded a new change for review.
Change subject: Define some constants in API.py
......................................................................
Define some constants in API.py
Signed-off-by: Adam Litke <agl(a)us.ibm.com>
Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
---
M vdsm/API.py
1 file changed, 16 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/62/2262/1
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
10:02 p.m.
Mark Wu has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 5: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
7:43 a.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Define some constants in API.py
......................................................................
Define some constants in API.py
The API bindings should be implemented in terms of the API bridge (API.py) and
not rely on vdsm internals. Currently the xmlrpc binding imports some internal
modules in order to provide some default values (ie. BLANK_UUID and volume
constants). Constants that are needed at the API level need to appear in
API.py.
This patch converts the obvious offenses. If this approach is acceptible I will
follow up with additional patches. In the future, APIs that take 'magic'
parameters must formally define the valid values in API.py.
Signed-off-by: Adam Litke <agl(a)us.ibm.com>
Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
---
M vdsm/API.py
M vdsm/BindingXMLRPC.py
2 files changed, 43 insertions(+), 15 deletions(-)
Approvals:
Mark Wu: Looks good to me, but someone else must approve
Dan Kenigsberg: Looks good to me, approved
Peter V. Saveliev: Verified; Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
7:43 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 5: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
7:39 a.m.
Peter V. Saveliev has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 5: Verified; Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
2:52 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 5: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
3:41 p.m.
Adam Litke has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 4: (3 inline comments)
....................................................
File vdsm/API.py
Line 610: INTERNAL = storage.volume.INTERNAL_VOL
Fair enough. I will remove it. We can always introduce it again if needed.
Line 760: DEFAULT_VERSION = storage.sd.DOM_SAFELEASE_VERS[0]
It's exposed so that the xmlrpc bindings can set it as a default parameter in
domainCreate. API.py already handles version==None in this way so I guess we can remove
it.
Line 945: MAX_HOST_ID = storage.safelease.MAX_HOST_ID
It's a default parameter for poolSpmStart in the xmlrpc bindings. Also handled in
API.py so removable.
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
8:51 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 4: Looks good to me, but someone else must approve
(2 inline comments)
approach looks good to me, but 2 minor questions within.
....................................................
File vdsm/API.py
Line 760: DEFAULT_VERSION = storage.sd.DOM_SAFELEASE_VERS[0]
why would we want DEFAULT_VERSION exposed in the API?
Line 945: MAX_HOST_ID = storage.safelease.MAX_HOST_ID
pardon, but why this has to be exposed to Vdsm API users?
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
12:34 a.m.
Mark Wu has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 4: Looks good to me, but someone else must approve
(1 inline comment)
....................................................
File vdsm/API.py
Line 610: INTERNAL = storage.volume.INTERNAL_VOL
Since currently the role "INTERNAL" is only used by the internal snapshot
management, I think we needn't export it in API. Does it make sense?
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
5:16 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm/API.py
Line 607: SHARED = 6
I understand why the Binding should not import vdsm internals. But I do not see how API.py
can get away without having these imports.
Here, you assume that API.Volume.Roles.SHARED == storage.volume.SHARED_VOL but this is
enforced only by hard-coding "6" for both enumerations, twice.
I do not fancy that. You can set
API.Volume.Roles.SHARED = storage.volume.SHARED_VOL
or have a completely different value there - but have an explicit conversion to the
internal value when you pass it to the internal function.
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
5:52 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 1: I would prefer that you didn't submit this
-1 for visibility
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
5:43 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 1:
I must say that I do not understand the implementation. How is a prospective user of
API.Volume.copy() is going to use these definition? Could you suggest a use case in the
commit message?
Do we really need the 'STRINGS' at the API level? (I hope that) currently they are
limited to the on-disk representation of metadata.
Note that I know too well how the current usage is confusing and needs a change...
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
4:48 p.m.
Adam Litke has posted comments on this change.
Change subject: Define some constants in API.py
......................................................................
Patch Set 1:
Hi, this patch is a first step at exposing some constants at the API bridge level. Do you
agree with the way I have implemented it?
--
To view, visit http://gerrit.ovirt.org/2262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaef19f956ec1d6f83fd48e71a2f0a018505266be
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
4404
days inactive
4446
days old
vdsm-patches@lists.fedorahosted.org
12 comments
4 participants
participants (4)
-
agl＠us.ibm.com -
Dan Kenigsberg -
peet＠redhat.com -
wudxw＠linux.vnet.ibm.com