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

[Wei Xu <wexu@redhat.com>]

Anyone can help on this, or at least help me with question 2 about how 
to test it using 'add-fd' command line?

Q2. Does anybody know how can i test it? i'm going to fork a process 
firstly and create a socket like libvirt, then exec qemu and pass it in, 
just wondering how can i map the socket to '/dev/fdset/' directory after 
created the socket? Another issue is if 'set=2' is a right usage for a 
socket fd, any comment?

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


On 2016年06月23日 00:46, 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 <wexu@redhat.com>
>>> ---
>>>   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