Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose DM udev visibility controls to clients #846

Open
bmr-cymru opened this issue Apr 3, 2023 · 1 comment
Open

Expose DM udev visibility controls to clients #846

bmr-cymru opened this issue Apr 3, 2023 · 1 comment
Assignees

Comments

@bmr-cymru
Copy link
Member

The device-mapper udev rules define several environment variables that can be used to control udev rule processing and device visibility for DM devices. These are exposed in DmUdevFlags and are used internally to control udev processing of devices managed with devicemapper-rs:

    pub struct DmUdevFlags: u32 {
        /// Disables basic device-mapper udev rules that create symlinks in /dev/<DM_DIR>
        /// directory.
        const DM_UDEV_DISABLE_DM_RULES_FLAG = dmi::DM_UDEV_DISABLE_DM_RULES_FLAG;
        /// Disable subsystem udev rules, but allow general DM udev rules to run.
        const DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG = dmi::DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG;
        /// Disable dm udev rules which create symlinks in /dev/disk/* directory.
        const DM_UDEV_DISABLE_DISK_RULES_FLAG = dmi::DM_UDEV_DISABLE_DISK_RULES_FLAG;
        /// Disable all rules that are not general dm nor subsystem related.
        const DM_UDEV_DISABLE_OTHER_RULES_FLAG = dmi::DM_UDEV_DISABLE_OTHER_RULES_FLAG;
        /// Instruct udev rules to give lower priority to the device.
        const DM_UDEV_LOW_PRIORITY_FLAG = dmi::DM_UDEV_LOW_PRIORITY_FLAG;
        /// Disable libdevmapper's node management.
        const DM_UDEV_DISABLE_LIBRARY_FALLBACK = dmi::DM_UDEV_DISABLE_LIBRARY_FALLBACK;
        /// Automatically appended to all IOCTL calls issues by libdevmapper for generating
        /// udev uevents.
        const DM_UDEV_PRIMARY_SOURCE_FLAG = dmi::DM_UDEV_PRIMARY_SOURCE_FLAG;
    }

Historically ThinDev devices are the only devices for which DM_UDEV_PRIMARY_SOURCE_FLAG is set. This is a design decision that supports Stratis' needs: only the leaf thin devices are exposed. Other device types (CacheDev, LinearDev, and ThinPoolDev) did not set this flag and hence were "hidden" devices.

The changes to add Udev synchronization inadvertently changed the default for all device types, since DM_UDEV_PRIMARY_SOURCE_FLAG is now set automatically for ioctl operations that generate uevents. This is a user visible change in behaviour and causes problems for other subsystems, for e.g. Udisks:

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.

The immediate issue is addressed in #845 by explicitly setting three DmUdevFlags values for these device types:

        DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG
        DM_UDEV_DISABLE_DISK_RULES_FLAG
        DM_UDEV_DISABLE_OTHER_RULES_FLAG

This restores the historical behaviour of devicemapper-rs 0.32.3 and earlier as expected by Stratis.

For a general purpose library this behaviour should be optional and under the control of users of the crate. This requires breaking changes to the library API to allow callers to specify create_options that govern the visibility of the objects being managed as discussed in stratis-storage/project#389

@bmr-cymru bmr-cymru self-assigned this Apr 3, 2023
@bmr-cymru
Copy link
Member Author

This is relatively straightforward: adding the create_options as an Option to the existing new() and setup() methods for each device type gives users of the crate four options to control visibility and other properties of the created devices:

       create_options          devicemapper-rs device behavior
+----------------------------+------------------------------------------------+
| None                       | Accept historical library defaults.            |
+----------------------------+------------------------------------------------+
| Some(DmOptions::default()) | Library defaults for visible devices.          |
+----------------------------+------------------------------------------------+
| Some(DmOptions::private()) | Library defaults for private (hidden) devices. |
+----------------------------+------------------------------------------------+
| Some(custom_options)       | Caller has complete control of options/flags.  |
+----------------------------+------------------------------------------------+

The changes in Stratis to make use of this are also smaller than I'd anticipated:

$ git diff --stat src/
 src/engine/strat_engine/backstore/backstore.rs | 14 ++++++++++++--
 src/engine/strat_engine/thinpool/filesystem.rs | 13 +++++++++++--
 src/engine/strat_engine/thinpool/thinpool.rs   |  9 +++++++++
 3 files changed, 32 insertions(+), 4 deletions(-)

I.e. if users pass None for the create_options then the behaviour is consistent with devicemapper-rs versions before 0.32.3. We should probably be explicit about this in Stratis and use DmOptions::default() and DmOptions::private() for visible and hidden devices respectively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant