Re: [Qemu-devel] [RFC Patch 1/3] chardev: add new socket fd parameter for unix socket

[Michael S. Tsirkin]

On Thu, Jun 23, 2016 at 12:46:46AM +0800, Wei Xu wrote:
> On 2016年06月22日 23:39, Eric Blake wrote:
> > On 06/22/2016 09:25 AM, Wei Xu wrote:
> > > There have been comments on this patch, but i forgot adding this patch to
> > > the list, just forward it again.
> > > 
> > > When manage VMs via libvirt, qemu ofter runs with limited permission,
> > > thus qemu can't create a file/socket, this patch is to  add a new
> > > parameter 'sockfd' to accept fd opened and passed in from libvirt.
> > > 
> > > Signed-off-by: Wei Xu <address@hidden>
> > > ---
> > >   qapi-schema.json | 3 ++-
> > >   qemu-char.c      | 3 +++
> > >   2 files changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index 8483bdf..e9f0268 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -2921,7 +2921,8 @@
> > >   ##
> > >   { 'struct': 'UnixSocketAddress',
> > >     'data': {
> > > -    'path': 'str' } }
> > > +    'path': 'str',
> > > +    'sockfd': 'int32' } }
> > 
> > Missing documentation.
> > 
> > This makes the new 'sockfd' parameter mandatory, but SocketAddress is an
> > input type.  This is not backwards compatible.  At best, you'd want to
> > make it optional, but I'm not even convinced you want to add it, since
> > we already can use the magic /dev/fdset/nnn in 'path' to pass an
> > arbitrary fd if the underlying code uses qemu_open().
> > 
> Thanks for commenting it again, i was going to forward it to the list and
> ask some questions.:)
> 
> Actually i'm going to try the magic way as you suggested, just a few
> questions about that.
> 
> Command line change should be like this according to my understanding.
> 
> Current command line:
> -chardev socket,id=char0,path=/var/run/openvswitch/vhost-user1,server
> 
> New command line:
> qemu-kvm -add-fd fd=3,set=2,opaque="/var/run/openvswitch/vhost-user1"
> -chardev socket,id=char0,path=/dev/fdset/3,server
> 
> 
> Q1. The 'sockfd' is not used anymore, but looks the 'path' parameter is
> still mandatory AFAIK, because it's a unix domain socket, which is different
> with a network tcp/udp socket, it works like a pipe file for local
> communication only, and the 'path' parameter is a must-have condition before
> a real connect could be established, it needs a bind() operation to a
> specific path before either connect() or listen(), this is caused by libvirt
> only takes the responsibility to creates a socket and pass the 'fd' in only,
> there is nothing more about the bind(), thus i think qemu will have to
> bind() it by itself, i'm thinking maybe 'opaque' can be used for this case.

I think libvirt will have to bind it.
Passing in an unbound socket wouldn't make sense.


> Q2. Do you know how can i test it? i'm going to fork a process first and
> create a socket like libvirt, then exec qemu and pass it in, just wondering
> how can i map it to '/dev/fdset/' directory after created the socket?

/dev/fdset/ is QEMU syntax to pass fd numbers where path is normally
used.

> Q3.
> -------------------------------------------------------------------
> Daniel's comment before:
> 'Path' refers to a UNIX domain socket path, so the code won't be using
> qemu_open() - it'll be using the sockets APIs to open/create a UNIX
> socket. I don't think qemu_open() would even work with a socket FD,
> since it'll be trying to set various modes on the FD via fcntl() that
> don't work with sockets AFAIK
> -------------------------------------------------------------------
> Seems what i should call is qemu_socket() to fill in qemu_open(), i should
> check if it's started as '/dev/fdset' like in qemu_open(), and just pick the
> 'fd' up, is this enough? should i check the modes?
> 
> Thanks,
> Wei