From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34497) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFlIl-0000FD-0Y for qemu-devel@nongnu.org; Wed, 22 Jun 2016 12:46:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bFlIi-0007lm-AF for qemu-devel@nongnu.org; Wed, 22 Jun 2016 12:46:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50479) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFlIi-0007li-2E for qemu-devel@nongnu.org; Wed, 22 Jun 2016 12:46:52 -0400 References: <1464712247-11655-2-git-send-email-wexu@redhat.com> <576AAE03.3000403@redhat.com> <576AB14D.7050304@redhat.com> From: Wei Xu Message-ID: <576AC0F6.3080804@redhat.com> Date: Thu, 23 Jun 2016 00:46:46 +0800 MIME-Version: 1.0 In-Reply-To: <576AB14D.7050304@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC Patch 1/3] chardev: add new socket fd parameter for unix socket List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: "Daniel P. Berrange" , Michal Privoznik , armbru@redhat.com, amit.shah@redhat.com, silbe@linux.vnet.ibm.com, Aaron Conole , fbl@redhat.com, Jason Wang , coreyb@linux.vnet.ibm.com On 2016=E5=B9=B406=E6=9C=8822=E6=97=A5 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 >> --- >> 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 a= n > 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=20 and ask some questions.:) Actually i'm going to try the magic way as you suggested, just a few=20 questions about that. Command line change should be like this according to my understanding. Current command line: -chardev socket,id=3Dchar0,path=3D/var/run/openvswitch/vhost-user1,server New command line: qemu-kvm -add-fd fd=3D3,set=3D2,opaque=3D"/var/run/openvswitch/vhost-user= 1" -chardev socket,id=3Dchar0,path=3D/dev/fdset/3,server Q1. The 'sockfd' is not used anymore, but looks the 'path' parameter is=20 still mandatory AFAIK, because it's a unix domain socket, which is=20 different with a network tcp/udp socket, it works like a pipe file for=20 local communication only, and the 'path' parameter is a must-have=20 condition before a real connect could be established, it needs a bind()=20 operation to a specific path before either connect() or listen(), this=20 is caused by libvirt only takes the responsibility to creates a socket=20 and pass the 'fd' in only, there is nothing more about the bind(), thus=20 i think qemu will have to bind() it by itself, i'm thinking maybe=20 '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=20 create a socket like libvirt, then exec qemu and pass it in, just=20 wondering how can i map it to '/dev/fdset/' directory after created the=20 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=20 should check if it's started as '/dev/fdset' like in qemu_open(), and=20 just pick the 'fd' up, is this enough? should i check the modes? Thanks, Wei