From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37427) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QNV0a-0006xP-Qa for qemu-devel@nongnu.org; Fri, 20 May 2011 15:05:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QNV0Z-0007DN-CZ for qemu-devel@nongnu.org; Fri, 20 May 2011 15:05:12 -0400 Received: from mail-gx0-f175.google.com ([209.85.161.175]:65259) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QNV0Z-0007DJ-9y for qemu-devel@nongnu.org; Fri, 20 May 2011 15:05:11 -0400 Received: by gxk3 with SMTP id 3so1923545gxk.34 for ; Fri, 20 May 2011 12:05:10 -0700 (PDT) Message-ID: <4DD6BB64.1000001@codemonkey.ws> Date: Fri, 20 May 2011 14:05:08 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <4DD6B777.9020800@us.ibm.com> In-Reply-To: <4DD6B777.9020800@us.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Add support for fd: protocol List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Corey Bryant Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, Tyler C Hicks On 05/20/2011 01:48 PM, Corey Bryant wrote: > sVirt provides SELinux MAC isolation for Qemu guest processes and their > corresponding resources (image files). sVirt provides this support > by labeling guests and resources with security labels that are stored > in file system extended attributes. Some file systems, such as NFS, do > not support the extended attribute security namespace, which is needed > for image file isolation when using the sVirt SELinux security driver > in libvirt. > > The proposed solution entails a combination of Qemu, libvirt, and > SELinux patches that work together to isolate multiple guests' images > when they're stored in the same NFS mount. This results in an > environment where sVirt isolation and NFS image file isolation can both > be provided. > > Currently, Qemu opens an image file in addition to performing the > necessary read and write operations. The proposed solution will move > the open out of Qemu and into libvirt. Once libvirt opens an image > file for the guest, it will pass the file descriptor to Qemu via a > new fd: protocol. > > If the image file resides in an NFS mount, the following SELinux policy > changes will provide image isolation: > > - A new SELinux boolean is created (e.g. virt_read_write_nfs) to > allow Qemu (svirt_t) to only have SELinux read and write > permissions on nfs_t files > > - Qemu (svirt_t) also gets SELinux use permissions on libvirt > (virtd_t) file descriptors > > Following is a sample invocation of Qemu using the fd: protocol: > > qemu -drive file=fd:4,format=qcow2 > > This patch contains the Qemu code to support this solution. I would > like to solicit input from the libvirt community prior to starting > the libvirt patch. > > This patch was tested with the following formats: raw, cow, qcow, > qcow2, vmdk, using the fd: protocol as well as existing file name > support. Non-valid file descriptors were also tested. > > Signed-off-by: Corey Bryant > --- > block/raw-posix.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++------- > qemu-doc.texi | 12 +++++++++ > qemu-options.hx | 8 ++++-- > 3 files changed, 78 insertions(+), 12 deletions(-) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index a95c8d4..6554b06 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -142,7 +142,8 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, > int bdrv_flags, int open_flags) > { > BDRVRawState *s = bs->opaque; > - int fd, ret; > + int fd = -1; > + int ret; > > s->open_flags = open_flags | O_BINARY; > s->open_flags&= ~O_ACCMODE; > @@ -159,15 +160,16 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, > else if (!(bdrv_flags& BDRV_O_CACHE_WB)) > s->open_flags |= O_DSYNC; > > - s->fd = -1; > - fd = qemu_open(filename, s->open_flags, 0644); > - if (fd< 0) { > - ret = -errno; > - if (ret == -EROFS) > - ret = -EACCES; > - return ret; > + if (s->fd == -1) { > + fd = qemu_open(filename, s->open_flags, 0644); > + if (fd< 0) { > + ret = -errno; > + if (ret == -EROFS) > + ret = -EACCES; > + return ret; > + } > + s->fd = fd; > } > - s->fd = fd; > s->aligned_buf = NULL; > > if ((bdrv_flags& BDRV_O_NOCACHE)) { > @@ -224,6 +226,7 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags) > { > BDRVRawState *s = bs->opaque; > > + s->fd = -1; > s->type = FTYPE_FILE; > return raw_open_common(bs, filename, flags, 0); > } > @@ -819,6 +822,50 @@ static BlockDriver bdrv_file = { > .create_options = raw_create_options, > }; > > +static int raw_open_fd(BlockDriverState *bs, const char *filename, int flags) > +{ > + BDRVRawState *s = bs->opaque; > + const char *fd_str; > + int i; > + > + /* extract the file descriptor - fail if it's not fd: */ > + if (!strstart(filename, "fd:",&fd_str)) { > + return -EINVAL; > + } > + > + for (i = 0; fd_str[i] != '\0'; i++) { > + if (!qemu_isdigit(fd_str[i])) > + return -EBADF; This is off CODING_STYLE wise (braces around if), but a better way to do this is to use strtol(), and check for the error condition from it. If it fails, you should call monitor_get_fd(cur_mon, fd_str) as this will allow named fds to be used. Regards, Anthony Liguori