[LEDE-DEV] [PATCH] fstools: add middle layer (original root overlay) to overlayfs when pivot the /overlay to the USB disk.

[John Crispin]

On 11/05/18 11:18, Andrzej Lossowsk wrote:
> Currently after pivot to the USB disk (extroot),
> configuration and data from original root overlay are gone.
> Still accessible, but all previous configuration steps must be repeated
> (or data from original root overlay must be copied to new USB disk overlay).
>
> The idea is to "merge" original root overlay with overlay from the USB disk.
> Using overlayfs multiple lower layers it can be done.
> All data from original root overlay will be accessible (as readonly layer) after pivot to the USB disk.
>
> Original root overlay (for exmaple /dev/mtdblock3) will be mounted to /overlay/internal (as readonly)
> Upper directory from original root overlay (/dev/mtdblock3/upper) will be the top (readonly layer),
> then "/" will be the bottom layer (readonly layer), an extroot still will be upper (writable layer).

Hi,
some code style comments inline.

     john

> Signed-off-by: Andrzej Lossowski <alossek at wp.pl>
> ---
>   libfstools/extroot.c    | 65 ++++++++++++++++++++++++++++++++++++++++++++++++-
>   libfstools/libfstools.h |  3 +++
>   libfstools/mount.c      | 27 +++++++++++++++++---
>   3 files changed, 90 insertions(+), 5 deletions(-)
>
> diff --git a/libfstools/extroot.c b/libfstools/extroot.c
> index 418df94..bbb0a3a 100644
> --- a/libfstools/extroot.c
> +++ b/libfstools/extroot.c
> @@ -22,6 +22,7 @@
>   #include <libgen.h>
>   
>   #include "libfstools.h"
> +#include "volume.h"
>   
>   char const *extroot_prefix = NULL;
>   
> @@ -104,7 +105,7 @@ int mount_extroot(void)
>   				if (mount_move("/tmp/extroot", "", "/overlay")) {
>   					ULOG_ERR("moving extroot failed - continue normal boot\n");
>   					umount("/tmp/extroot/overlay");
> -				} else if (fopivot("/overlay", "/rom")) {
> +				} else if (fopivot_multi("/overlay", "/rom")) {
>   					ULOG_ERR("switching to extroot failed - continue normal boot\n");
>   					umount("/overlay");
>   				} else {
> @@ -119,3 +120,65 @@ int mount_extroot(void)
>   	}
>   	return -1;
>   }
> +
> +int fopivot_multi(char *rw_root, char *ro_root)
> +{
> +	if (mount_internal_overlay()) {
> +		ULOG_ERR("mounting /tmp/overlay failed\n");
> +	} else if (mkdir("/overlay/internal", 0755)) {
> +		ULOG_ERR("failed to mkdir /overlay/internal: %m\n");
> +	} else if (mount_move("/tmp/overlay", "/overlay/internal", "")) {
> +		umount("/tmp/overlay");
> +	} else if (mfopivot(rw_root, ro_root, "/overlay/internal/upper")) {
> +		umount("/overlay/internal");
> +	} else {
> +		return 0;
> +	}

dont build a if then else chain. call a function check the return value 
and exit if there is a fail. worste case use a goto if you want a single 
return point.

> +	return -1;
> +}
> +
> +int mount_internal_overlay()
move this above fopivot_multi, make it static and drop the prototype 
inside the header2
> +{
> +	struct volume *v = volume_find("rootfs_data");
> +
> +	if (!v)
> +		return -1;
> +
> +	volume_init(v);
> +	char *mp;

dont define variables in the middle of a function please
> +	mp = find_mount_point(v->blk, 0);
> +	if (mp) {
> +		ULOG_ERR("rootfs_data:%s is already mounted as %s\n", v->blk, mp);
> +		return -1;
> +	}
> +
> +	char *fstype;
same here
> +	switch (volume_identify(v)) {
> +		case FS_EXT4:
> +			fstype = "ext4";
> +			break;
> +		case FS_F2FS:
> +			fstype = "f2fs";
> +			break;
> +		case FS_UBIFS:
> +			fstype = "ubifs";
> +			break;
> +		case FS_JFFS2:
> +		default:
> +			fstype = "jffs2";
> +			break;
> +	}
> +
> +	if (mkdir("/tmp/overlay", 0755)) {
> +		ULOG_ERR("failed to mkdir /tmp/overlay: %m\n");
> +		return -1;
> +	}
> +
> +	if (mount(v->blk, "/tmp/overlay", fstype, MS_NOATIME | MS_RDONLY, NULL)) {
> +		ULOG_ERR("failed to mount -t %s %s /tmp/overlay: %m\n", fstype, v->blk);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> diff --git a/libfstools/libfstools.h b/libfstools/libfstools.h
> index f27307a..8c740a4 100644
> --- a/libfstools/libfstools.h
> +++ b/libfstools/libfstools.h
> @@ -46,6 +46,9 @@ extern int mount_overlay(struct volume *v);
>   extern int mount_move(const char *oldroot, const char *newroot, const char *dir);
>   extern int pivot(char *new, char *old);
>   extern int fopivot(char *rw_root, char *ro_root);
> +extern int mfopivot(char *rw_root, char *ro_lowerlevel0, char *ro_lowerlevel1);
> +extern int fopivot_multi(char *rw_root, char *ro_root);
> +extern int mount_internal_overlay();
>   extern int ramoverlay(void);
>   
>   extern int find_overlay_mount(char *overlay);
> diff --git a/libfstools/mount.c b/libfstools/mount.c
> index c7f2789..bb82390 100644
> --- a/libfstools/mount.c
> +++ b/libfstools/mount.c
> @@ -94,7 +94,20 @@ pivot(char *new, char *old)
>   int
>   fopivot(char *rw_root, char *ro_root)
>   {
> -	char overlay[64], mount_options[64];
> +	return mfopivot(rw_root, ro_root, NULL);
> +}
> +
just drop all invocations of fopivot and use the new signature instead.
> +/**
> + * fopivot - switch to overlay using passed dir as upper one
> + *
> + * @rw_root: writable directory that will be used as upper dir
> + * @ro_root: directory where old root will be put
> + * @ro_lowerlevel1: directory where internal overlay will be put
> + */
> +int
> +mfopivot(char *rw_root, char *ro_root, char *ro_lowerlevel1)
> +{
> +	char overlay[64], mount_options[128];
>   
>   	if (find_filesystem("overlay")) {
>   		ULOG_ERR("BUG: no suitable fs found\n");
> @@ -110,15 +123,21 @@ fopivot(char *rw_root, char *ro_root)
>   	 */
>   	snprintf(mount_options, sizeof(mount_options), "lowerdir=/,upperdir=%s", rw_root);
>   	if (mount(overlay, "/mnt", "overlayfs", MS_NOATIME, mount_options)) {
> -		char upperdir[64], workdir[64], upgrade[64], upgrade_dest[64];
> +		char upperdir[64], workdir[64], upgrade[64], upgrade_dest[64], lowerdir[64];
>   		struct stat st;
>   
>   		snprintf(upperdir, sizeof(upperdir), "%s/upper", rw_root);
>   		snprintf(workdir, sizeof(workdir), "%s/work", rw_root);
>   		snprintf(upgrade, sizeof(upgrade), "%s/sysupgrade.tgz", rw_root);
>   		snprintf(upgrade_dest, sizeof(upgrade_dest), "%s/sysupgrade.tgz", upperdir);
> -		snprintf(mount_options, sizeof(mount_options), "lowerdir=/,upperdir=%s,workdir=%s",
> -			 upperdir, workdir);
> +
> +		if (ro_lowerlevel1)
> +			snprintf(lowerdir, sizeof(lowerdir), "%s:/", ro_lowerlevel1);
> +		else
> +			snprintf(lowerdir, sizeof(lowerdir), "/");
> +
> +		snprintf(mount_options, sizeof(mount_options), "lowerdir=%s,upperdir=%s,workdir=%s",
> +			lowerdir, upperdir, workdir);
>   
>   		/*
>   		 * Overlay FS v23 and later requires both a upper and