On 3/4/24 17:09, Martin Wilck wrote: > On Mon, 2024-03-04 at 11:48 +0100, Peter Rajnoha wrote: >> >> 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. > > As event 2) doesn't have DM_COOKIE, I don't think we need to bother > about it much. IMO we should treat it like a synthetic "change" event, > which has almost no effect as far as dm is concerned. Well, partly yes, but the important thing here is that the other rules don not consider this as something they should react on. Here, it's like the (genuine) "add" event for a DM device that has almost zero value to others (we still need the table to get loaded and the device resumed for it to be usable). That's why the DISK_RO uevent was marked with DUDORF flag in the original 10-dm.rules as anything before the activation mark is simply considered spurious. > Event 3) doesn't have DISK_RO=1 set. If any later rules are interested > in the state of write protection, they need to check the "ro" sysfs > attribute instead. The unfortunate thing about the DISK_RO, unlike other events, is that it is triggered *before* that DM activation (DM activation == table load + resume). Also, frankly, I don't like this event at all - it's just notifying about one of the many device attributes. A question then arises: "Why don't we have uevents for changes in other attributes?", but that's how it is for now and it's probably for a separate debate. Till now, we've been marking that DISK_RO uevent as spurious completely. But yes, we should have probably done that only in case it happens before the activation, not if the device is already activated, if that's the case too. > > It would make some sense to be able tell later rules that they don't > need to bother with a given uevent because (from device mapper PoV) > nothing relevant has changed. I am not sure if we should use > DM_UDEV_DISABLE_OTHER_RULES_FLAG=1 for this purpose. "Don't bother, > nothing has changed" is not the same thing as "don't bother, you can't > access this device right now", which to my understanding is the meaning > of DM_UDEV_DISABLE_OTHER_RULES_FLAG=1. Actually, depending on the perspective, the DISK_RO uevent happening before the activation is that kind of "you can't access this device right now" (because it has not been activated fully yet). > > Actually we have DM_ACTIVATION=1 to tell other rules that they do need > to take action. Later rules only need to rescan a DM device if > DM_ACTIVATION=1; in all other cases they could just import properties > from the db. Currently kpartx and lvm are the only rules that check > DM_ACTIVATION. Yes, indeed, good point, the DM_ACTIVATION=1 is a little helper so the rules which need to react *only on the event where the DM device has just been activated/reactivated* with a new table. It is set in two cases, IIRC: - on genuine "change" event where a new DM table is activated (a resume after table load) - on synthetic "add" event *after* the device has already been activated before (to cover the coldplug/udevadm trigger --action=add) If there's a case where other rules do care about reacting only on this particular event, then they should check DM_ACTIVATION, yes. But is there such a case for other (non-dm, non-dm-subysystem) rules? For the DISK_RO, it's about whether it comes before or after the activation, not whether the DISK_RO is set with the activation at the same time (which is not, it's a separate event). For this case, we don't have anything to check right now - we just simply "disable" all the events coming before the activation. -- Peter
