From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50773) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfdW3-0000DZ-Gm for qemu-devel@nongnu.org; Fri, 15 Jun 2012 16:53:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SfdW1-0002R0-Mb for qemu-devel@nongnu.org; Fri, 15 Jun 2012 16:53:11 -0400 Received: from e6.ny.us.ibm.com ([32.97.182.146]:33622) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfdW1-0002QN-ID for qemu-devel@nongnu.org; Fri, 15 Jun 2012 16:53:09 -0400 Received: from /spool/local by e6.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 15 Jun 2012 16:53:05 -0400 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 71A8338C801C for ; Fri, 15 Jun 2012 16:49:14 -0400 (EDT) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q5FKnEhn34406490 for ; Fri, 15 Jun 2012 16:49:14 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q5FKnD6C008356 for ; Fri, 15 Jun 2012 17:49:14 -0300 Message-ID: <4FDB9FC8.4020305@linux.vnet.ibm.com> Date: Fri, 15 Jun 2012 16:49:12 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1339689305-27031-1-git-send-email-coreyb@linux.vnet.ibm.com> <1339689305-27031-4-git-send-email-coreyb@linux.vnet.ibm.com> <4FDB51E8.8060406@redhat.com> <4FDB7C1B.8020802@linux.vnet.ibm.com> <4FDB82F9.8020708@redhat.com> <4FDB8AAC.3030002@linux.vnet.ibm.com> <4FDB9477.9060709@redhat.com> In-Reply-To: <4FDB9477.9060709@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 3/5] osdep: Enable qemu_open to dup pre-opened fd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Kevin Wolf , aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, pbonzini@redhat.com On 06/15/2012 04:00 PM, Eric Blake wrote: > On 06/15/2012 01:19 PM, Corey Bryant wrote: > >>>> There are some flags that I don't think we'll be able to change. For >>>> example: O_RDONLY, O_WRONLY, O_RDWR. I assume libvirt would open all >>>> files O_RDWR. >>> >>> I think we need to check all of them and fail qemu_open() if they don't >>> match. Those that qemu can change, should be just changed, of course. >>> >> >> Ok. I remember a scenario where QEMU opens a file read-only (perhaps to >> check headers and determine the file format) before re-opening it >> read-write. Perhaps this is only when format= isn't specified with >> -drive. I'm thinking we may need to change flags to read-write where >> they used to be read-only, in some circumstances. > > In those situations, libvirt would pass fd with O_RDWR, and qemu_open() > would be fine requesting O_RDONLY the first time (subset is okay), and > O_RDWR the second time. Where you have to error out is where libvirt > passes O_RDONLY but qemu wants O_RDWR, and so forth. > I'll plan on going with this approach. > >>> >>> In which scenario would any client break if we set FD_CLOEXEC? I don't >>> think compatibility means we can't fix any bugs. >>> >> >> I don't know if it breaks any client. Maybe it's not a compatibility >> error. It dopes change behavior down the line though. If you think s/dopes/does >> it's ok to set FD_CLOEXEC for getfd too, then I'm happy to do it. > > The only case that a client might break is if there were a way to pass > an fd into qemu and then intentionally see that fd in a child process of > qemu. But in the case of 'migrate fd:nnn', you aren't spawning a child > process, and even in the case of 'migrate exec:command' (which libvirt > no longer uses if fd:nnn works), I don't see how the client could have > ever intentionally tried to use 'getfd' in advance to pass an extra fd > for use inside the 'exec:command' child. Besides, before 'pass-fd' was > around, how would the management app triggering the 'exec:command' even > know what fd number would accidentally be inherited into the > exec:command child? I think it is pretty much a straight bug-fix for > 'getfd' to always set FD_CLOEXEC, and preferably set it atomically via > MSG_CMSG_CLOEXEC. > Alright, I'll go ahead and make this update in the next version of the patch series. Thanks for all the input! -- Regards, Corey