From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55723) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHmot-0001UB-B3 for qemu-devel@nongnu.org; Tue, 28 Jun 2016 02:48:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bHmom-0004Qe-PJ for qemu-devel@nongnu.org; Tue, 28 Jun 2016 02:48:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41873) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHmom-0004Q3-FB for qemu-devel@nongnu.org; Tue, 28 Jun 2016 02:48:20 -0400 Date: Tue, 28 Jun 2016 09:48:14 +0300 From: "Michael S. Tsirkin" Message-ID: <20160628094532-mutt-send-email-mst@redhat.com> References: <1464712247-11655-2-git-send-email-wexu@redhat.com> <576AAE03.3000403@redhat.com> <576AB14D.7050304@redhat.com> <576AC0F6.3080804@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <576AC0F6.3080804@redhat.com> 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: Wei Xu Cc: Eric Blake , qemu-devel@nongnu.org, Michal Privoznik , Aaron Conole , coreyb@linux.vnet.ibm.com, armbru@redhat.com, amit.shah@redhat.com, fbl@redhat.com, Jason Wang , silbe@linux.vnet.ibm.com 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 pa= tch to > > > the list, just forward it again. > > >=20 > > > When manage VMs via libvirt, qemu ofter runs with limited permissio= n, > > > 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. > > >=20 > > > Signed-off-by: Wei Xu > > > --- > > > qapi-schema.json | 3 ++- > > > qemu-char.c | 3 +++ > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > >=20 > > > 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' } } > >=20 > > Missing documentation. > >=20 > > 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(). > >=20 > Thanks for commenting it again, i was going to forward it to the list a= nd > ask some questions.:) >=20 > Actually i'm going to try the magic way as you suggested, just a few > questions about that. >=20 > Command line change should be like this according to my understanding. >=20 > Current command line: > -chardev socket,id=3Dchar0,path=3D/var/run/openvswitch/vhost-user1,serv= er >=20 > New command line: > qemu-kvm -add-fd fd=3D3,set=3D2,opaque=3D"/var/run/openvswitch/vhost-us= er1" > -chardev socket,id=3Dchar0,path=3D/dev/fdset/3,server >=20 >=20 > 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 diff= erent > 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 b= efore > a real connect could be established, it needs a bind() operation to a > specific path before either connect() or listen(), this is caused by li= bvirt > 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 c= ase. 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 an= d > create a socket like libvirt, then exec qemu and pass it in, just wonde= ring > 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 sho= uld > check if it's started as '/dev/fdset' like in qemu_open(), and just pic= k the > 'fd' up, is this enough? should i check the modes? >=20 > Thanks, > Wei