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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list