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

add example how to mount the correct data partition for redundant layout #370

Merged
merged 3 commits into from
Oct 27, 2019

Conversation

FabianKnapp
Copy link

This is one possible solution how to select the correct data partition when using redundant data slots.

As talked with ejoerns at #rauc.

Feel free to modify.

@codecov
Copy link

codecov bot commented Nov 24, 2018

Codecov Report

Merging #370 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #370   +/-   ##
=======================================
  Coverage   66.53%   66.53%           
=======================================
  Files          24       24           
  Lines        6455     6455           
=======================================
  Hits         4295     4295           
  Misses       2160     2160

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 5c35d0d...ba07781. Read the comment docs.

@ejoerns ejoerns added the documentation Patch targets fixing or extending documentation label Nov 26, 2018
Copy link
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

The example implementation provided is still very use-case specific.

I would suggest boiling this down to 2 lines of udev as you can use the expr magic etc. also inside the shell script.

Then having %n only is possibl in this case, but will not work when having data volumes on separate volumes for example, because names are ubi0_x ubi1_y then.

Instead I would use %k, i.e. the udev 'kernel' name for this device. A bit of cut and sed will extract the name you need for comparsion.

@FabianKnapp
Copy link
Author

Yes, you are right, either the shell script or the udev rules are highly use-case specific:

Individual logic is always needed to find the correct rootfs slot for a given data slot.

I decided to make the udev rule more specific since I dont like shell scripting that much ;)

Will provide an example with the individual logic inside the shell script.

@ejoerns
Copy link
Member

ejoerns commented Nov 26, 2018

I decided to make the udev rule more specific since I dont like shell scripting that much ;)

You'd be the first one I know who likes udev scripting more than shell scripting, but.. ok.. 😏

@FabianKnapp
Copy link
Author

For my case the udev was indeed easier since I get the partition for free from udev.

Another thing:
Barebox uses ubi0:$ubiroot notation for booting from nand (which is also returned by cat /proc/cmdline):

bootargs="$bootargs root=ubi0:$ubiroot ubi.mtd=$rootfs_mtdblock"
line 89

But udev uses ubiX_Y notation from the kernel.

Any idea how to solve this without patching / scripting barebox?

@FabianKnapp
Copy link
Author

FabianKnapp commented Nov 26, 2018

Ok, logic inside the shell script.

/usr/bin/is-parent-active :

#!/bin/bash

cmdline=$( cat /proc/cmdline )

# Modify this to get the corresponding rootfs partition from a data partition.
rootfs=$( echo $1 | sed 's/_3/_2/' )
rootfs=$( echo $rootfs | sed 's/_7/_6/' )

if [[ $cmdline == *"root=$rootfs "* ]]; then
	echo 1
else
	echo 0
fi

udev rules:

KERNEL=="ubi[0-9]_[0-9]", PROGRAM="/usr/bin/is-parent-active %k", ENV{isroot}="%c"
KERNEL=="ubi[0-9]_[0-9]", ENV{isroot}=="1", SYMLINK+="data"

Should we use this for documentation?

@ejoerns
Copy link
Member

ejoerns commented Dec 10, 2018

