[Qemu-devel] Re: [V7 PATCH 7/9] virtio-9p: Support for creating special files

[Stefan Hajnoczi]

On Fri, Mar 4, 2011 at 9:25 AM, M. Mohan Kumar <address@hidden> wrote:
> +static int chroot_do_create_special(V9fsFileObjectRequest *request)
> +{
> +    int cur_uid, cur_gid;
> +    int retval = -1;
> +
> +    cur_uid = geteuid();
> +    cur_gid = getegid();
> +
> +    if (setfsuid(request->data.uid) < 0) {
> +        return -errno;
> +    }
> +    if (setfsgid(request->data.gid) < 0) {
> +        retval = -errno;
> +        goto unset_uid;
> +    }
> +
> +    switch (request->data.type) {
> +    case T_MKDIR:
> +        retval = mkdir(request->path.path, request->data.mode);
> +        break;
> +    case T_SYMLINK:
> +        retval = symlink(request->path.old_path, request->path.path);
> +        break;
> +    case T_LINK:
> +        retval = link(request->path.old_path, request->path.path);
> +        break;
> +    default:
> +        retval = mknod(request->path.path, request->data.mode,
> +                        request->data.dev);
> +        break;
> +    }
> +
> +    if (retval < 0) {
> +        retval = -errno;
> +    }
> +    setfsgid(cur_gid);
> +unset_uid:
> +    setfsuid(cur_uid);
> +    return retval;
> +}

It would be nice to take this one step further and move file create
and open here too.  The prototype we need is:

static int chroot_handle_request(V9fsFileObjectRequest *request, int *fd)
{
    *fd = -1;

It returns 0 on success or -errno and *fd >= 0 if a file descriptor
was opened and -1 otherwise.

This function becomes the main request processing function called from
v9fs_chroot() and the switch statement there can be eliminated.

Sending the response back to QEMU then gets a cleaned up prototype:
chroot_sendfd(int chroot_sock, int result, int fd) where result is 0
on success or -errno and fd >= 0 if present or -1 if not.

> +int v9fs_create_special(FsContext *fs_ctx, V9fsFileObjectRequest *request)
> +{
> +    int retval, sock_error;
> +    qemu_mutex_lock(&fs_ctx->chroot_mutex);
> +    if (fs_ctx->chroot_ioerror) {
> +        retval = -EIO;
> +        goto unlock;
> +    }
> +    if (v9fs_write_request(fs_ctx->chroot_socket, request) < 0) {
> +        fs_ctx->chroot_ioerror = 1;
> +        retval = -EIO;
> +        goto unlock;
> +    }
> +    retval = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error);
> +    if (retval < 0 && sock_error) {
> +        fs_ctx->chroot_ioerror = 1;
> +    }
> +unlock:
> +    qemu_mutex_unlock(&fs_ctx->chroot_mutex);
> +    return retval;
> +}

This function is a duplicate of v9fs_request().  Can't there be just
one function?

Stefan