[libvirt] [PATCH] rpc: ensure the value of nrequests for client not to be negative

[Daniel P. Berrange]

On Thu, Aug 25, 2011 at 10:07:36PM +0800, Guannan Ren wrote:
> The value of nrequests should always be zero or positive
> but in the case of stream, when error occurrs, it could be negative
> and makes the client socket fd neither writable nor readable. The
> case will lead to hang on libvirt client side.
> This patch aim to fix the problem.

What is the scenario you use to demonstrate the original problem ?

IMHO the fix below is not right, because it is merely patching
up the problems caused by the bug, rather than actually fixing
the bug.

> ---
>  src/rpc/virnetserverclient.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
> index a73b06d..882ba77 100644
> --- a/src/rpc/virnetserverclient.c
> +++ b/src/rpc/virnetserverclient.c
> @@ -78,8 +78,8 @@ struct _virNetServerClient
>       * ie RPC calls in progress. Does not count
>       * async events which are not used for
>       * throttling calculations */
> -    size_t nrequests;
> -    size_t nrequests_max;
> +    ssize_t nrequests;
> +    ssize_t nrequests_max;
>      /* Zero or one messages being received. Zero if
>       * nrequests >= max_clients and throttling */
>      virNetMessagePtr rx;
> @@ -889,6 +889,10 @@ virNetServerClientDispatchWrite(virNetServerClientPtr client)
>                  (msg->header.type == VIR_NET_STREAM &&
>                   msg->header.status != VIR_NET_CONTINUE)) {
>                  client->nrequests--;
> +
> +                if (client->nrequests < 0)
> +                    client->nrequests = 0;
> +
>                  /* See if the recv queue is currently throttled */
>                  if (!client->rx &&
>                      client->nrequests < client->nrequests_max) {


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 :|