The udev rules now looks fine for me (maybe we can add another example for (more common) sdX devices, such as

KERNEL=="sd?[0-9]", PROGRAM="/usr/bin/is-parent-active %k", ENV{isroot}="%c"

but basically that's it.

The script however is a bit too specifc for an example and I also realized that using %k is not required in any case, but there's nothing bad to about having it ;)

If I remember correctly, you also have access to udev variables (e.g. $ID_PART_TABLE_UUID) from within the script.

One check I've used on a similar setup is

DISK_ID=$(cat /proc/cmdline | sed -e 's/^.*root=PARTUUID=\([0-9a-z-]*\).*/\1/')

[ "$DISK_ID" == "$ID_PART_TABLE_UUID" ]  ...

Maybe we can sum it up to something like

ROOTFS_DEV=<determine rootfs by using proc cmdline or mount>
TEST_DEV=<obtain expected rootfs device for processed device (%k)>

if [[ $ROOTFS_DEV == $TEST_DEV]]; then
	echo 1
else
	echo 0
fi

The actual handling is probably too use-case specific?

@FabianKnapp
Copy link
Author

FabianKnapp commented Dec 11, 2018

Yep, in my opinion

ROOTFS_DEV=<determine rootfs by using proc cmdline or mount>
TEST_DEV=<obtain expected rootfs device for processed device (%k)>

if [[ $ROOTFS_DEV == $TEST_DEV]]; then
	echo 1
else
	echo 0
fi

is the best for documentation. Maybe, we can add one specific example.

@jluebbe
Copy link
Member

jluebbe commented Dec 20, 2018

I'm wondering if this can't be done by letting udev look at the kernel cmdline:

IMPORT{cmdline}="root"
SUBSYSTEM="ubi", ENV{root}=="/dev/ubi0:aroot", ATTRS{mtd_num}=="1", ATTRS{name}=="adata", SYMLINK+="data"
SUBSYSTEM="ubi", ENV{root}=="/dev/ubi0:broot", ATTRS{mtd_num}=="1", ATTRS{name}=="bdata", SYMLINK+="data"

That way you have an explicit mapping by name. I've not tried this very recently, so I'm not sure I got the details correct.

Also often useful is:

SUBSYSTEM=="ubi", KERNEL=="ubi*_*", SYMLINK+="ubi_mtd%s{mtd_num}_%s{name}"

@ejoerns
Copy link
Member

ejoerns commented Jan 4, 2019

@jluebbe is right for this case and maybe we should use this by default.

It will not work when dealing with PARTUUID's as I had it in my setup, there we need a solution similar to what is in @FabianKnapp's last comment.

So, we should maybe start with the simple ubifs example and shortly note that other as we already did.

@FabianKnapp would you like to?

@FabianKnapp
Copy link
Author

As always: Feel free to modify.

@jluebbe
Copy link
Member

jluebbe commented Jan 28, 2019

As always: Feel free to modify.

I've added another commit on top, which simplifies the udev rules to get rid of the temporary variable. That makes it possible to use just a single rule.
I don't have a system ready to try this, but it should work like this. :)
If you have a chance to test this successfully, I'd squash these commits into one and merge it.
Were you able to try the approach based on IMPORT{cmdline}="root" and no external tool?

@FabianKnapp
Copy link
Author

Were you able to try the approach based on IMPORT{cmdline}="root" and no external tool?

Not yet, but it is on my todo's

@jluebbe jluebbe added this to the Release v1.2 milestone Jun 14, 2019
@thirtythreeforty
Copy link

thirtythreeforty commented Oct 7, 2019

We hit this problem, so we solved it with a small tool and a simple configuration file:

https://github.com/KopisMobile/bootslot-mounts

It does require systemd, which is a lot of complexity on a very small system. But if you're already using systemd, it's pretty straightforward to use.

… a redundant data layout

Signed-off-by: Fabian Knapp <knapp@ambibox.de>
@jluebbe
Copy link
Member

jluebbe commented Oct 25, 2019

I've updated the branch to current master and changed some minor wording.

@jluebbe jluebbe requested a review from ejoerns October 25, 2019 09:33
@jluebbe jluebbe changed the title [Doc] Added example how to mount the correct data partition for redundant layout. add example how to mount the correct data partition for redundant layout Oct 25, 2019
jluebbe and others added 2 commits October 27, 2019 20:46
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Enrico Jörns <ejo@pengutronix.de>
Signed-off-by: Enrico Joerns <ejo@pengutronix.de>
Copy link
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a minor fix in the example script and updated CHANGES.

Looks fine for me now.

@jluebbe jluebbe merged commit 0dd4ab6 into rauc:master Oct 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Patch targets fixing or extending documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants