[libvirt] PATCH: 8/25: Concurrent dispatch of RPC methods

Cole Robinson crobinso at redhat.com
Fri Jan 16 15:18:32 UTC 2009


Daniel P. Berrange wrote:
> On Fri, Jan 16, 2009 at 12:49:06PM +0100, Jim Meyering wrote:
>> "Daniel P. Berrange" <berrange at redhat.com> wrote:
>>> This patch re-writes the code for dispatching RPC calls in the
>>> remote driver to allow use from multiple threads. Only one thread
>>> is allowed to send/recv on the socket at a time though. If another
>>> thread comes along it will put itself on a queue and go to sleep.
>>> The first thread may actually get around to transmitting the 2nd
>>> thread's request while it is waiting for its own reply. It may
>>> even get the 2nd threads reply, if its own RPC call is being really
>>> slow. So when a thread wakes up from sleeping, it has to check
>>> whether its own RPC call has already been processed. Likewise when
>>> a thread owning the socket finishes with its own wor, it may have
>>> to pass the buck to another thread. The upshot of this, is that
>>> we have mutliple RPC calls executing in parallel, and requests+reply
>>> are no longer guarenteed to be FIFO on the wire if talking to a new
>>> enough server.
>>>
>>> This refactoring required use of a self-pipe/poll trick for sync
>>> between threads, but fortunately gnulib now provides this on Windows
>>> too, so there's no compatability problem there.

<snip>

> 
>>> +        goto error;
>>> +    }
>>> +    xdr_destroy (&xdr);
>>> +
>>> +    return rv;
>>> +
>>> +error:
>>> +    VIR_FREE(ret);
>>> +    return NULL;
>> The above should free rv, not ret:
>>
>>        VIR_FREE(rv);
> 
> Doh, bad naming convention for arguments - we always use 'ret' for return
> values. I should rename the argument to 'reply' since its what contains
> the RPC reply, so we can use the normal convention of 'ret' for return
> value.
> 

I actually just tracked this down independently: the above bug was
crashing virt-manager, where we incorrectly were passing 'None' to
interfaceStats occasionally. So, good catch Jim :)

Thanks,
Cole




More information about the libvir-list mailing list