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

[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

	John