Re: [PATCH 2/2] btrfs-progs: add udev rule to forget removed device

[Boris Burkov <boris@bur.io>]

On Thu, Feb 29, 2024 at 08:53:39PM +0100, David Sterba wrote:
> On Thu, Feb 29, 2024 at 10:36:55AM -0800, Boris Burkov wrote:
> > Now that btrfs supports forgetting devices that don't exist, we can add
> > a udev rule to take advantage of that. This avoids bad edge cases
> > with cached devices in multi-device filesystems without having to rescan
> > all the devices on every change.
> > 
> > Signed-of-by: Boris Burkov <boris@bur.io>
> > ---
> >  64-btrfs-rm.rules | 7 +++++++
> >  Makefile          | 2 +-
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> >  create mode 100644 64-btrfs-rm.rules
> > 
> > diff --git a/64-btrfs-rm.rules b/64-btrfs-rm.rules
> > new file mode 100644
> > index 000000000..852155d28
> > --- /dev/null
> > +++ b/64-btrfs-rm.rules
> > @@ -0,0 +1,7 @@
> 
> Please add a comment that explains when and why this udev rule should be
> used.
> 

Definitely happy to add a comment.

This is certainly the discussion I was hoping to have, as well, but I
thiiink we just always want this? Basically if we don't have it,
multi-device users are in danger of accidentally making a stale device
cache between mounts. It's probably not that big of a risk in general,
but we did hit an easier to hit variant in v5.19 at Meta.

OTOH, there is also the problem that this is a no-op unless the kernel
has the patch I sent at the same time:
btrfs: support device name lookup in forget

I don't think there is any downside to running this command which will
simply fail on an older kernel.

If this becomes ubiquitous, then we can also remove the special case for
single device cache clearing from the btrfs unmount code.

> > +SUBSYSTEM!="block", GOTO="btrfs_rm_end"
> > +ACTION!="remove", GOTO="btrfs_rm_end"
> > +ENV{ID_FS_TYPE}!="btrfs", GOTO="btrfs_rm_end"
> > +
> > +RUN+="/usr/local/bin/btrfs device scan -u $devnode"
> 
> Is the full path mandatory or is 'btrfs' sufficient? I think systemd
> uses own tool of the same name.

Unfortunately, it did not work for me. I saw logs saying
/usr/lib/udev/rules.d/btrfs file not found or something like that in
dmesg.

I also considered the btrfs udev "builtin" but from experimenting and
checking out the code, it looks like that only does device ready, not
all device commands.

> 
> Please use long option name so it's more obvious what it does.