From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42339) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHnaZ-00021T-Ka for qemu-devel@nongnu.org; Tue, 28 Jun 2016 03:37:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bHnaV-000067-F8 for qemu-devel@nongnu.org; Tue, 28 Jun 2016 03:37:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39042) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHnaV-00005s-7Q for qemu-devel@nongnu.org; Tue, 28 Jun 2016 03:37:39 -0400 References: <1464712247-11655-2-git-send-email-wexu@redhat.com> <576AAE03.3000403@redhat.com> <576AB14D.7050304@redhat.com> <576AC0F6.3080804@redhat.com> <20160628094532-mutt-send-email-mst@redhat.com> From: Wei Xu Message-ID: <57722939.90300@redhat.com> Date: Tue, 28 Jun 2016 15:37:29 +0800 MIME-Version: 1.0 In-Reply-To: <20160628094532-mutt-send-email-mst@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: "Michael S. Tsirkin" Cc: Michal Privoznik , Aaron Conole , coreyb@linux.vnet.ibm.com, qemu-devel@nongnu.org, armbru@redhat.com, amit.shah@redhat.com, fbl@redhat.com, Jason Wang , silbe@linux.vnet.ibm.com On 2016=E5=B9=B406=E6=9C=8828=E6=97=A5 14:48, Michael S. Tsirkin wrote: > On Thu, Jun 23, 2016 at 12:46:46AM +0800, Wei Xu wrote: >> 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 pat= ch 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= an >>> input type. This is not backwards compatible. At best, you'd want t= o >>> make it optional, but I'm not even convinced you want to add it, sinc= e >>> 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=3Dchar0,path=3D/var/run/openvswitch/vhost-user1,ser= ver >> >> New command line: >> qemu-kvm -add-fd fd=3D3,set=3D2,opaque=3D"/var/run/openvswitch/vhost-u= ser1" >> -chardev socket,id=3Dchar0,path=3D/dev/fdset/3,server >> >> >> Q1. The 'sockfd' is not used anymore, but looks the 'path' parameter i= s >> still mandatory AFAIK, because it's a unix domain socket, which is dif= ferent >> 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 l= ibvirt >> 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. Yes, but libvirt only cares about the creation for all sockets, not sure=20 if this will break the rule and make libvirt considering more beyond=20 it's responsiblility, actually the 'opaque' parameter in the command=20 line is ok for qemu to get the path info and bind it. > > >> Q2. Do you know how can i test it? i'm going to fork a process first a= nd >> create a socket like libvirt, then exec qemu and pass it in, just wond= ering >> 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. I see, thanks. > >> 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 sh= ould >> check if it's started as '/dev/fdset' like in qemu_open(), and just pi= ck the >> 'fd' up, is this enough? should i check the modes? >> >> Thanks, >> Wei >