Re: [Qemu-devel] [PATCH] Add support for fd: protocol

[Anthony Liguori <anthony@codemonkey.ws>]

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<coreyb@linux.vnet.ibm.com>
> ---
>   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