On Tue, Nov 01, 2011 at 02:14:54PM +0100, Michal Privoznik wrote:
If daemon is using SASL it reads client data into a cache. This cache is big (usually 65KB) and can thus contain 2 or more messages. However, on socket event we can dispatch only one message. So if we read two messages at once, the second will not be dispatched as the socket event goes away with filling the cache. Moreover, when dispatching the cache we need to remember to take care of client max requests limit. --- src/rpc/virnetserverclient.c | 34 +++++++++++++++++++++++++++++++++- 1 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 1256f0f..69af746 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -71,6 +71,8 @@ struct _virNetServerClient virNetTLSSessionPtr tls; #if HAVE_SASL virNetSASLSessionPtr sasl; + int sasl_timer; /* Timer to be fired upon cached data, + so we jump out from poll() immediately */ #endif
Agreed with other comments that this shouldn't be #if HAVE_SASL protected - we might someday hit this in other cases. Just call it sockTimer or something like that.
@@ -204,6 +220,19 @@ static void virNetServerClientUpdateEvent(virNetServerClientPtr client) mode = virNetServerClientCalculateHandleMode(client);
virNetSocketUpdateIOCallback(client->sock, mode); +#ifdef HAVE_SASL + if (client->nrequests < client->nrequests_max &&
This line is bogus. We set client->rx to non-NULL, if-and-only-if client->nrequests < client->nrequests_max. We don't want to duplicate this check here.
+ client->rx && + virNetSocketHasCachedData(client->sock)) { + if (client->sasl_timer) + virEventUpdateTimeout(client->sasl_timer, 0); + else + client->sasl_timer = virEventAddTimeout(0, + virNetServerClientDispatchReadTimerFunc, + client, + NULL); + } +#endif
Missing a check for failure of virEventAddTimeout. I would have unconditionally setup the timer with period -1, during the constructor virNetServerClientNew. Then we can just use virEventUpdateTimeout which is guaranteed to not fail. Regards, 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 :|