Re: [PATCH 4/4] split rootfs finalization and unmounting into two tasks

[Henning Schild <henning.schild@siemens.com>]

That split is nice. But i would try and run it always after in a catch
block, not in another task.
That way it will become more robust while not having to touch the
task-chain ... which is not nice for layers that hook in "low".

I would not hold this one back for a series of "improved mounting" ...
divide and conquer! If it fixes an issue it is valid, only if you have
the time to do a full "mount rework" i would hold it back.

Henning

Am Wed, 22 Sep 2021 11:27:54 +0200
schrieb Felix Moessbauer <felix.moessbauer@siemens.com>:

> This patch separates the unmounting logic from do_rootfs_finalize
> to inject tasks in between. By that, the do_wic_image tasks can
> use the mount-points and finally everything is unmounted correctly.
> 
> Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> ---
>  meta/classes/image.bbclass   | 49
> +++++++++++++++++++++++++----------- meta/classes/wic-img.bbclass |
> 4 +-- 2 files changed, 36 insertions(+), 17 deletions(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 1f39690..988f0d7 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -190,21 +190,6 @@ do_rootfs_finalize() {
>              find "${ROOTFSDIR}/usr/bin" \
>                  -maxdepth 1 -name 'qemu-*-static' -type f -delete
>  
> -        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
> -            umount -l ${ROOTFSDIR}/isar-apt
> -        rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
> -
> -        mountpoint -q '${ROOTFSDIR}/base-apt' && \
> -            umount -l ${ROOTFSDIR}/base-apt
> -        rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
> -
> -        mountpoint -q '${ROOTFSDIR}/dev' && \
> -            umount -l ${ROOTFSDIR}/dev
> -        mountpoint -q '${ROOTFSDIR}/proc' && \
> -            umount -l ${ROOTFSDIR}/proc
> -        mountpoint -q '${ROOTFSDIR}/sys' && \
> -            umount -l ${ROOTFSDIR}/sys
> -
>          rm -f "${ROOTFSDIR}/etc/apt/apt.conf.d/55isar-fallback.conf"
>  
>          rm -f "${ROOTFSDIR}/etc/apt/sources.list.d/isar-apt.list"
> @@ -219,5 +204,39 @@ EOSUDO
>  }
>  addtask rootfs_finalize before do_rootfs after do_rootfs_postprocess
>  
> +do_rootfs_unmount() {
> +    sudo -s <<'EOSUDO'
> +        ret=0
> +        if mountpoint -q '${ROOTFSDIR}/isar-apt'; then
> +            umount -vl ${ROOTFSDIR}/isar-apt || ret=1
> +        fi
> +        if test -d ${ROOTFSDIR}/isar-apt; then
> +            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
> || ret=1
> +        fi
> +
> +        if mountpoint -q ${ROOTFSDIR}/base-apt; then
> +            umount -vl ${ROOTFSDIR}/base-apt || ret=1
> +        fi
> +        if test -d ${ROOTFSDIR}/base-apt; then
> +            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
> || ret=1
> +        fi
> +
> +        if mountpoint -q ${ROOTFSDIR}/dev; then
> +            umount -vl ${ROOTFSDIR}/dev || ret=1
> +        fi
> +        if mountpoint -q ${ROOTFSDIR}/proc; then
> +            umount -vl ${ROOTFSDIR}/proc || ret=1
> +        fi
> +        if mountpoint -q '${ROOTFSDIR}/sys'; then
> +            umount -vl ${ROOTFSDIR}/sys || ret=1
> +        fi
> +
> +        if [$ret -ne 0]; then
> +            bbwarn "could not unmount all mountpoints"
> +        fi
> +EOSUDO
> +}
> +addtask rootfs_unmount before do_build after do_rootfs_finalize
> +
>  # Last so that the image type can overwrite tasks if needed
>  inherit ${IMAGE_TYPE}
> diff --git a/meta/classes/wic-img.bbclass
> b/meta/classes/wic-img.bbclass index 573537c..0f5932b 100644
> --- a/meta/classes/wic-img.bbclass
> +++ b/meta/classes/wic-img.bbclass
> @@ -142,12 +142,12 @@ python do_wic_image() {
>      bb.build.exec_func("wic_do_mounts", d)
>      try:
>          bb.build.exec_func("generate_wic_image", d)
> -        bb.build.exec_func("check_for_wic_warnings", d)
>      finally:
>          bb.build.exec_func("wic_undo_mounts", d)
>          bb.utils.unlockfile(lock)
> +    bb.build.exec_func("check_for_wic_warnings", d)
>  }
> -addtask wic_image before do_image after do_image_tools
> do_rootfs_finalize +addtask wic_image before do_image
> do_rootfs_unmount after do_image_tools 
>  wic_do_mounts() {
>      buildchroot_do_mounts