Re: [PATCH 4/4] virtiofsd: Implement blocking posix locks

[Stefan Hajnoczi]

On Fri, Nov 15, 2019 at 03:55:43PM -0500, Vivek Goyal wrote:
> diff --git a/contrib/virtiofsd/fuse_lowlevel.c 
> b/contrib/virtiofsd/fuse_lowlevel.c
> index d4a42d9804..f706e440bf 100644
> --- a/contrib/virtiofsd/fuse_lowlevel.c
> +++ b/contrib/virtiofsd/fuse_lowlevel.c
> @@ -183,7 +183,8 @@ int fuse_send_reply_iov_nofree(fuse_req_t req, int error, 
> struct iovec *iov,
>  {
>       struct fuse_out_header out;
>  
> -     if (error <= -1000 || error > 0) {
> +     /* error = 1 has been used to signal client to wait for notificaiton */
> +     if (error <= -1000 || error > 1) {
>               fuse_log(FUSE_LOG_ERR, "fuse: bad error value: %i\n",   error);
>               error = -ERANGE;
>       }

What is this?

> +int fuse_lowlevel_notify_lock(struct fuse_session *se, uint64_t req_id,
> +                           int32_t error)
> +{
> +     struct fuse_notify_lock_out outarg;

Missing = {} initialization to avoid information leaks to the guest.

> @@ -1704,6 +1720,15 @@ int fuse_lowlevel_notify_delete(struct fuse_session 
> *se,
>  int fuse_lowlevel_notify_store(struct fuse_session *se, fuse_ino_t ino,
>                              off_t offset, struct fuse_bufvec *bufv,
>                              enum fuse_buf_copy_flags flags);
> +/**
> + * Notify event related to previous lock request
> + *
> + * @param se the session object
> + * @param req_id the id of the request which requested setlkw

The rest of the code calls this id "unique":

  + * @param req_unique the unique id of the setlkw request

> +    /* Pop an element from queue */
> +    req = vu_queue_pop(dev, q, sizeof(FVRequest), &bad_in_num, &bad_out_num);
> +    if (!req) {
> +        /* TODO: Implement some sort of ring buffer and queue notifications
> +      * on that and send these later when notification queue has space
> +      * available.
> +      */
> +        return -ENOSPC;

Ah, I thought the point of the notifications processing thread was
exactly this case.  It could wake any threads waiting for buffers.

This wakeup could be implemented with a condvar - no ring buffer
necessary.

signature.asc
Description: PGP signature