[Qemu-devel] Re: [V8 PATCH 05/11] virtio-9p: Add support to open a file in chroot environment

[Stefan Hajnoczi]

On Wed, Mar 9, 2011 at 5:15 PM, M. Mohan Kumar <address@hidden> wrote:
> +/* Return opened file descriptor on success or -errno on error */
> +int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *request)
>  {
> -    (void)v9fs_receivefd;
> -    (void)v9fs_write_request;
> +    int fd, sock_error;
> +    qemu_mutex_lock(&fs_ctx->chroot_mutex);
> +    if (fs_ctx->chroot_ioerror) {
> +        fd = -EIO;
> +        goto unlock;
> +    }
> +    if (v9fs_write_request(fs_ctx->chroot_socket, request) < 0) {
> +        fs_ctx->chroot_ioerror = 1;
> +        fd = -EIO;
> +        goto unlock;
> +    }
> +    fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error);
> +    if (fd < 0 && sock_error) {
> +        fs_ctx->chroot_ioerror = 1;
> +    }
> +unlock:
> +    qemu_mutex_unlock(&fs_ctx->chroot_mutex);
> +    return fd;
>  }

If the socket is broken why not just close it?  Right now we're
keeping the file descriptor and have an additional chroot_ioerror
variable to keep track of the fact that we don't want to touch the
socket.

> +/* Helper routine to fill V9fsFileObjectRequest structure */
> +static int fill_fileobjectrequest(V9fsFileObjectRequest *request,
> +                const char *path, FsCred *credp)
> +{
> +    if (strlen(path) >= PATH_MAX) {
> +        return -ENAMETOOLONG;
> +    }
> +    memset(request, 0, sizeof(*request));

Perhaps remove this since request is a big struct (two PATH_MAX
buffers) or is it necessary?

> +    strcpy(request->path.path, path);
> +    if (credp) {
> +        request->data.mode = credp->fc_mode;
> +        request->data.uid = credp->fc_uid;
> +        request->data.gid = credp->fc_gid;
> +        request->data.dev = credp->fc_rdev;
> +    }
> +    return 0;
> +}
> +
> +static int passthrough_request(FsContext *fs_ctx, const char *old_path,
> +                const char *path, int flags, FsCred *credp, int type)
> +{
> +    V9fsFileObjectRequest request;
> +    int retval;
> +
> +    retval = fill_fileobjectrequest(&request, path, credp);

This function could also handle old_path, flags, and type.  It seems
to only fill half the request struct at the moment.

> +    if (retval < 0) {
> +        errno = -retval;
> +        return -1;
> +    }
> +    if (old_path) {
> +        if (strlen(old_path) >= PATH_MAX) {
> +            errno = -ENAMETOOLONG;

errno = ENAMETOOLONG;

Stefan