Created attachment 1954661 [details] udisk dump stratisd before and after update Description of problem: the latest update on fedora of stratisd 3.5.1-1.fc37 -> 3.5.2-1.fc37 broke cockpit-storaged tests. It looks like now udisks lists as block devices some stratis private objects. Version-Release number of selected component (if applicable): stratisd-3.5.2-1.fc37.x86_64 udisks2-2.9.4-5.fc37.x86_64 device-mapper-1.02.175-9.fc37.x86_64 lvm2-2.03.11-9.fc37.x86_64 How reproducible: Always Steps to Reproduce: 1. Create a stratis pool [root@fedora-37-127-0-0-2-2201 ~]# stratis pool create pool0 /dev/sda /dev/sdb 2. Run `udisksctl dump` [root@fedora-37-127-0-0-2-2201 ~]# udisksctl dump | grep dm /org/freedesktop/UDisks2/block_devices/dm_2d0: Device: /dev/dm-0 Id: by-id-dm-name-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-physical-originsub Symlinks: /dev/disk/by-id/dm-name-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-physical-originsub /dev/disk/by-id/dm-uuid-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-physical-originsub /org/freedesktop/UDisks2/block_devices/dm_2d1: Device: /dev/dm-1 Id: by-id-dm-name-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-flex-thinmeta Symlinks: /dev/disk/by-id/dm-name-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-flex-thinmeta /dev/disk/by-id/dm-uuid-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-flex-thinmeta /org/freedesktop/UDisks2/block_devices/dm_2d2: Device: /dev/dm-2 Id: by-id-dm-name-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-flex-thindata Symlinks: /dev/disk/by-id/dm-name-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-flex-thindata /dev/disk/by-id/dm-uuid-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-flex-thindata /org/freedesktop/UDisks2/block_devices/dm_2d3: Device: /dev/dm-3 Id: by-id-dm-name-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-flex-mdv Symlinks: /dev/disk/by-id/dm-name-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-flex-mdv /dev/disk/by-id/dm-uuid-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-flex-mdv /org/freedesktop/UDisks2/block_devices/dm_2d4: Device: /dev/dm-4 Id: by-id-dm-name-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-thinpool-pool Symlinks: /dev/disk/by-id/dm-name-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-thinpool-pool /dev/disk/by-id/dm-uuid-stratis-1-private-1b2fd552330e47fb979c3e52e724c2ec-thinpool-pool 3. Notice objects like the following now being listed. by-id-dm-name-stratis-1-private-*-physical-originsub by-id-dm-name-stratis-1-private-*-flex-thinmeta by-id-dm-name-stratis-1-private-*-flex-mdv by-id-dm-name-stratis-1-private-*-thinpool-pool I am not sure if that's expected, if this is expected I apologize for the noise. I attach the udiskctl dumpxml before and after the update for comparison.
Posting also the udevinfo for one of the problematic devices before and after the upgrade. Before the upgrade: [root@fedora-37-127-0-0-2-2201 ~]# udevadm info /dev/dm-0 P: /devices/virtual/block/dm-0 M: dm-0 R: 0 U: block T: disk D: b 253:0 N: dm-0 L: 0 S: mapper/stratis-1-private-bc0cc9cf1b2a4aec845a96eaa1592637-physical-originsub Q: 9 E: DEVPATH=/devices/virtual/block/dm-0 E: DEVNAME=/dev/dm-0 E: DEVTYPE=disk E: DISKSEQ=9 E: MAJOR=253 E: MINOR=0 E: SUBSYSTEM=block E: USEC_INITIALIZED=29644776 E: DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG=1 E: DM_UDEV_DISABLE_DISK_RULES_FLAG=1 E: DM_UDEV_DISABLE_OTHER_RULES_FLAG=1 E: DM_NAME=stratis-1-private-bc0cc9cf1b2a4aec845a96eaa1592637-physical-originsub E: DM_UUID=stratis-1-private-bc0cc9cf1b2a4aec845a96eaa1592637-physical-originsub E: DM_SUSPENDED=0 E: DM_UDEV_RULES_VSN=2 E: DEVLINKS=/dev/mapper/stratis-1-private-bc0cc9cf1b2a4aec845a96eaa1592637-physical-originsub E: TAGS=:systemd: E: CURRENT_TAGS=:systemd: After the upgrade: [root@fedora-37-127-0-0-2-2201 ~]# udevadm info /dev/dm-0 P: /devices/virtual/block/dm-0 M: dm-0 R: 0 U: block T: disk D: b 253:0 N: dm-0 L: 0 S: disk/by-id/dm-name-stratis-1-private-1859f5002007496581c6ca7560a7a936-physical-originsub S: disk/by-id/dm-uuid-stratis-1-private-1859f5002007496581c6ca7560a7a936-physical-originsub S: mapper/stratis-1-private-1859f5002007496581c6ca7560a7a936-physical-originsub Q: 9 E: DEVPATH=/devices/virtual/block/dm-0 E: DEVNAME=/dev/dm-0 E: DEVTYPE=disk E: DISKSEQ=9 E: MAJOR=253 E: MINOR=0 E: SUBSYSTEM=block E: USEC_INITIALIZED=30073205 E: DM_UDEV_PRIMARY_SOURCE_FLAG=1 E: DM_ACTIVATION=1 E: DM_NAME=stratis-1-private-1859f5002007496581c6ca7560a7a936-physical-originsub E: DM_UUID=stratis-1-private-1859f5002007496581c6ca7560a7a936-physical-originsub E: DM_SUSPENDED=0 E: DM_UDEV_RULES_VSN=2 E: DEVLINKS=/dev/disk/by-id/dm-name-stratis-1-private-1859f5002007496581c6ca7560a7a936-physical-originsub /dev/disk/by-id/dm-uuid-stratis-1-private-1859f5002007496581c6ca7560a7a936-physical-originsub /dev/mapper/stratis-1-private-1859f5002007496581c6ca7560a7a936-physical-originsub E: TAGS=:systemd: E: CURRENT_TAGS=:systemd:
And you can see that there is one major difference between the info before and after, namely before there are the following entries: E: DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG=1 E: DM_UDEV_DISABLE_DISK_RULES_FLAG=1 E: DM_UDEV_DISABLE_OTHER_RULES_FLAG=1 Which are missing after the upgrade.
DM_UDEV_DISABLE_OTHER_RULES_FLAG=1 causes UDisks2 to ignore a block device. To be more explicit about this, we should probably set UDISKS_IGNORE explicitly. This could maybe happen in 80-udisks2.rules, which is part of UDisks2 itself.
Upstream link: https://github.com/stratis-storage/stratisd/issues/3301
We expect that this will be resolved in the next release of stratisd, 3.5.3.
We believe that stratisd's crypt DM devices have never been made private via udev properties and have therefore always been visible via udiskctl. These devices are not constructed, of course, unless a pool is encrypted. Does Cockpit construct encrypted pools? Do you have any stake in whether or not the crypt devices also have these udev properties set? Let us know, thanks.
(In reply to mulhern from comment #6) > We believe that stratisd's crypt DM devices have never been made private via > udev properties and have therefore always been visible via udiskctl. Yes, that seems to be the case. > These devices are not constructed, of course, unless a pool is encrypted. > > Does Cockpit construct encrypted pools? Yes. > Do you have any stake in whether or > not the crypt devices also have these udev properties set? Cockpit did not show these block devices because it generally doesn't show the "crypt" devices for encrypted things. It treats it as a implementation detail, if you will. So while the "crypt" devices created by Stratis are not explicitly ignored anywhere because they would be internal to Stratis, they happen to be ignored in practice because of how Cockpit treats "crypt" device mapper devices. It's a bit of a mess, no? :-) I think the cleanest thing would be for the UDISKS_IGNORE property to be set explicitly on internal Stratis device mapper devices. This could be done by Stratis itself, or by UDisks2. The property is documented in "man 8 udisks", https://manpages.debian.org/testing/udisks2/udisks.8.en.html Relying on properties like DM_UDEV_DISABLE_OTHER_RULES_FLAG to have any well defined meaning outside of the device mapper rule labyrinth is probably unwise. I am not aware of any documentation of DM_UDEV_DISABLE_OTHER_RULES_FLAG as public API. In fact, setting it yourself might have unintended consequences when the device mapper udev rules find them, no? I don't know what those attributes are for, tbh.
Thanks for the further information. It is a bit of a mess, certainly, and always has been. At present, our fix just restores previous behavior. Since the UDISKS_IGNORE property is udisk-specific, and, as far as we know you are the only consumer of udisks D-Bus information for Stratis, your suggestion seems reasonable, modulo implementation details. We'll look into the suggestion. However, once we have restored previous behavior and resolved the immediate problem in this bz, we'll want to close it, for tidiness' sake.
> I am not aware of any documentation of DM_UDEV_DISABLE_OTHER_RULES_FLAG as public API. In fact, setting it yourself might have unintended consequences when the device mapper udev rules find them, no? I don't know what those attributes are for, tbh. These flags are documented in the public device-mapper API defined in libdevmapper.h: /* * DM_UDEV_DISABLE_SUBSYTEM_RULES_FLAG is set in case we need to disable * subsystem udev rules, but still we need the general DM udev rules to * be applied (to create the nodes and symlinks under /dev and /dev/disk). */ #define DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG 0x0002 /* * DM_UDEV_DISABLE_DISK_RULES_FLAG is set in case we need to disable * general DM rules that set symlinks in /dev/disk directory. */ #define DM_UDEV_DISABLE_DISK_RULES_FLAG 0x0004 /* * DM_UDEV_DISABLE_OTHER_RULES_FLAG is set in case we need to disable * all the other rules that are not general device-mapper nor subsystem * related (the rules belong to other software or packages). All foreign * rules should check this flag directly and they should ignore further * rule processing for such event. */ #define DM_UDEV_DISABLE_OTHER_RULES_FLAG 0x0008 They have not changed since 2009 when udev synchronization was added to libdevmapper (although it would be nice if this was collected together in a manual page somewhere...) - we are using them here for their defined purpose so I don't think there's any problem depending on this behaviour. Although setting UDISKS_IGNORE sounds straightforward it's actually quite complicated for DM devices - everything affecting udev has to be encoded into the "cookie" that is passed to the kernel with the ioctl call, and then decoded and acted upon by udev rules (either the standard set of DM rules or additional subsystem rules provided by Stratis). In practice this would mean using one of the 8 reserved DM_SUBSYSTEM_UDEV_FLAGX values to mean "set UDISKS_IGNORE" for Stratis devices and then adding new Stratis udev rules to decode the flag and set the property. This would be specific to Stratis and would need to be implemented by our udev rules - we'd also probably still want to set the same DM_UDEV_DISABLE_* flags to prevent any interference with/from other subsystems and udev rules. On the other hand the same set of three flags (DISKS, SUBSYSTEM, OTHER_RULES) are used both by lvm2 for "internal"/hidden LVs, and by cryptsetup for the temporary devices that it creates so I think we are consistent with the wider DM world here.
(In reply to Bryn M. Reeves from comment #9) > > I am not aware of any documentation of DM_UDEV_DISABLE_OTHER_RULES_FLAG as public API. In fact, setting it yourself might have unintended consequences when the device mapper udev rules find them, no? I don't know what those attributes are for, tbh. > > These flags are documented in the public device-mapper API defined in > libdevmapper.h: I see, thanks! > /* > * DM_UDEV_DISABLE_OTHER_RULES_FLAG is set in case we need to disable > * all the other rules that are not general device-mapper nor subsystem > * related (the rules belong to other software or packages). All foreign > * rules should check this flag directly and they should ignore further > * rule processing for such event. > */ > #define DM_UDEV_DISABLE_OTHER_RULES_FLAG 0x0008 > > They have not changed since 2009 when udev synchronization was added to > libdevmapper (although it would be nice if this was collected together in a > manual page somewhere...) - we are using them here for their defined purpose > so I don't think there's any problem depending on this behaviour. Yes, that makes sense. > Although setting UDISKS_IGNORE sounds straightforward it's actually quite > complicated for DM devices - everything affecting udev has to be encoded > into the "cookie" that is passed to the kernel with the ioctl call, and then > decoded and acted upon by udev rules (either the standard set of DM rules or > additional subsystem rules provided by Stratis). I was thinking of some udev rule that matches on whatever is necessary, such as DM_NAME like 61-stratisd.rules does for creating the symlinks in /dev/stratis/. > On the other hand the same set of three flags (DISKS, SUBSYSTEM, > OTHER_RULES) are used both by lvm2 for "internal"/hidden LVs, and by > cryptsetup for the temporary devices that it creates so I think we are > consistent with the wider DM world here. Yes, and it all worked as intended earlier, before those flags were dropped for internal stratis block devices. So, yeah, if you say that the DM_UDEV_DISABLE flags should be set in any case for other reasons, then also setting UDISKS_IGNORE is not necessary. But if you would only set those flags to hide the block devices in UDisks2, then maybe using UDISKS_IGNORE instead would be cleaner, imo. Fyi, here is the comment from UDisks2 that explains why it ignores uevents with the OTHER_RULES flag: /* Ignore the uevent if the device-mapper layer requests * that other rules ignore this uevent * * It's somewhat nasty to do this but it avoids all kinds of * race-conditions caused by the design of device-mapper * (such as temporary-cryptsetup nodes and cleartext devices * without ID_FS properties properly set). */ The impression I got from that was that it's essentially a hack. But after more than a decade in the code, I guess this hack is now the Right Thing. :-)
Update is available at: https://bodhi.fedoraproject.org/updates/FEDORA-2023-63b22d16f3
Would you consider this fixed?
@kkoukiou I'm going to close this. Please reopen if it shouldn't be closed. Thanks.
The needinfo request[s] on this closed bug have been removed as they have been unresolved for 120 days