-
Notifications
You must be signed in to change notification settings - Fork 1.3k
libblkid and libmount: add support for ID_FS_MOUNTTYPE= #3872
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
base: master
Are you sure you want to change the base?
Conversation
|
#3867 may solve the failures on fedora-rawhide-*. |
1981b14 to
97ec244
Compare
| return NULL; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Involving libblkid in this looks like a layering validation.
The filesystem is ntfs, irrespective of how it will be mounted in the end and that is what libblkid should return.
We can have the mapping/udev logic in libmount alone.
With some simple bespoke logic instead of tying it into struct blkid_idinfo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want ID_FS_MOUNTYPE= in udev db together with classic ID_FS_TYPE=. The information should be globally available, not just for libmount only. You can mount without libmount.
blkid_fstype_to_mounttype() is just a hack for situations where you have a fs-type, but you have no mount-type. For example, if you have a new libblkid, but an old udev, etc. The final goal is not to use this function by default during the mount process.
Unfortunately, we have |
Currently, there is only NAME= (also known as filesystem type). This is used by libmount to specify the filesystem type during mounting. Unfortunately, this traditional solution does not work as expected in all cases. For example, NTFS can be mounted by multiple filesystem kernel drivers, and the driver's behavior depends on the filesystem name, making kernel-level aliases potentially useless for some users. It would be possible to simply change the FS type from ntfs to ntfs3, but this introduces backwardly incompatible behavior for tools like udisks, where "ntfs" is hardcoded to provide specific filesystem behavior. After discussion, the best solution seems to be separating the filesystem name and filesystem mount type (driver). Addresses: util-linux#3618 Signed-off-by: Karel Zak <kzak@redhat.com>
Autotools: ./configure --with-ntfs-mounttype=TYPE Meson: meson setup build -D ntfs-mounttype=TYPE The current default is "ntfs3". The new build system options allow you to override it. Signed-off-by: Karel Zak <kzak@redhat.com>
Signed-off-by: Karel Zak <kzak@redhat.com>
Use MOUNTTYPE= from blkid as the filesystem type; if not defined, use the classic TYPE=. Addresses: util-linux#3618 Signed-off-by: Karel Zak <kzak@redhat.com>
Move the current code into two small functions to improve readability and facilitate future extensions. Signed-off-by: Karel Zak <kzak@redhat.com>
Split the function into smaller functions to make it easier to read and extend in the future. Signed-off-by: Karel Zak <kzak@redhat.com>
Signed-off-by: Karel Zak <kzak@redhat.com>
97ec244 to
1ed10ef
Compare
|
The funny thing is that https://lore.kernel.org/lkml/CAKYAXd98388-=qOwa++aNuggKrJbOf24BMQZrvm6Gnjp_7qOTQ@mail.gmail.com/ Therefore, once Even so, using the udev database is useful to speed up mount time, and it provides future-proofing should other cases arise where multiple drivers exist for the same filesystem signature. |
tbzatek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either way I like this proposal, thank you for listening to our needs! True that once the ntfsplus situation settles down, there won't be need for this PR (once the new util-linux hits the distros, ntfs3 may be gone).
|
Nice. The question is whether we really need ID_FS_MOUNTTYPE= in the udev database and libblkid. Perhaps it would be sufficient to have a compile-time option for libmount to overwrite "ntfs" with something else (and suggest --with-ntfs-mounttype=ntfs3 for now). Note that without ID_FS_MOUNTTYPE=, we do not need to modify udev and systemd. |
UDisks can still use this value as an indication of a preferred filesystem driver to use, if available, as a first authoritative source before its own definition. Besides, there's always the option to override the value through custom udev rules system-wide, so I see it as a good benefit for everyone to have this. |
The library traditionally uses libblkid to obtain device properties (such as FS-type if not specified). This can be a relatively costly operation to scan the device and requires read access to the device. All relevant libblkid information is usually cached by the udev DB. This commit adds the possibility to reuse the information from udev, with a fallback to libblkid if udev is not available. This change improves the speed of the mount process and makes it possible to modify elements used for filesystem mounting in udev rules. For example, it will be possible to modify ID_FS_MOUNTTYPE=. The commit also adds $ ./configure --disable-libmount-udev-support $ meson setup build -Dbuild-libmount-udev-support=disabled to completely disable this feature and avoid libmount's dependence on libsystemd. Signed-off-by: Karel Zak <kzak@redhat.com>
1ed10ef to
d1ba5fb
Compare
|
systemd/udev part: systemd/systemd#39982 |
| AS_IF([test "x$enable_libmount_udev_support" != xno && test "x$build_libmount" = xyes], [ | ||
| AS_CASE([$enable_libmount_udev_support:$have_systemd], | ||
| [yes:no], | ||
| [AC_MSG_ERROR([libmount udev support expected but libsystemd not found])], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dependency on a particular init system is unfortunate as it limits reach of this feature. I see it's only used for querying udev properties on a particular device, can this be ported to udev_device_get_property_value() instead? Funny that udev_device_get_property_value() uses sd_device_get_property_value() internally, the difference is that libudev.so apparently links those statically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libudev is deprecated and should not be used for new projects; is there any other provider of the udev library other than the systemd project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, not all Linux distributions do provide libsystemd and udev is often being built from the systemd tree as a standalone package. An alternative implementation (a fork actually) of libsystemd is provided by the elogind project. The world still uses libudev as there's no simple alternative with reasonably little dependencies (i.e. not something that pulls in a different init system).
Based on the discussion from #3618
This PR introduces mount-type (also known as ID_FS_MOUNTTYPE= in udev naming conventions) to libblkid and libmount. This new filesystem property is an optional extension to the classic fs-type. This separation allows the use of
stable FS-types (names) and variable mount-types (kernel fs driver).
The advantage is that tools like UDisks can continue to use FS-type specific features without changes, regardless of how the FS is mounted.
This PR also introduces the ability for libmount to read cached libblkid data from the udev DB, rather than directly scanning devices with libblkid. This improves performance and allows modification of FS properties (e.g. ID_FS_MOUNTTYPE=) via udev rules without modifying libblkid, specifically per-device. This feature can be disabled with
--disable-libmount-udev-support(autotools) or-Dbuild-libmount-udev-support=disabled(meson).The mount-type is enabled for ntfs, with the default mount-type set to ntfs3. The default can be modified
--with-ntfs-mounttype=TYPE(autotools) or -Dntfs-mounttype=TYPE(meson).Note that systemd udev-builtin-blkid needs to be updated to export ID_FS_MOUNTTYPE to udev rules and the udev database. It will also be necessary to update
systemd/src/mount/mount-tool.cto teach systemd to use the mount-typetool. I'll prepare a patch for this.