Re: [PATCH] virtiofs: Export filesystem tags through sysfs

[Vivek Goyal <vgoyal@redhat.com>]

On Sat, Oct 21, 2023 at 04:10:21PM +0000, Alyssa Ross wrote:
> On Thu, Oct 05, 2023 at 04:30:30PM -0400, Vivek Goyal wrote:
> > virtiofs filesystem is mounted using a "tag" which is exported by the
> > virtiofs device. virtiofs driver knows about all the available tags but
> > these are not exported to user space.
> >
> > People have asked 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
> 
> Hi, I was one of those people. :)

Hi Alyssa,

Aha... Thanks. I was not able to remember who had requested this.

> 
> > This patch exports tags through sysfs. One tag is associated with each
> > virtiofs device. A new "tag" file appears under virtiofs device dir.
> > Actual filesystem tag can be obtained by reading this "tag" file.
> >
> > For example, if a virtiofs device exports tag "myfs", a new file "tag"
> > will show up here.
> >
> > /sys/bus/virtio/devices/virtio<N>/tag
> >
> > # cat /sys/bus/virtio/devices/virtio<N>/tag
> > myfs
> >
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> 
> Are you still thinking about exposing this in the uevent as well?
> That would be much more convenient for me, because with this approach
> by the time the "remove" uevent arrives, it's no longer possible to
> check what tag was associated with the device — you have to store it
> somewhere when the device appears, so you can look it up again when the
> device is removed.  (Not everybody uses udev.)

Looks like systemd + udev combination can already take care of it. I just
had to specify "StopWhenUnneeded=true" in my systemd .mount unit file. And
that made sure that when device goes away, virtiofs is unmounted and
service is deactivated.

Following is my mount unit file.

$ cat /etc/systemd/system/mnt-virtiofs.mount

[Unit]
Description=Virtiofs mount myfs
DefaultDependencies=no
ConditionPathExists=/mnt/virtiofs
ConditionCapability=CAP_SYS_ADMIN
Before=sysinit.target
StopWhenUnneeded=true 

[Mount]
What=myfs
Where=/mnt/virtiofs
Type=virtiofs

And following is the udev rule I used.

$ cat /etc/udev/rules.d/80-local.rules
SUBSYSTEM=="virtio", DRIVER=="virtiofs", ATTR{tag}=="myfs", TAG+="systemd", ENV{SYSTEMD_WANTS}+="mnt-virtiofs.mount"

And a combination of above two seems to work. virtiofs is automatically
mounted when device is hotplugged and it is unmounted when device is
hot unplugged.

> 
> Regardless,
> 
> Tested-by: Alyssa Ross <hi@alyssa.is>
> 
> … and a review comment below.
> 
> > ---
> >  fs/fuse/virtio_fs.c | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >
> > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> > index 5f1be1da92ce..a5b11e18f331 100644
> > --- a/fs/fuse/virtio_fs.c
> > +++ b/fs/fuse/virtio_fs.c
> > @@ -107,6 +107,21 @@ static const struct fs_parameter_spec virtio_fs_parameters[] = {
> >  	{}
> >  };
> >
> > +/* Forward Declarations */
> > +static void virtio_fs_stop_all_queues(struct virtio_fs *fs);
> > +
> > +/* sysfs related */
> > +static ssize_t tag_show(struct device *dev, struct device_attribute *attr,
> > +			char *buf)
> > +{
> > +	struct virtio_device *vdev = container_of(dev, struct virtio_device,
> > +						  dev);
> > +	struct virtio_fs *fs = vdev->priv;
> > +
> > +	return sysfs_emit(buf, "%s", fs->tag);
> 
> All of the other files in the device directory end with trailing
> newlines.  Should this one be an exception?

I did not think much about it but you are right that other files seem
to have a newline at the end. I will add one as well.

Thanks
Vivek