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

[Wei Xu]

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.

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?

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