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