Skip to content

src/slot: fix mount point detection for symlinks #1722

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

src/slot: fix mount point detection for symlinks #1722

wants to merge 1 commit into from

Conversation

amuetzel
Copy link

@amuetzel amuetzel commented May 28, 2025 •
edited
Loading

In multiple use cases, it is desirable to use a symlink to a device in the device=/dev/<device> attribute of a slot in system.conf instead of the actual path of the device:
One might want to identify the slots using attributes such as the disk id or path by using udev-provided symlinks such as /dev/disk/by-id/... or /dev/disk/by-path.
Or, on devices that support booting from multiple media, dynamically create a symlink such as
/dev/disk/rootfs-0 that points to the appropriate partition on the currently-active boot device.

This currently makes the mount point detection fail because RAUC cannot identify that such a device is actually mounted to a slot. (rauc status does not identify such a slot as mounted: /<mountpoint>.)

The reason seems to be the way that update_external_mount_points() matches mount points with slots:

To get the currently-mounted devices, g_unix_mounts_get() is used and returns the real paths of all mounted devices.
Then, find_config_slot_by_device() is used and calls r_slot_find_by_device() to match active mounts with config entries.

This iterates over all slots and checks whether the slot->device is equal to the given device name. In the symlink case, this will fail because the path of the slot->device is never resolved. This MR adds handling for the symlink case to r_slot_find_by_device() by using realpath to resolve the slot's device path.

I could imagine two alternative approaches:

  • Always and unconditionally checking the realpath() instead of only when the direct comparison fails. This would introduce a small additional cost in the regular case.
    Also, it would make the function not identify a slot as mounted if the slot->device does not point to an existing file that is listed as a mount point nonetheless - which is relevant at least in one of the bootchooser unit tests (but most likely not in real-world use-cases because non-existing devices should not be returned by g_unix_mounts_get()).
  • Resolving the paths early after loading the config - I am not sure whether this would be desirable, so I went with the minimally-invasive route.

@amuetzel
src/slot: fix mount point detection for symlinks
4f90e71 
It may be desirable to specify the device of a slot not as the real
device path, but as a symlink to it (e.g. by using a udev-provided
symlink such as /dev/disk/by-id/... instead of /dev/mmcblk...).

The slot mount point detection failed in these cases because the device
passed to r_slot_find_by_device() is always the real path of the device.

Fix this by checking the realpath() of each slot device in addition to
the specified path.

Signed-off-by: Andreas Mützel <andreas.muetzel@emlix.com>
@jluebbe
Copy link
Member

jluebbe commented May 28, 2025

Hmm. I would have thought that we already have something like this...

We could resolve the path during config loading and store it in a new field of RaucSlot. This would allow us to print both in rauc status if they differ, which could be helpful.

Resolving during config loading would rely on the assumption that the symlinks don't change at runtime, but I think that's reasonable. If realpath() fails, we could print a warning and simply copy the existing string to be backwards compatible.

@codecov Codecov
Copy link

codecov bot commented May 28, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 84.49%. Comparing base (a86560c) to head (4f90e71).
Report is 45 commits behind head on master.

Files with missing lines Patch % Lines
src/slot.c 66.66% 2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (66.66%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1722      +/-   ##
==========================================
- Coverage   84.50%   84.49%   -0.01%     
==========================================
  Files          76       76              
  Lines       22381    22387       +6     
==========================================
+ Hits        18913    18917       +4     
- Misses       3468     3470       +2
Flag Coverage Δ
service=false 80.99% <66.66%> (-0.01%) ⬇️
service=true 84.45% <66.66%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@amuetzel
Copy link
Author

amuetzel commented May 28, 2025 •
edited
Loading

This is somewhat similar to #406, which was supposed to fix #388 but never got finished.

The major difference seems to be that #406 also normalizes the path of the mounted device (not just of the mount point specified in the config). That might indeed lead to somewhat strange behaviour if a symlink called like a special filesystem exists in the current RAUC working directory.

For example (not tested, just deduced from the related discussions):

  • Put a symlink tmpfs -> /dev/example into the RAUC working directory
  • Set device=/dev/example in a slot
  • If any "device" tmpfs is returned by g_unix_mounts_get(), the slot might incorrectly get identified as mounted.

This whole situation is avoided in this PR because only the mount point specified in the config is resolved/normalized, not the mount point returned by g_unit_mounts_get().

Resolving this at load time and storing it as an additional field (normalized_path?) has some minor pros and cons compared to this PR:

Pro:

  • Only need to normalize the path once.
  • More easily accessible, should it be needed in other use cases/location (such as rauc status, as you suggested).
  • Possibly easier to test?

Con:

  • Distributes the related code more widely around the codebase.
  • More integration and test effort.
  • Will not handle possibly-changing symlinks correctly, e.g. when updating slots located on hot-pluggable devices that are plugged in after RAUC is started.

I think the last point is the most important point. Are such use-cases relevant?

IMHO, this is mostly an architectural decision - if you prefer, I could adapt this MR to the alternative approach.

@jluebbe jluebbe self-assigned this Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jluebbe jluebbe

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@amuetzel @jluebbe