Hi John,

thansk for taking a look. I actually sent it this way by intention,
looking to get in touch with someone who had the original bug.

Am 29.06.2016 um 08:30 schrieb John Crispin:
> Hi,
> 
> the patch is an attachement making inline commenting impossible. please
> send patches inline
>From 59a8fa6d7490ba3da76aec710a1beb241ffaa089 Mon Sep 17 00:00:00 2001
From: Josua Mayer <[email protected]>
Date: Thu, 16 Jun 2016 18:35:30 +0200
Subject: [PATCH 2/4] mount_root: Don't mount ext4 rootfs twice

When there is a) no rootfs_data overlay partition,
and b) /dev/root points to an ext4 partition
the partition would be mounted twice, once as / and then as /overlay.
The essence of this change is to return before mounting /overlay,
if /dev/root has been mounted as /.
>
Signed-off-by: Josua Mayer <[email protected]>
---
 mount_root.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/mount_root.c b/mount_root.c
index 608ce5d..13e5772 100644
--- a/mount_root.c
+++ b/mount_root.c
@@ -37,25 +37,45 @@ start(int argc, char *argv[1])
        if (!getenv("PREINIT") && stat("/tmp/.preinit", &s))
                return -1;

+       /*
+        * When the default overlay partition name rootfs_data can not be found,
+        * fall back to the special /dev/root device.
+        */
        if (!data) {
                root = volume_find("rootfs");
                volume_init(root);
+
+               // mount /dev/root at /
                ULOG_NOTE("mounting /dev/root\n");
                mount("/dev/root", "/", NULL, MS_NOATIME | MS_REMOUNT, 0);
-       }

-       /*
-        * Before trying to mount and use "rootfs_data" let's check if there is
-        * extroot configured. Following call will handle reading config from
-        * the "rootfs_data" on its own.
-        */
-       extroot_prefix = "";
-       if (!mount_extroot()) {
-               ULOG_NOTE("switched to extroot\n");
+               /*
+                * Now that / has been mounted, and there is no overlay device,
+                * see if extroot is configured.
+                *
+                * The following call will handle reading configuration from
+                * rootfs on its own.
+                */
+               extroot_prefix = "";
+               if (!mount_extroot()) {
+                       ULOG_NOTE("switched to extroot\n");
+                       /*
+                        * extroot succeeded mounting an overlay partition, 
return.
+                        */
+                       return 0;
+               }
+
+               /*
+                * Even if extroot was not configured, considering that no 
overlay
+                * partition was found, and / was mounted, return now.
+                */
                return 0;
        }

-       /* There isn't extroot, so just try to mount "rootfs_data" */
+       /*
+        * neither /dev/root nor extroot were used.
+        * Attempt to mount the overlay partition.
+        */
        switch (volume_identify(data)) {
        case FS_NONE:
                ULOG_WARN("no usable overlay filesystem found, using tmpfs 
overlay\n");
-- 
2.6.6
> 
>> mount_root: Don't mount ext4 rootfs twice
> 
> the patch is incorrect, it breaks the vase where there is an overlay
> setup + extroot. after your patch the extroot codepath wont ever run if
> there is a rootfs_data partition
Good point. I had indeed been thinking about this while creating the
patch. I would really be interested if you derived that from reading the
code, or testing. Eitehr way here is what I think how it should work:
If there is rootfs_data, the code wil proceed till down to the switch
statement, and then end up either in a) ramoverlay(), b)
mount_overlay(data) or c) mount_snapshot(data).

a) extroot will not happen.
b) mount_overlay calls mount_extroot
c) I didnt think of that case, and it doesnt mount extroot.

The only change, and I really hope I am not reading my code wrong (if
so, correct me please) is that besides a bunch of comments, is that when
there is no rootfs_data, and /dev/root gets mounted as /, the code
immediately returns before either a), b) or c).
> 
>       John
> 
br
Josua Mayer

_______________________________________________
Lede-dev mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/lede-dev

Reply via email to