From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57525) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbYg2-0005hT-N9 for qemu-devel@nongnu.org; Mon, 04 Jun 2012 10:54:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SbYfw-00029v-7s for qemu-devel@nongnu.org; Mon, 04 Jun 2012 10:54:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48457) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbYfv-00029J-VD for qemu-devel@nongnu.org; Mon, 04 Jun 2012 10:54:32 -0400 Message-ID: <4FCCCC22.2050000@redhat.com> Date: Mon, 04 Jun 2012 16:54:26 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1338815410-24890-1-git-send-email-coreyb@linux.vnet.ibm.com> <1338815410-24890-3-git-send-email-coreyb@linux.vnet.ibm.com> In-Reply-To: <1338815410-24890-3-git-send-email-coreyb@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Corey Bryant Cc: aliguori@us.ibm.com, eblake@redhat.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com Am 04.06.2012 15:10, schrieb Corey Bryant: > The main goal of this patch series is to enable isolation of guest > images that are stored on the same NFS mount. This can be achieved > if the management application opens files for QEMU, and QEMU is > restricted from opening files. > > This patch adds support to the block layer open paths to dup(X) a > pre-opened file descriptor if the filename is of the format > /dev/fd/X. > > One nice thing about this approach is that no new SELinux policy is > required to prevent open of NFS files (files with type nfs_t). The > virt_use_nfs boolean type simply needs to be set to false, and open > will be prevented (yet dup will be allowed). For example: > > # setsebool virt_use_nfs 0 > # getsebool virt_use_nfs > virt_use_nfs --> off > > Signed-off-by: Corey Bryant > --- > block.c | 15 +++++++++++++++ > block/raw-posix.c | 20 ++++++++++---------- > block/raw-win32.c | 4 ++-- > block/vdi.c | 5 +++-- > block/vmdk.c | 21 +++++++++------------ > block/vpc.c | 2 +- > block/vvfat.c | 21 +++++++++++---------- > block_int.h | 13 +++++++++++++ > 8 files changed, 64 insertions(+), 37 deletions(-) > > diff --git a/block.c b/block.c > index af2ab4f..8416313 100644 > --- a/block.c > +++ b/block.c > @@ -102,6 +102,21 @@ static BlockDriverState *bs_snapshots; > /* If non-zero, use only whitelisted block drivers */ > static int use_bdrv_whitelist; > > +int file_open(const char *filename, int flags, mode_t mode) > +{ > +#ifndef _WIN32 > + int fd; > + const char *p; > + > + if (strstart(filename, "/dev/fd/", &p)) { > + fd = atoi(p); > + return dup(fd); > + } > +#endif > + > + return qemu_open(filename, flags, mode); > +} What's the reason for having separate file_open() and qemu_open() functions? Are there places where you don't want allow to use /dev/fd? Otherwise I would have expected that we just extend qemu_open(). I'd have the remaining open -> qemu_open conversions as a separate patch then. They do not only add fd support as advertised in the commit message, but also add O_CLOEXEC. I don't think that's bad, these are likely bonus bug fixes, but they should be mentioned in the commit message. Kevin