Re: [Virtio-fs] [PATCH v2] virtiofsd: send reply correctly on read failure

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Eryu Guan (eguan@linux.alibaba.com) wrote:
> Currently when a lo_read() operation fails, we don't send the failure
> back to fuse client, and read(2) operation from guest kernel would hang
> on waiting for the reply.
> 
> This is easily triggered by a direct read with non-aligned length.
> 
> Fix it by detecting preadv(2) error in virtio_send_data_iov(), and
> teaching fuse_reply_data() to reply error on error case.
> 
> Reviewed-by: Liu Bo <bo.liu@linux.alibaba.com>
> Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Thank you, I've added that to my dev tree.
> ---
> v2:
> - return postive error number on read error
> - also return postive EIO in short read case
> - drop "-1" -> "-EOPNOTSUPP" update, which should be in a sperate patch
> 
>  contrib/virtiofsd/fuse_virtio.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/contrib/virtiofsd/fuse_virtio.c b/contrib/virtiofsd/fuse_virtio.c
> index ca988aac923f..7337f2a2c3ab 100644
> --- a/contrib/virtiofsd/fuse_virtio.c
> +++ b/contrib/virtiofsd/fuse_virtio.c
> @@ -333,8 +333,13 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch,
>                  ret = preadv(buf->buf[0].fd, in_sg_ptr, in_sg_cpy_count, buf->buf[0].pos);
>  
>                  if (se->debug)
> -                        fprintf(stderr, "%s: preadv_res=%d len=%zd\n",
> -                                __func__, ret, len);
> +                        fprintf(stderr, "%s: preadv_res=%d(%s) len=%zd\n",
> +                                __func__, ret, strerror(errno), len);
> +                if (ret == -1) {
> +                        ret = errno;
> +                        free(in_sg_cpy);
> +                        goto err;
> +                }
>                  if (ret < len && ret) {
>                          if (se->debug)
>                                  fprintf(stderr, "%s: ret < len\n", __func__);
> @@ -356,7 +361,7 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch,
>                  if (ret != len) {
>                          if (se->debug)
>                                  fprintf(stderr, "%s: ret!=len\n", __func__);
> -                        ret = -EIO;
> +                        ret = EIO;
>                          free(in_sg_cpy);
>                          goto err;
>                  }
> @@ -379,7 +384,8 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch,
>          vu_queue_notify(&se->virtio_dev->dev, q);
>  
>  err:
> -        ch->qi->reply_sent = true;
> +        if (ret == 0)
> +                ch->qi->reply_sent = true;
>  
>          return ret;
>  }
> -- 
> 2.14.4.44.g2045bb6
> 
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK