[libvirt] [PATCH] Revert "rpc: Discard non-blocking calls only when necessary"
This reverts commit b1e374a7ac56927cfe62435179bf0bba1e08b372, which was rather bad since I failed to consider all sides of the issue. Thus, the reverted patch actually breaks more than what it fixes and clients (which may even be libvirtd during p2p migrations) will likely end up with a corrupted dispatch queue. --- I will send proper replacement for the reverted patch later once I'm convinced it's correct (more than I was convinced the original one was correct) :-P src/rpc/virnetclient.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 3a60db6..d88288d 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1265,13 +1265,6 @@ static void virNetClientIOEventLoopPassTheBuck(virNetClientPtr client, virNetCli } client->haveTheBuck = false; - /* Remove non-blocking calls from the dispatch list since there is no - * call with a thread in the list which could take care of them. - */ - virNetClientCallRemovePredicate(&client->waitDispatch, - virNetClientIOEventLoopRemoveNonBlocking, - thiscall); - VIR_DEBUG("No thread to pass the buck to"); if (client->wantClose) { virNetClientCloseLocked(client); @@ -1315,9 +1308,12 @@ static int virNetClientIOEventLoop(virNetClientPtr client, if (virNetSocketHasCachedData(client->sock) || client->wantClose) timeout = 0; - /* If we are non-blocking, we don't want to sleep in poll() + /* If there are any non-blocking calls in the queue, + * then we don't want to sleep in poll() */ - if (thiscall->nonBlock) + if (virNetClientCallMatchPredicate(client->waitDispatch, + virNetClientIOEventLoopWantNonBlock, + NULL)) timeout = 0; fds[0].events = fds[0].revents = 0; @@ -1422,6 +1418,13 @@ static int virNetClientIOEventLoop(virNetClientPtr client, virNetClientIOEventLoopRemoveDone, thiscall); + /* Iterate through waiting calls and if any are + * non-blocking, remove them from the dispatch list... + */ + virNetClientCallRemovePredicate(&client->waitDispatch, + virNetClientIOEventLoopRemoveNonBlocking, + thiscall); + /* Now see if *we* are done */ if (thiscall->mode == VIR_NET_CLIENT_MODE_COMPLETE) { virNetClientCallRemove(&client->waitDispatch, thiscall); -- 1.7.10.2
On 05/22/2012 07:34 AM, Jiri Denemark wrote:
This reverts commit b1e374a7ac56927cfe62435179bf0bba1e08b372, which was rather bad since I failed to consider all sides of the issue. Thus, the reverted patch actually breaks more than what it fixes and clients (which may even be libvirtd during p2p migrations) will likely end up with a corrupted dispatch queue. ---
I will send proper replacement for the reverted patch later once I'm convinced it's correct (more than I was convinced the original one was correct) :-P
src/rpc/virnetclient.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
ACK for being a straight revert, although I would hold off pushing this until the cleaner replacement has been reviewed and can be pushed at the same time. Just so I understand, how was this patch causing new problems? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, May 22, 2012 at 11:34:33 -0600, Eric Blake wrote:
On 05/22/2012 07:34 AM, Jiri Denemark wrote:
This reverts commit b1e374a7ac56927cfe62435179bf0bba1e08b372, which was rather bad since I failed to consider all sides of the issue. Thus, the reverted patch actually breaks more than what it fixes and clients (which may even be libvirtd during p2p migrations) will likely end up with a corrupted dispatch queue. ---
I will send proper replacement for the reverted patch later once I'm convinced it's correct (more than I was convinced the original one was correct) :-P
src/rpc/virnetclient.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
ACK for being a straight revert, although I would hold off pushing this until the cleaner replacement has been reviewed and can be pushed at the same time.
Just so I understand, how was this patch causing new problems?
The replacement patch will need some time to settle down and I'd rather push this revert soon. Eric agreed on irc. Thus I made the comment better in describing what I failed to consider in the original patch and pushed this revert. Jirka
participants (2)
-
Eric Blake -
Jiri Denemark