[libvirt] [PATCH 0/4] Small fixes to non-blocking I/O in client
I missed these when reviewing the series... Jiri Denemark (4): rpc: Pass the buck only to the first available thread rpc: Fix a typo in virNetClientSendNonBlock documentation rpc: Fix handling of non-blocking calls that could not be sent rpc: Add some debug messages to virNetClient src/rpc/virnetclient.c | 15 ++++++++++++--- 1 files changed, 12 insertions(+), 3 deletions(-) -- 1.7.8.rc3
--- src/rpc/virnetclient.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index deeeaad..b518abd 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1102,7 +1102,7 @@ static void virNetClientIOEventLoopPassTheBuck(virNetClientPtr client, virNetCli if (tmp != thiscall && tmp->haveThread) { VIR_DEBUG("Passing the buck to %p", tmp); virCondSignal(&tmp->cond); - break; + return; } tmp = tmp->next; } -- 1.7.8.rc3
On Tue, Nov 22, 2011 at 04:32:31PM +0100, Jiri Denemark wrote:
--- src/rpc/virnetclient.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index deeeaad..b518abd 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1102,7 +1102,7 @@ static void virNetClientIOEventLoopPassTheBuck(virNetClientPtr client, virNetCli if (tmp != thiscall && tmp->haveThread) { VIR_DEBUG("Passing the buck to %p", tmp); virCondSignal(&tmp->cond); - break; + return; } tmp = tmp->next; }
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- src/rpc/virnetclient.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b518abd..0effceb 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1595,7 +1595,7 @@ int virNetClientSendNoReply(virNetClientPtr client, * Send a message asynchronously, without any reply * * The caller is responsible for free'ing @msg, *except* if - * this method returns -1. + * this method returns 1. * * Returns 2 on full send, 1 on partial send, 0 on no send, -1 on error */ -- 1.7.8.rc3
On Tue, Nov 22, 2011 at 04:32:32PM +0100, Jiri Denemark wrote:
--- src/rpc/virnetclient.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b518abd..0effceb 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1595,7 +1595,7 @@ int virNetClientSendNoReply(virNetClientPtr client, * Send a message asynchronously, without any reply * * The caller is responsible for free'ing @msg, *except* if - * this method returns -1. + * this method returns 1. * * Returns 2 on full send, 1 on partial send, 0 on no send, -1 on error */
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
When virNetClientIOEventLoop is called for a non-blocking call and not even a single byte can be sent from this call without blocking, we properly reported that to the caller which properly frees the call. But we never removed the call from a call queue. --- src/rpc/virnetclient.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 0effceb..c99e87c 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1256,7 +1256,12 @@ static int virNetClientIOEventLoop(virNetClientPtr client, /* We're not done, but we're non-blocking */ if (thiscall->nonBlock) { virNetClientIOEventLoopPassTheBuck(client, thiscall); - return thiscall->sentSomeData ? 1 : 0; + if (thiscall->sentSomeData) { + return 1; + } else { + virNetClientCallRemove(&client->waitDispatch, thiscall); + return 0; + } } if (fds[0].revents & (POLLHUP | POLLERR)) { -- 1.7.8.rc3
On Tue, Nov 22, 2011 at 04:32:33PM +0100, Jiri Denemark wrote:
When virNetClientIOEventLoop is called for a non-blocking call and not even a single byte can be sent from this call without blocking, we properly reported that to the caller which properly frees the call. But we never removed the call from a call queue. --- src/rpc/virnetclient.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 0effceb..c99e87c 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1256,7 +1256,12 @@ static int virNetClientIOEventLoop(virNetClientPtr client, /* We're not done, but we're non-blocking */ if (thiscall->nonBlock) { virNetClientIOEventLoopPassTheBuck(client, thiscall); - return thiscall->sentSomeData ? 1 : 0; + if (thiscall->sentSomeData) { + return 1; + } else { + virNetClientCallRemove(&client->waitDispatch, thiscall); + return 0; + } }
if (fds[0].revents & (POLLHUP | POLLERR)) {
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- src/rpc/virnetclient.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index c99e87c..025d270 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1040,6 +1040,7 @@ static bool virNetClientIOEventLoopRemoveDone(virNetClientCallPtr call, VIR_DEBUG("Waking up sleep %p", call); virCondSignal(&call->cond); } else { + VIR_DEBUG("Removing completed call %p", call); if (call->expectReply) VIR_WARN("Got a call expecting a reply but without a waiting thread"); ignore_value(virCondDestroy(&call->cond)); @@ -1070,6 +1071,8 @@ static bool virNetClientIOEventLoopRemoveNonBlocking(virNetClientCallPtr call, if (call->haveThread) { VIR_DEBUG("Waking up sleep %p", call); virCondSignal(&call->cond); + } else { + VIR_DEBUG("Keeping unfinished call %p in the list", call); } return false; } else { @@ -1081,6 +1084,7 @@ static bool virNetClientIOEventLoopRemoveNonBlocking(virNetClientCallPtr call, VIR_DEBUG("Waking up sleep %p", call); virCondSignal(&call->cond); } else { + VIR_DEBUG("Removing call %p", call); if (call->expectReply) VIR_WARN("Got a call expecting a reply but without a waiting thread"); ignore_value(virCondDestroy(&call->cond)); -- 1.7.8.rc3
On Tue, Nov 22, 2011 at 04:32:34PM +0100, Jiri Denemark wrote:
--- src/rpc/virnetclient.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index c99e87c..025d270 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1040,6 +1040,7 @@ static bool virNetClientIOEventLoopRemoveDone(virNetClientCallPtr call, VIR_DEBUG("Waking up sleep %p", call); virCondSignal(&call->cond); } else { + VIR_DEBUG("Removing completed call %p", call); if (call->expectReply) VIR_WARN("Got a call expecting a reply but without a waiting thread"); ignore_value(virCondDestroy(&call->cond)); @@ -1070,6 +1071,8 @@ static bool virNetClientIOEventLoopRemoveNonBlocking(virNetClientCallPtr call, if (call->haveThread) { VIR_DEBUG("Waking up sleep %p", call); virCondSignal(&call->cond); + } else { + VIR_DEBUG("Keeping unfinished call %p in the list", call); } return false; } else { @@ -1081,6 +1084,7 @@ static bool virNetClientIOEventLoopRemoveNonBlocking(virNetClientCallPtr call, VIR_DEBUG("Waking up sleep %p", call); virCondSignal(&call->cond); } else { + VIR_DEBUG("Removing call %p", call); if (call->expectReply) VIR_WARN("Got a call expecting a reply but without a waiting thread"); ignore_value(virCondDestroy(&call->cond));
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Jiri Denemark