On 3/4/24 16:17, Martin Wilck wrote:
> 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@xxxxxxxx>
>>> ---
>>> 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.
>
I agree.
The only issue I can identify here is that someone may fire its rule
based on the ID_FS_TYPE existence, but that would also mean they are
ignoring the DM_UDEV_DISABLE_OTHER_RULES_FLAG, which is much bigger
issue. I was just curious whether it makes sense in that case to import
it if nobody is reading it, but it's true some rule can still compare
previous and current state of ID_FS_TYPE and then act on a state change.
So yes, better to have that in.
--
Peter
