Re: [RFC PATCH 2/7] 10-dm.rules: don't deactivate devices for DISK_RO=1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/4/24 11:48, Peter Rajnoha wrote:
> On 3/1/24 23:40, Martin Wilck wrote:
>> DISK_RO=1 is set in the environment of a block-device uevent if and only if
>> the device has just been set read-only by a driver by calling set_disk_ro().
>> It is not synoymous with the "ro" sysfs attribute; the device could very well
>> be write-protected if DISK_RO is not set. Probing should be possible even if
>> this flag is set, because blkid and friends usually don't write to the
>> device. Upper-layer subsystems that do need to write would need to check the
>> "ro" sysfs attribute rather than DISK_RO.
>>
>> Skip the DISK_RO check in 10-dm.rules.
>>
>> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
>> ---
>>  udev/10-dm.rules.in | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/udev/10-dm.rules.in b/udev/10-dm.rules.in
>> index 4ffd3e2..c08d829 100644
>> --- a/udev/10-dm.rules.in
>> +++ b/udev/10-dm.rules.in
>> @@ -50,9 +50,6 @@ ACTION!="add|change", GOTO="dm_end"
>>  # kernels >= 2.6.31 only. Cookie is not decoded for remove event.
>>  ENV{DM_COOKIE}=="?*", IMPORT{program}="(DM_EXEC)/dmsetup udevflags $env{DM_COOKIE}"
>>  
>> -# Rule out easy-to-detect inappropriate events first.
>> -ENV{DISK_RO}=="1", GOTO="dm_disable"
>> -
>>  # There is no cookie set nor any flags encoded in events not originating
>>  # in libdevmapper so we need to detect this and try to behave correctly.
>>  # For such spurious events, regenerate all flags from current udev database content
> 
> 
> Yes, I'd like to get rid of this rule, but, unfortunately, there's one
> issue during the DM device creation/activation.
> 
> For example, if I try:
> 
>   dmsetup create --readonly --table "0 1 zero"
> 
> Then I get these uevents:
> 
> 1)
> ACTION=add
> DM_UDEV_DISABLE_OTHER_RULES_FLAG=1
> DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG=1
> DM_UDEV_DISABLE_DISK_RULES_FLAG=1
> SYSTEMD_READY=0
> 
> 
> 2)
> ACTION=change
> DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG=1
> DM_UDEV_DISABLE_DISK_RULES_FLAG=1
> DM_UDEV_DISABLE_OTHER_RULES_FLAG=
> 
> 
> 3)
> ACTION=change
> DM_COOKIE=6335392
> DM_COOKIE_DISABLE_OTHER_RULES_FLAG=
> 
> 
> The uevent 3) coming with the DM_COOKIE is the actual event when the
> device is ready for use (that's the uevent notifying the DM device
> resume/activation).
> 
> If we remove the DISK_RO rule, then the DM_UDEV_DISABLE_OTHER_RULES_FLAG
> is unset for uevent 2), which in turn causes the SYSTEMD_READY=0 to get
> dropped, which in turn will start all the systemd hooks because the
> device is considered "ready" for systemd then.
> 
> But the DM dev is ready only after uevent 3) that comes with the
> DM_COOKIE. So we still need to cover this scenario.
> 

Hmm, this doesn't even work with original rules!

The 99-systemd.rules has:

SUBSYSTEM=="block", ACTION=="add",
ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}=="1", ENV{SYSTEMD_READY}="0"

which applies only to "add" uevent, not "change". This means that
SYSTEMD_READY="0" is even dropped for the DISK_RO="1" uevent even if it
is marked as DM_UDEV_DISABLE_OTHER_RULES_FLAG="1".

This is actually a bug. But a minor one, because here we always have the
change uevent with DM_COOKIE coming right after the DISK_RO change
uevent. So there's almost no window practically.

Now, I'm asking myself why we have the 'ACTION="add"' in the systemd
rule at all. Why is it not just checking  DM_UDEV_DISABLE_OTHER_RULES_FLAG?

-- 
Peter





[Index of Archives]     [Gluster Users]     [Kernel Development]     [Linux Clusters]     [Device Mapper]     [Security]     [Bugtraq]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]

  Powered by Linux