Re: [PATCH v2 2/3] virtiofs: export filesystem tags through sysfs

On Thu, Feb 08, 2024 at 02:32:10PM -0500, Stefan Hajnoczi wrote:
> The virtiofs filesystem is mounted using a "tag" which is exported by
> the virtiofs device:
> 
>   # mount -t virtiofs <tag> /mnt
> 
> The virtiofs driver knows about all the available tags but these are
> currently not exported to user space.
> 
> People have asked for these tags to be exported to user space. Most
> recently Lennart Poettering has asked for it as he wants to scan the
> tags and mount virtiofs automatically in certain cases.
> 
> https://gitlab.com/virtio-fs/virtiofsd/-/issues/128
> 
> This patch exports tags at /sys/fs/virtiofs/<N>/tag where N is the id of
> the virtiofs device. The filesystem tag can be obtained by reading this
> "tag" file.
> 
> There is also a symlink at /sys/fs/virtiofs/<N>/device that points to
> the virtiofs device that exports this tag.
> 
> This patch converts the existing struct virtio_fs into a full kobject.
> It already had a refcount so it's an easy change. The virtio_fs objects
> can then be exposed in a kset at /sys/fs/virtiofs/. Note that virtio_fs
> objects may live slightly longer than we wish for them to be exposed to
> userspace, so kobject_del() is called explicitly when the underlying
> virtio_device is removed. The virtio_fs object is freed when all
> references are dropped (e.g. active mounts) but disappears as soon as
> the virtiofs device is gone.
> 
> Originally-by: Vivek Goyal <[email protected]>
> Signed-off-by: Stefan Hajnoczi <[email protected]>
> ---
>  fs/fuse/virtio_fs.c                         | 113 ++++++++++++++++----
>  Documentation/ABI/testing/sysfs-fs-virtiofs |  11 ++
>  2 files changed, 103 insertions(+), 21 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-fs-virtiofs
> 
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index de9a38efdf1e..28e96b7cde00 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -31,6 +31,9 @@
>  static DEFINE_MUTEX(virtio_fs_mutex);
>  static LIST_HEAD(virtio_fs_instances);
>  
> +/* The /sys/fs/virtio_fs/ kset */
> +static struct kset *virtio_fs_kset;
> +
>  enum {
>  	VQ_HIPRIO,
>  	VQ_REQUEST
> @@ -55,7 +58,7 @@ struct virtio_fs_vq {
>  
>  /* A virtio-fs device instance */
>  struct virtio_fs {
> -	struct kref refcount;
> +	struct kobject kobj;
>  	struct list_head list;    /* on virtio_fs_instances */
>  	char *tag;
>  	struct virtio_fs_vq *vqs;
> @@ -161,18 +164,43 @@ static inline void dec_in_flight_req(struct virtio_fs_vq *fsvq)
>  		complete(&fsvq->in_flight_zero);
>  }
>  
> -static void release_virtio_fs_obj(struct kref *ref)
> +static ssize_t virtio_fs_tag_attr_show(struct kobject *kobj,
> +		struct kobj_attribute *attr, char *buf)
>  {
> -	struct virtio_fs *vfs = container_of(ref, struct virtio_fs, refcount);
> +	struct virtio_fs *fs = container_of(kobj, struct virtio_fs, kobj);
> +
> +	return sysfs_emit(buf, fs->tag);
> +}
> +
> +static struct kobj_attribute virtio_fs_tag_attr = {
> +	.attr = { .name = "tag", .mode= 0644 },
> +	.show = virtio_fs_tag_attr_show,
> +};

__ATTR_RO()?
That way we all know you got the mode setting correct :)

Other than that minor thing, looks good, nice job!

greg k-h