Re: [RFC PATCH 1/7] 13-dm-disk.rules: import ID_FS_TYPE

[Martin Wilck <mwilck@suse.com>]

On Mon, 2024-03-04 at 11:37 +0100, Peter Rajnoha wrote:
> On 3/1/24 23:40, Martin Wilck wrote:
> > ID_FS_TYPE is the most important udev property for most follow-up
> > rules. It must be imported from the udev db if blkid can't be run.
> > 
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> >  udev/13-dm-disk.rules.in | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/udev/13-dm-disk.rules.in b/udev/13-dm-disk.rules.in
> > index dca00bc..eaad972 100644
> > --- a/udev/13-dm-disk.rules.in
> > +++ b/udev/13-dm-disk.rules.in
> > @@ -26,6 +26,7 @@ ENV{DM_NOSCAN}=="1", GOTO="dm_watch"
> >  GOTO="dm_link"
> >  
> >  LABEL="dm_import"
> > +IMPORT{db}="ID_FS_TYPE"
> >  IMPORT{db}="ID_FS_USAGE"
> >  IMPORT{db}="ID_FS_UUID_ENC"
> >  IMPORT{db}="ID_FS_LABEL_ENC"
> 
> I think this one was left out deliberately. The original intention
> was
> to import only the minimal set of "blkid" variables needed to
> properly
> keep the symlinks based on these values in place (patch 94f77a4). And
> the "ID_FS_TYPE" is not actually needed for this.

Why would we want to maintain the symlinks, but not other properties?
ID_FS_TYPE is the blkid property with the highest number of consumers
in a standard rule set. It is interpreted by most rule sets which build
additional layers on top of dm devices.

> Though, importing even the "ID_FS_TYPE" together with other blkid
> variables should not be an issue because this "import from previous
> state" happens only if we have DM_SUSPENDED or DM_NOSCAN set for
> current
> event, in which case any other rules should be also disabled with
> DM_UDEV_DISABLE_OTHER_RULES_FLAG="1".
> 
> So even if some other rule fires based on ID_FS_TYPE value, the
> DM_UDEV_DISABLE_OTHER_RULES_FLAG should stop it anyway.

Exactly.

> Then, the question is whether we really need to
> 'IMPORT{db}="ID_FS_TYPE'?

I believe the question to ask rather "why should we not?". What harm do
you think could be done by importing ID_FS_TYPE from the db?

If we don't import ID_FS_TYPE, a later rule set could justly assume
that the fstype had changed, and in the worst case, might even take
some destructive action in response. AFAICS no rule set currently does,
but there's no guarantee for that. We should provide the follow-up
rules as much information as we can about the device, and if a device
is suspended, or the NOSCAN flag is set, there is no reason to assume
that the device has changed it fstype, or is about to change it.

The multipath rules have imported ID_FS_TYPE since 2014, and we've seen
no issues with it.

Regards
Martin