Daniel P. Berrange wrote:
On Fri, Jan 16, 2009 at 12:49:06PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange(a)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