From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58156) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Slzay-0003Yg-C7 for qemu-devel@nongnu.org; Tue, 03 Jul 2012 05:40:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Slzat-0001BT-BM for qemu-devel@nongnu.org; Tue, 03 Jul 2012 05:40:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57718) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Slzat-0001AM-35 for qemu-devel@nongnu.org; Tue, 03 Jul 2012 05:40:27 -0400 Message-ID: <4FF2BDFF.3020405@redhat.com> Date: Tue, 03 Jul 2012 11:40:15 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1340390174-7493-1-git-send-email-coreyb@linux.vnet.ibm.com> <20120626091004.GA14451@redhat.com> <4FE9A0F0.2050809@redhat.com> <20120626175045.2c7011b3@doriath.home> <4FEA37A9.10707@linux.vnet.ibm.com> <4FEA3D9C.8080205@redhat.com> <4FF21A67.8010100@linux.vnet.ibm.com> <4FF2212D.9020608@redhat.com> In-Reply-To: <4FF2212D.9020608@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, Corey Bryant , qemu-devel@nongnu.org, Luiz Capitulino , pbonzini@redhat.com Am 03.07.2012 00:31, schrieb Eric Blake: > On 07/02/2012 04:02 PM, Corey Bryant wrote: > >> Here's another option that Kevin and I discussed today on IRC. I've >> modified a few minor details since the discussion. And Kevin please >> correct me if anything is wrong. >> >> Proposal Four: Pass a set of fds via 'pass-fds'. The group of fds >> should all refer to the same file, but may have different access flags >> (ie. O_RDWR, O_RDONLY). qemu_open can then dup the fd that has the >> matching access mode flags. > > But this means that libvirt has to open a file O_RDWR up front for any > file that it _might_ need qemu to reopen later, and that qemu is now > hanging on to 2 fds per fdset instead of 1 fd for the life of any client > of the fdset. It doesn't have to be up front. There's no reason not to have monitor commands that add or remove fds from a given fdset later. > I see no reason why libvirt can't pass in an O_RDWR fd when qemu only > needs to use an O_RDONLY fd; making qemu insist on an O_RDONLY fd makes > life harder for libvirt. On the other hand, I can see from a safety > standpoint that passing in an O_RDWR fd opens up more potential for > errors than passing in an O_RDONLY fd, Yes, this is exactly my consideration. Having a read-only file opened as O_RDWR gives us the chance to make stupid mistakes. > but if you don't know up front > whether you will ever need to write into a file, then it would be better > to pass in O_RDONLY. At least I don't see it as a security risk: > passing in O_RDWR but setting SELinux permissions on the fd to only > allow read() but not write() via the labeling of the fd may be possible, > so that libvirt could still prevent accidental writes into an O_RDWR > file that starts life only needing to service reads. But this would assume that libvirt knows exactly when a reopen happens, for each current and future qemu version. I wouldn't want to tie qemu's internals so closely to the management application, even if libvirt could probably get it reasonably right (allow rw before sending a monitor command; revoke rw when receiving a QMP event that the commit has completed). It wouldn't work if we had a qemu-initiated ro->rw transition, but I don't think we have one at the moment. >> pass-fds: >> { 'command': 'pass-fds', >> 'data': {'fdsetname': 'str', '*close': 'bool'}, >> 'returns': 'string' } > > This still doesn't solve Dan's complaint that things are not atomic; if > the monitor dies between the pass-fds and the later use of the fdset, we > would need a counterpart monitor command to query what fdsets are > currently known by qemu. If you want a query-fdsets, that should be easy enough. Actually, we might be able to even make the command transactionable. This would of course require blockdev-add to be transactionable as well to be of any use. > And like you pointed out, you didn't make it > clear how a timeout mechanism would be implemented to auto-close fds > that are not consumed in a fixed amount of time - would that be another > optional parameter to 'pass-fds'? Do you think a timeout is helpful? Can't we just drop libvirt's reference automatically if the monitor connection gets closed? The BH/timer thing Corey mentioned is more about the qemu internal problem that during a reopen there may be a short period where the old fd is closed, but the new one not yet opened, so the fdset might need to survive a short period with refcount 0. > Or do we need a way to initially create a set with only one O_RDONLY fd, > but later pass in a new O_RDWR fd but associate it with the existing set > rather than creating a new set (akin to the 'force' option of 'pass-fd' > in proposal two)? As I said above, yes, I think this makes a lot sense. Kevin