6:51 a.m.
Shahar Havivi has uploaded a new change for review.
Change subject: setVmTicket: add before and after hooks
......................................................................
setVmTicket: add before and after hooks
setVmTicket get two new additional params: user-name and user-id, for
the benefit of the hook scripts.
Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Signed-off-by: Shahar Havivi <shaharh(a)redhat.com>
---
M vdsm.spec.in
M vdsm/API.py
M vdsm/BindingXMLRPC.py
M vdsm/hooks.py
M vdsm/libvirtvm.py
M vdsm/vdsmd.8.in
M vdsm_cli/vdsClient.py
M vdsm_hooks/Makefile.am
8 files changed, 29 insertions(+), 11 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/65/3565/1
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
3:31 a.m.
Shahar Havivi has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 8: (1 inline comment)
....................................................
File vdsm/vdsmd.8.in
Line 76: "params" with a json-encoded value that was passed by setVmTicket
caller.
?
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
4:01 a.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: setVmTicket: add before and after hooks
......................................................................
setVmTicket: add before and after hooks
setVmTicket gets additional params dictionary,
currently engine will send user-name and user-id, for
the benefit of the setVmTicket hooks scripts.
Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Signed-off-by: Shahar Havivi <shaharh(a)redhat.com>
---
M vdsm.spec.in
M vdsm/API.py
M vdsm/BindingXMLRPC.py
M vdsm/hooks.py
M vdsm/libvirtvm.py
M vdsm/vdsmd.8.in
M vdsm_cli/vdsClient.py
M vdsm_hooks/Makefile.am
8 files changed, 42 insertions(+), 13 deletions(-)
Approvals:
Dan Kenigsberg: Verified; Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
4:01 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 9: Verified; Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
3:17 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 8: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm/vdsmd.8.in
Line 76: "params" with a json-encoded value that was passed by setVmTicket
caller.
but you did NOT agree with me that this should be the interface.
your current behavior is:
The environment of before_vm_set_ticket and after_vm_set_ticket hooks is augmented with a
set of params passed by the caller of setVmTicket.
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
2:50 a.m.
Shahar Havivi has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 7: (1 inline comment)
....................................................
File vdsm/vdsmd.8.in
Line 74:
Done
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
2:41 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 7: (2 inline comments)
....................................................
File vdsm/hooks.py
Line 51: scriptenv.update(params)
In my opinion it is a too limiting interface... but I'm not 100% sure.
....................................................
File vdsm/vdsmd.8.in
Line 74:
suggested text: (not sure)
before_vm_set_ticket and after_vm_set_ticket hooks receive a variable called
"params" with a json-encoded value that was passed by setVmTicket caller.
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
2:09 a.m.
Shahar Havivi has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 7: (4 inline comments)
....................................................
File vdsm/hooks.py
Line 37: def _runHooksDir(domxml, dir, vmconf={}, raiseError=True, params = {}):
Done
Line 51: scriptenv.update(params)
yes its string
Line 135: def before_vm_set_ticket(domxml, params, vmconf={}):
Done
....................................................
File vdsm/vdsmd.8.in
Line 65: .B vmId.
what do you mean?
please define the string you want me to add/change
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
4:29 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 7: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File vdsm/hooks.py
Line 37: def _runHooksDir(domxml, dir, vmconf={}, raiseError=True, params = {}):
no spaces around this '=' sign please.
can you think of a better name than "params"? I cannot....
Line 51: scriptenv.update(params)
the custom params of vm are strings, by design.
Is that what we want for these extra params, too? I am not sure. I would prefer that these
params are left general, and are marshaled via json or pickle for the use of the script.
Line 135: def before_vm_set_ticket(domxml, params, vmconf={}):
all hooks have vmconf as the second arg (if they have it at all). I'd like to keep
this consistency.
....................................................
File vdsm/vdsmd.8.in
Line 65: .B vmId.
the new hooks have extra environ variables, from the "params" param
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
8:11 a.m.
Ayal Baron has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 7: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
7:38 a.m.
Shahar Havivi has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
7:38 a.m.
Shahar Havivi has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 7: Verified
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
4:22 a.m.
Shahar Havivi has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 3: (3 inline comments)
....................................................
File vdsm/API.py
Line 518: :param pamram additoinal parameters in dict format
Done
....................................................
File vdsm_cli/vdsClient.py
Line 315: params = {}
Done
Line 1632: 'Optional adiitonal parameters in dictionary
format, name:value,name:value'
Done
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
3:54 a.m.
Ayal Baron has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 3: (3 inline comments)
Minor nit picks (sorry)
....................................................
File vdsm/API.py
Line 518: :param pamram additoinal parameters in dict format
s/pamram//
s/additoinal/additional/
....................................................
File vdsm_cli/vdsClient.py
Line 315: params = {}
no need to define params here in the if... else, just:
params = {}
if len(args) >= 4:
params = self...
Line 1632: 'Optional adiitonal parameters in dictionary
format, name:value,name:value'
s/adiitional/additional
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
1:37 a.m.
Shahar Havivi has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 2: (3 inline comments)
....................................................
File vdsm_cli/vdsClient.py
Line 311: params = {'userName': userName, 'userId': userId}
yes, but me must check if it get 3 params since by now the forth param was optional...
Line 1625: ('<vmId> <password> <sec>
[disconnect|keep|fail]',
Done
....................................................
File vdsm/libvirtvm.py
Line 1925: userName = params.get('userName', '')
Done
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
9:18 a.m.
Ayal Baron has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm_cli/vdsClient.py
Line 311: params = {'userName': userName, 'userId': userId}
should be generic. First 4 (mandatory) params should be passed by name, the rest should
be just pushed into a dict.
vmId, otp64, secs, connAct = args[:4]
params = args[4:] (probably just a string like backend passes? or need to convert into a
dict or something)
...
Line 1625: ('<vmId> <password> <sec>
[disconnect|keep|fail]',
no mention of optional 'params'?
....................................................
File vdsm/libvirtvm.py
Line 1925: userName = params.get('userName', '')
why do we expose the specific params here?
It should be up to the script to determine what it needs ?!?
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
8:18 a.m.
Shahar Havivi has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 1: (3 inline comments)
....................................................
Commit Message
Line 10: the benefit of the hook scripts.
Done
....................................................
File vdsm/API.py
Line 502: def setTicket(self, password, ttl, userName, userId, existingConnAction):
Done
....................................................
File vdsm.spec.in
Line 554: %dir %{_libexecdir}/%{vdsm_name}/hooks/before_set_ticket
Done
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
7:11 a.m.
Ayal Baron has posted comments on this change.
Change subject: setVmTicket: add before and after hooks
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(3 inline comments)
Just a few minor comments.
....................................................
Commit Message
Line 10: the benefit of the hook scripts.
Like the rest of the hooks, setVmTicket should get a custom properties dict and not
'user-name' and 'user-id'.
engine would always send user-name and user-id and the custom properties users have
defined there to allow for normal hooks behaviour.
....................................................
File vdsm/API.py
Line 502: def setTicket(self, password, ttl, userName, userId, existingConnAction):
As I mentioned on the commit message, should be custom properties the same as the rest of
the hooks (I don't remember the name of the dictionary, but should be persistent)
....................................................
File vdsm.spec.in
Line 554: %dir %{_libexecdir}/%{vdsm_name}/hooks/before_set_ticket
please rename to include 'vm' as the rest of the hooks are named.
i.e. before_vm_set_ticket and after_vm_set_ticket
--
To view, visit http://gerrit.ovirt.org/3565
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48cbc934e48055630b734a2d3c79c72b6c36f28f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
4452
days inactive
4455
days old
vdsm-patches@lists.fedorahosted.org
17 comments
3 participants
participants (3)
-
abaron＠redhat.com -
Dan Kenigsberg -
shavivi＠redhat.com