Normalize device names to find mounted slots #406

xyzzy42 · 2019-02-08T20:54:07Z

This way a mounted slot can be found it the path used to mount the
device is not the same as the path in the rauc configuration file.
E.g., the configuration file uses a link like
/dev/disk/by-partlabel/rootfs0 while the kernel's list of mounted
devices uses a name like /dev/mmcblk1p2.

Not being able to detect that the current rootfs is already mounted
causes rauc to try to mount the slot, which will fail.

Fixes #388.

Signed-off-by: Trent Piepho [email protected]

ejoerns · 2019-02-11T11:38:16Z

👍 Thanks for your submission!
@xyzzy42 please let uncrustify run on your code and fix the errors mentioned by Travis so we can have this PR ready for review.

xyzzy42 · 2019-02-11T21:00:27Z

There's an issue getting the tests to work.

find_slot_by_device/find_config_slot_by_device are used twice in rauc's code, both times on a running target system where we want to know if a real device corresponds to a slot.

It's also used over a dozen times in the test code. This use doesn't work as intended because the devices, both the one used to look up a slot and the device named in the slot, do not exist. On a target, we couldn't say if they are the same or not. So the lookup fails. In actual use this behavior seems to me like it would be correct. The "is mounted?" and "was booted from?" checks should return false given a device that does not exist.

It looks to me like find_config_slot_by_device() was being used simply as away to get a handle to slot, lacking any other way. It wasn't intended to test the by device lookup code. Maybe a better way would be to create a find_slot_by_name() function and use that in the test code?

src/config_file.c

xyzzy42 · 2019-02-25T19:39:34Z

Believe I have addressed all comments.

I changed it to do the pathname compare first, device based compare second. The path string compare is much cheaper (no syscalls), and if the paths are the same, then the device comparison must the same too (excluding races!). So this is a faster way to get the same result as device compare first, path compare second.

xyzzy42 · 2019-03-20T21:49:43Z

Is there anything else left here? I believe I've made all requested changes.

src/install.c

obbardc · 2020-01-30T10:00:46Z

Hi @ejoerns what is the status of this pull request, please?

ejoerns · 2020-01-30T10:52:10Z

@obbardc the 'funny' thing is that it just hit me, too in a project today. There I still use a different method for solving the same issue and iirc we had a 3rd one somewhere around, too.

We definitely need to face and fix this. This PR should not have been open for so long

@obbardc Did you already test these patches?

obbardc · 2020-01-30T10:56:26Z

@obbardc the 'funny' thing is that it just hit me, too in a project today. There I still use a different method for solving the same issue and iirc we had a 3rd one somewhere around, too.
We definitely need to face and fix this. This PR should not have been open for so long
@obbardc Did you already test these patches?

Hardcoded device names is not great, but for now what I have works.
Not tested the patches yet; it's on my list very early next week :-)

jluebbe · 2020-02-10T11:47:53Z

@ejoerns I've rebased this onto the current master, which had moved some functions to src/slot.c.

codecov · 2020-02-10T11:55:47Z

Codecov Report

❗ No coverage uploaded for pull request base (master@889602b). Click here to learn what that means.
The diff coverage is 75.67%.

@@            Coverage Diff            @@
##             master     #406   +/-   ##
=========================================
  Coverage          ?   66.34%           
=========================================
  Files             ?       24           
  Lines             ?     6516           
  Branches          ?        0           
=========================================
  Hits              ?     4323           
  Misses            ?     2193           
  Partials          ?        0

Impacted Files	Coverage Δ
include/config_file.h	`100.00% <ø> (ø)`
include/slot.h	`0.00% <ø> (ø)`
src/slot.c	`90.37% <68.00%> (ø)`
src/install.c	`80.43% <91.66%> (ø)`
src/config_file.c	`82.36% <0.00%> (ø)`
src/emmc.c	`0.00% <0.00%> (ø)`
src/bundle.c	`65.13% <0.00%> (ø)`
src/network.c	`77.21% <0.00%> (ø)`
src/manifest.c	`80.24% <0.00%> (ø)`
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 889602b...8b758bf. Read the comment docs.

jluebbe · 2020-02-10T11:55:10Z

src/install.c

+ * etc. which we want to ignore rather than try to follow some
+ * random symlink named "tmpfs" in the CWD.
+ */
+ if (g_unix_mount_get_device_path(m)[0] != '/')


I'm not sure this is correct for ubifs, which has device names such as ubi0:data.

jluebbe · 2020-02-10T11:55:39Z

src/slot.c

