Adam Litke has uploaded a new change for review.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Make the xmlrpc binding run in a thread
Currently, the xmlrpc binding runs in the vdsmd main thread. To prepare for making xmlrpc optional, change the binding loading logic so that all bindings run in their own threads. The vdsmd main thread will sleep until shutdown and then join() with any active binding threads before exiting.
Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Signed-off-by: Adam Litke agl@us.ibm.com --- M vdsm/BindingXMLRPC.py M vdsm/clientIF.py M vdsm/config.py.in 3 files changed, 23 insertions(+), 11 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/91/3891/1 -- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com
Adam Litke has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 1: Verified
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(1 inline comment)
very minor comment; I'd like to see this (and following patch) in.
.................................................... File vdsm/BindingXMLRPC.py Line 77: self.thread = threading.Thread(target=threaded_start, I love stating private implementation details as such (self._thread)
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Ayal Baron has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/clientIF.py Line 156: while self._enabled: why do we keep this thread alive and not join as part of prepareForShutdown?
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(1 inline comment)
seems that you've missed our comments to ps32
.................................................... File vdsm/BindingXMLRPC.py Line 79: self.thread.start() self._thread maybe?
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Adam Litke has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Adam Litke has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 3: (1 inline comment)
Yep, those comments are easy to miss when viewing patches in the dashboard. Without a -1/+1 review, I cannot even see that any comments were made unless I click on each patch.
.................................................... File vdsm/BindingXMLRPC.py Line 79: self.thread.start() Done
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 3:
what d'ya mean "Without a -1/+1 review"? both Ayal and I gave an explicit -1.
But right, it is easy to miss a comment. The reviewer should not be shy, and nag again (which I did) ;-)
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 4: Looks good to me, but someone else must approve
(1 inline comment)
however, I could see an improvement with daemon=True.
.................................................... File vdsm/BindingXMLRPC.py Line 77: self._thread = threading.Thread(target=threaded_start, I would still prefer this to be set to daemon, so that vdsm avoids blocking its exit on a stalled xmlrpc request.
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Adam Litke has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 4: (1 inline comment)
.................................................... File vdsm/BindingXMLRPC.py Line 77: self._thread = threading.Thread(target=threaded_start, Ahh, yes. Thanks for the reminder. I will make that change.
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Saggi Mizrahi has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 4: (1 inline comment)
.................................................... File vdsm/BindingXMLRPC.py Line 2: # Copyright 2011 Red Hat, Inc. 2012?
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Adam Litke has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 4: (1 inline comment)
.................................................... File vdsm/BindingXMLRPC.py Line 2: # Copyright 2011 Red Hat, Inc. Done
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 5: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Adam Litke has posted comments on this change.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Patch Set 5: Verified
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Make the xmlrpc binding run in a thread ......................................................................
Make the xmlrpc binding run in a thread
Currently, the xmlrpc binding runs in the vdsmd main thread. To prepare for making xmlrpc optional, change the binding loading logic so that all bindings run in their own threads. The vdsmd main thread will sleep until shutdown and then join() with any active binding threads before exiting.
Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Signed-off-by: Adam Litke agl@us.ibm.com --- M vdsm/BindingXMLRPC.py M vdsm/clientIF.py M vdsm/config.py.in 3 files changed, 24 insertions(+), 12 deletions(-)
Approvals: Adam Litke: Verified Dan Kenigsberg: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/3891 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: Ic0ea2589d5cb258c4b8d3d82d4f9c390b702a24f Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
vdsm-patches@lists.fedorahosted.org