diff mbox series

virtiofs: Fail dax mount if device does not support it

Message ID 20210209224754.GG3171@redhat.com (mailing list archive)
State New, archived
Headers show
Series virtiofs: Fail dax mount if device does not support it | expand

Commit Message

Vivek Goyal Feb. 9, 2021, 10:47 p.m. UTC
Right now "mount -t virtiofs -o dax myfs /mnt/virtiofs" succeeds even
if filesystem deivce does not have a cache window and hence DAX can't
be supported.

This gives a false sense to user that they are using DAX with virtiofs
but fact of the matter is that they are not.

Fix this by returning error if dax can't be supported and user has asked
for it.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/fuse/virtio_fs.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Stefan Hajnoczi Feb. 10, 2021, 2:58 p.m. UTC | #1
On Tue, Feb 09, 2021 at 05:47:54PM -0500, Vivek Goyal wrote:
> Right now "mount -t virtiofs -o dax myfs /mnt/virtiofs" succeeds even
> if filesystem deivce does not have a cache window and hence DAX can't
> be supported.
> 
> This gives a false sense to user that they are using DAX with virtiofs
> but fact of the matter is that they are not.
> 
> Fix this by returning error if dax can't be supported and user has asked
> for it.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  fs/fuse/virtio_fs.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> Index: redhat-linux/fs/fuse/virtio_fs.c
> ===================================================================
> --- redhat-linux.orig/fs/fuse/virtio_fs.c	2021-02-04 10:40:21.704370721 -0500
> +++ redhat-linux/fs/fuse/virtio_fs.c	2021-02-09 15:56:45.693653979 -0500
> @@ -1324,8 +1324,15 @@ static int virtio_fs_fill_super(struct s
>  
>  	/* virtiofs allocates and installs its own fuse devices */
>  	ctx->fudptr = NULL;
> -	if (ctx->dax)
> +	if (ctx->dax) {
> +		if (!fs->dax_dev) {
> +			err = -EINVAL;
> +			pr_err("virtio-fs: dax can't be enabled as filesystem"
> +			       " device does not support it.\n");
> +			goto err_free_fuse_devs;
> +		}
>  		ctx->dax_dev = fs->dax_dev;
> +	}
>  	err = fuse_fill_super_common(sb, ctx);
>  	if (err < 0)
>  		goto err_free_fuse_devs;

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Vivek Goyal March 5, 2021, 1:34 p.m. UTC | #2
On Tue, Feb 09, 2021 at 05:47:54PM -0500, Vivek Goyal wrote:
> Right now "mount -t virtiofs -o dax myfs /mnt/virtiofs" succeeds even
> if filesystem deivce does not have a cache window and hence DAX can't
> be supported.
> 
> This gives a false sense to user that they are using DAX with virtiofs
> but fact of the matter is that they are not.
> 
> Fix this by returning error if dax can't be supported and user has asked
> for it.

Hi Miklos,

Did you get a chance to look at this patch.

Vivek

> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  fs/fuse/virtio_fs.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> Index: redhat-linux/fs/fuse/virtio_fs.c
> ===================================================================
> --- redhat-linux.orig/fs/fuse/virtio_fs.c	2021-02-04 10:40:21.704370721 -0500
> +++ redhat-linux/fs/fuse/virtio_fs.c	2021-02-09 15:56:45.693653979 -0500
> @@ -1324,8 +1324,15 @@ static int virtio_fs_fill_super(struct s
>  
>  	/* virtiofs allocates and installs its own fuse devices */
>  	ctx->fudptr = NULL;
> -	if (ctx->dax)
> +	if (ctx->dax) {
> +		if (!fs->dax_dev) {
> +			err = -EINVAL;
> +			pr_err("virtio-fs: dax can't be enabled as filesystem"
> +			       " device does not support it.\n");
> +			goto err_free_fuse_devs;
> +		}
>  		ctx->dax_dev = fs->dax_dev;
> +	}
>  	err = fuse_fill_super_common(sb, ctx);
>  	if (err < 0)
>  		goto err_free_fuse_devs;
Miklos Szeredi March 5, 2021, 2:48 p.m. UTC | #3
On Fri, Mar 5, 2021 at 2:34 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> On Tue, Feb 09, 2021 at 05:47:54PM -0500, Vivek Goyal wrote:
> > Right now "mount -t virtiofs -o dax myfs /mnt/virtiofs" succeeds even
> > if filesystem deivce does not have a cache window and hence DAX can't
> > be supported.
> >
> > This gives a false sense to user that they are using DAX with virtiofs
> > but fact of the matter is that they are not.
> >
> > Fix this by returning error if dax can't be supported and user has asked
> > for it.
>
> Hi Miklos,
>
> Did you get a chance to look at this patch.

Thanks for the reminder.  Pushed out now.

Miklos
diff mbox series

Patch

Index: redhat-linux/fs/fuse/virtio_fs.c
===================================================================
--- redhat-linux.orig/fs/fuse/virtio_fs.c	2021-02-04 10:40:21.704370721 -0500
+++ redhat-linux/fs/fuse/virtio_fs.c	2021-02-09 15:56:45.693653979 -0500
@@ -1324,8 +1324,15 @@  static int virtio_fs_fill_super(struct s
 
 	/* virtiofs allocates and installs its own fuse devices */
 	ctx->fudptr = NULL;
-	if (ctx->dax)
+	if (ctx->dax) {
+		if (!fs->dax_dev) {
+			err = -EINVAL;
+			pr_err("virtio-fs: dax can't be enabled as filesystem"
+			       " device does not support it.\n");
+			goto err_free_fuse_devs;
+		}
 		ctx->dax_dev = fs->dax_dev;
+	}
 	err = fuse_fill_super_common(sb, ctx);
 	if (err < 0)
 		goto err_free_fuse_devs;