+ if (g_stat(devicepath, &st) == -1) {
+ /* Virtual filesystems like devpts trigger case */
+ g_debug("Can't stat '%s', assuming unmountable: %s", devicepath, g_strerror(errno));
+ return NULL;


This will break support for UBIFS.

jluebbe · 2020-02-10T11:56:28Z

src/slot.c


 g_return_val_if_fail(slots, NULL);
 g_return_val_if_fail(device, NULL);

+ obj = normalize_mountable_object(device);


How should we handle NULL here?

jluebbe · 2020-02-10T11:57:26Z

I think we can merge the first two commits as they are, though.

This way a mounted slot can be found if the path used to mount the device is not the same as the path in the rauc configuration file. E.g., the configuration file uses a link like /dev/disk/by-partlabel/rootfs0 while the kernel's list of mounted devices uses a name like /dev/mmcblk1p2. This is a fallback after simply comparing the paths, as if the paths are the same then the devices must the same too. This avoids changing behavior in cases there the devices do not exist, for instance if rauc status before a device is online. Not being able to detect that the current rootfs is already mounted causes rauc to try to mount the slot, which will fail. Fixes rauc#388. Signed-off-by: Trent Piepho <[email protected]>

jluebbe · 2020-04-21T09:15:50Z

I've merged the first two commits and rebased this PR.

jluebbe · 2020-04-21T10:52:14Z

The open comments need to be solved, but the general approach seems fine.

jluebbe · 2022-01-27T15:02:57Z

The open points still need to be resolved and we'd need test-cases to cover the new alternatives. Moving to the next release.

ejoerns added the enhancement Adds new functionality or enhanced handling to RAUC label Feb 9, 2019

xyzzy42 force-pushed the slot-mounted-fix branch from 31c469b to 213a68e Compare February 11, 2019 20:35

xyzzy42 force-pushed the slot-mounted-fix branch 2 times, most recently from 03c1130 to 43c1888 Compare February 11, 2019 23:46

ejoerns requested changes Feb 12, 2019

View reviewed changes

src/config_file.c Outdated Show resolved Hide resolved

src/config_file.c Outdated Show resolved Hide resolved

src/config_file.c Outdated Show resolved Hide resolved

src/config_file.c Outdated Show resolved Hide resolved

xyzzy42 force-pushed the slot-mounted-fix branch 2 times, most recently from 6a0ae0d to ef102e1 Compare February 25, 2019 19:22

xyzzy42 force-pushed the slot-mounted-fix branch from ef102e1 to def12c7 Compare March 20, 2019 21:48

ejoerns reviewed Mar 21, 2019

View reviewed changes

src/install.c Outdated Show resolved Hide resolved

ejoerns added this to the Release v1.1 milestone Mar 21, 2019

xyzzy42 force-pushed the slot-mounted-fix branch 2 times, most recently from adac6a3 to a235dee Compare March 21, 2019 22:27

ejoerns previously approved these changes Mar 26, 2019

View reviewed changes

ejoerns requested a review from jluebbe April 15, 2019 10:46

ejoerns removed this from the Release v1.1 milestone Jun 4, 2019

jluebbe added this to the Release v1.2 milestone Jun 14, 2019

ejoerns removed this from the Release v1.2 milestone Oct 27, 2019

jluebbe self-assigned this Feb 10, 2020

jluebbe dismissed ejoerns’s stale review via 857f08a February 10, 2020 11:47

jluebbe force-pushed the slot-mounted-fix branch from a235dee to 857f08a Compare February 10, 2020 11:47

jluebbe requested changes Feb 10, 2020

View reviewed changes

jluebbe requested a review from ejoerns February 10, 2020 11:57

jluebbe force-pushed the slot-mounted-fix branch from 857f08a to 8b758bf Compare April 21, 2020 09:15

jluebbe added the needs rework label Apr 21, 2020

jluebbe added this to the Release v1.4 milestone Apr 21, 2020

ejoerns modified the milestones: Release v1.4, Release v1.5 Jul 16, 2020

ejoerns modified the milestones: Release v1.5, Release 1.6 Jan 4, 2021

LeSpocky mentioned this pull request Oct 6, 2021

RFC: Support alternative ubi rootfs format #777

Closed

jluebbe modified the milestones: Release v1.6, Release v1.7 Jan 27, 2022

jluebbe modified the milestones: Release v1.7, Release v1.8 Jun 3, 2022

ejoerns modified the milestones: Release v1.8, Release v1.9 Sep 30, 2022

ejoerns modified the milestones: Release v1.9, Unplanned Mar 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalize device names to find mounted slots #406

Normalize device names to find mounted slots #406

xyzzy42 commented Feb 8, 2019

ejoerns commented Feb 11, 2019

xyzzy42 commented Feb 11, 2019

xyzzy42 commented Feb 25, 2019

xyzzy42 commented Mar 20, 2019

obbardc commented Jan 30, 2020

ejoerns commented Jan 30, 2020

obbardc commented Jan 30, 2020 •

edited

jluebbe commented Feb 10, 2020

codecov bot commented Feb 10, 2020 •

edited

jluebbe Feb 10, 2020

jluebbe Feb 10, 2020

jluebbe Feb 10, 2020

jluebbe commented Feb 10, 2020

jluebbe commented Apr 21, 2020

jluebbe commented Apr 21, 2020

jluebbe commented Jan 27, 2022

Normalize device names to find mounted slots #406

Are you sure you want to change the base?

Normalize device names to find mounted slots #406

Conversation

xyzzy42 commented Feb 8, 2019

ejoerns commented Feb 11, 2019

xyzzy42 commented Feb 11, 2019

xyzzy42 commented Feb 25, 2019

xyzzy42 commented Mar 20, 2019

obbardc commented Jan 30, 2020

ejoerns commented Jan 30, 2020

obbardc commented Jan 30, 2020 • edited

jluebbe commented Feb 10, 2020

codecov bot commented Feb 10, 2020 • edited

Codecov Report

jluebbe Feb 10, 2020

Choose a reason for hiding this comment

jluebbe Feb 10, 2020

Choose a reason for hiding this comment

jluebbe Feb 10, 2020

Choose a reason for hiding this comment

jluebbe commented Feb 10, 2020

jluebbe commented Apr 21, 2020

jluebbe commented Apr 21, 2020

jluebbe commented Jan 27, 2022

obbardc commented Jan 30, 2020 •

edited

codecov bot commented Feb 10, 2020 •

edited