From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:60452) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbZsc-0003S4-7u for qemu-devel@nongnu.org; Mon, 04 Jun 2012 12:11:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SbZsV-0000G8-O5 for qemu-devel@nongnu.org; Mon, 04 Jun 2012 12:11:41 -0400 Received: from e7.ny.us.ibm.com ([32.97.182.137]:36542) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbZsV-0000At-Ju for qemu-devel@nongnu.org; Mon, 04 Jun 2012 12:11:35 -0400 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 4 Jun 2012 12:11:28 -0400 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id E160B38C8543 for ; Mon, 4 Jun 2012 12:07:32 -0400 (EDT) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q54G7Dur10420412 for ; Mon, 4 Jun 2012 12:07:14 -0400 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q54Lc5Ph022730 for ; Mon, 4 Jun 2012 17:38:06 -0400 Message-ID: <4FCCDD2E.3050705@linux.vnet.ibm.com> Date: Mon, 04 Jun 2012 12:07:10 -0400 From: Corey Bryant 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> <4FCCCC22.2050000@redhat.com> In-Reply-To: <4FCCCC22.2050000@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed 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: Kevin Wolf Cc: aliguori@us.ibm.com, eblake@redhat.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com On 06/04/2012 10:54 AM, Kevin Wolf wrote: > 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'm not sure there's any good reason to have a separate file_open() vs just adding this code to qemu_open(). I followed the lead of Anthony's prototype which used file_open() so he may have a reason. Otherwise I'll move the code to qemu_open() in v2. > > 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. Yes good point. I'll be sure to note the addition of O_CLOEXEC for these cases in the commit message. -- Regards, Corey