Re: [Libvir] Remote patch, 2007/02/19

On Fri, Feb 23, 2007 at 10:37:32AM +0000, Richard W.M. Jones wrote: > Daniel P. Berrange wrote: >> On Wed, Feb 21, 2007 at 06:13:40PM +0000, Richard W.M. Jones wrote: >>> Daniel P. Berrange wrote: >>> >>>> With that in mind I'd venture to suggest we ditch the whole idea of >>>> cookies >>>> completely. >>>> >>>> Every method on the server end is already given a >>>> >>>> 'struct svc_req *req' >>>> >>>> This struct contains a field >>>> >>>> ' SVCXPRT *rq_xprt;' >>>> >>>> Which represents the data transport of the client. And the SVCXPRT struct >>>> has as its first member the ' int xp_sock' which is the socket >>>> associated >>>> with the client. >>>> >>>> So we can trivially & securely map from a client's TCP connetion to the >>>> virConnectPtr without needing any magic cookies. >>> What concerns me here is that xp_sock is just a file descriptor and fds >>> can be reused. It is also an fd that could be any of: >>> * a TCPv6 socket >>> * a TCPv4 socket >>> * a Unix domain socket >>> * on the client side, a socketpair (which on Linux is a funny type of >>> Unix domain socket) >>> So finding something unique about it may be tricky. What happens if two >>> clients connect in succession over the local Unix domain socket? >> I've just noticed that since we are providing our own server side transport >> implementations in sunrpc/svc_{tcp,ext,gnutls}.c we already have a place >> where we can safely put in cleanup hooks. In the 'svctcp_destroy' just >> after we call 'xprt_unregister' we can call out to purge client state >> associated with that FD. This ensures that we cleanup state before any new >> connection can re-use the same FD number. So there's actually no need to >> replace the svc_run() method in this case after all. > I was hoping to avoid this. Note that the contents of the sunrpc/ > subdirectory (the client and server transports) are independent of > libvirt and I hoped to publish them separately so they could be reused > in other places. The other problem is the Unix transport, where we use > the transport from glibc directly. To keep them untied from libvirt one could allow a generic 'void (*cleanup)(void *)' callback to be passed into the create methods for each transport, and simply invoke that from the descructor. We'd need to provide an alternate impl for hte Unix transport too.