[LEDE-DEV] fstools: ext4 overlay support - rootfs mounted twice bug

[John Crispin]

On 29/06/2016 12:34, Josua Mayer wrote:
> 
> 
> Am 29.06.2016 um 12:21 schrieb John Crispin:
>>
>>
>> On 29/06/2016 12:11, Josua Mayer wrote:
>>> 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 <josua.mayer97 at gmail.com>
>>> 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 <josua.mayer97 at gmail.com>
>>> ---
>>>  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");
>>>
>>
>> derived from looking at the patch. as the patch is not inline i am not
>> able to actually import it locally so i can only judge it by looking at it.
>>
>> after applying your patch, mount_extroot() would be hidden behind a
>> if(!data) {} which means that it will only ever be called if there is no
>> rootfs_data
> That is only partially right. Like I pointed out, it is called inside
> mount_overlay().
> If there is a way to fix this, I need to understand what should happen:
> code path before my patch:
> data = volume_find("rootfs_data");
> if (!data) {/* mount /dev/root as / */}
> mount_extroot()
> switch (volume_identify(data))
> 
> From this artificial trace, I extracted these runtime conditions for
> extroot:
> 1. rootfs_data exists but not mounted yet
> 2. rootfs_data does not exist and /dev/root mounted as /
> I then guessed that mount_extroot only makes sense when there is an
> overlay partition, or the read-write rootfs mounted in order to have its
> configuration available.
> So my guess was: if overlay isnt mounted yet but will be later, no point
> in executing mount_extroot. Especially considering it will be inherently
> called from the call to mount_overlay thats occuring later on.
>>
>> 	John
>>
> 
> _______________________________________________
> Lede-dev mailing list
> Lede-dev at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
> 

send a proper inline patch so i can test this on device please.

	John