99-systemd.rules: rework SYSTEMD_READY logic for device mapper (v254)

[mwilck]

Device mapper devices are set up in multiple steps. The first step, which
generates the initial "add" event, only creates an empty container, which is
useless for higher layers. SYSTEMD_READY should be set to 0 on this event to
avoid premature device activation.

The event that matters is the "activation" event: the first "change" event on
which DM_UDEV_DISABLE_OTHER_RULES_FLAG=1 is not set. When this event arrives,
the device is ready for being scanned by blkid and similar tools, and for being
activated by systemd.

Intermittent events with DM_UDEV_DISABLE_OTHER_RULES_FLAG=1 should be ignored
as far as systemd or higher-level block layers are concerned. Previous device
properties and symlinks should be preserved: the device shouldn't be scanned or
activated, but shouldn't be deactivated, either. In particular, SYSTEM_READY
shouldn't be set to 0 if it wasn't set before, because that might cause mounted
file systems to be unmounted. Such intermittent events may occur any time,
before or after the "activation" event.

DM_UDEV_DISABLE_OTHER_RULES_FLAG=1 can have multiple reasons. One possible reason
is that the device is suspended. There are other reasons that depend on the
device-mapper subsystem (LVM, multipath, dm-crypt, etc.).

The current systemd rule set

1. sets SYSTEMD_READY=0 if DM_UDEV_DISABLE_OTHER_RULES_FLAG is set in "add"
   events;
2. imports SYSTEMD_READY from the udev db if DM_SUSPENDED is set, and jumps to systemd_end;
3. sets SYSTEMD_READY=1, otherwise.

This logic has several flaws:

• 1. can cause file systems to be unmounted if an coldplug event arrives while
     a file system is suspended. This rule shouldn't be applied for coldplug events
     or in general, "synthetic" add events;
• 2. evaluates DM_SUSPENDED=1, which is a device-mapper internal property.
     It's wrong to infer that a device is accessible if DM_SUSPENDED=0.
     The jump to systemd_end may cause properties and/or symlinks to be lost;
• 3. is superfluous, because SYSTEMD_READY=1 is equivalent with SYSTEMD_READY
     being unset, and can create the wrong impression that the device was explicitly
     activated.

This patch fixes the logic as follows:

• apply 1) only if DM_NAME is empty, which is only the case for the first
  "genuine add" event;
• change 2) to use DM_UDEV_DISABLE_OTHER_RULES_FLAG instead of DM_SUSPENDED,
  and remove the GOTO directive;
• remove 3).

Fixes: b7cf1b6 ("udev: use SYSTEMD_READY to mask uninitialized DM devices")
Fixes: 35a6750 ("rules: set SYSTEMD_READY=0 on DM_UDEV_DISABLE_OTHER_RULES_FLAG=1 only with ADD event (#2747)")

Signed-off-by: Martin Wilck [email protected]
(cherry picked from commit c072860)

[mwilck]

Backport of systemd/systemd#31661.

There has been some confusion about the dependency between this change in the systemd udev rules and the related change in LVM2 2.03.24.

The systemd/udev change (this PR) is a fix for the logic with which systemd handles SYSTEMD_READY for dm devices. It is independent of the changes in the device-mapper udev rules in LVM 2.03.24, and can be applied even if the latter are not applied.

OTOH, updating to LVM 2.03.24 without applying the changes from this PR comes with a slight regression risk as explained in this comment, because the updated device-mapper rules don't set DM_SUSPENDED any more.

Therefore it's better and safer for everyone to backport this PR to the systemd-stable branches.

For information: @prajnoha, @bluca, @yuwata, @fbui, @bmarzins, @msekletar

[packit-as-a-service]

We were not able to find or create Copr project packit/systemd-systemd-stable-411 specified in the config with the following error:

Cannot create a new Copr project (owner=packit project=systemd-systemd-stable-411 chroots=[]): chroots: '[]' is not a valid choice for this field..

---

Unless the HTTP status code above is >= 500, please check your configuration for:

1. typos in owner and project name (groups need to be prefixed with @)
2. whether the project name doesn't contain not allowed characters (only letters, digits, underscores, dashes and dots must be used)
3. whether the project itself exists (Packit creates projects only in its own namespace)
4. whether Packit is allowed to build in your Copr project
5. whether your Copr project/group is not private

[yuwata]

rebased.

[bluca]

Why is it necessary to push udev changes this far back? It's very easy to change behaviour and introduce regressions, it already happened

[yuwata]

To resolve issue when combined with newer LVM, as explained #411 (comment) .

We do not have any regression reports for the change in upstream, and we have several basic test cases for udev with LVM. Hopefully fine.

[bluca]

The newer lvm2 is only in fedora rawhide, suse tumbleweed, debian testing and ubuntu devel, none of which should be using these older branches, so let's not introduce risks when there's no use case, it went to 256-stable and 255-stable and that should be enough, until an actual use case appears