Re: [PATCH v2 2/2] execute do_wic_image under a lock to ensure mountpoints remain mounted

[Jan Kiszka <jan.kiszka@web.de>]

On 23.09.21 15:12, Felix Moessbauer wrote:
> This patch adds the isar.lock to the do_wic_image task to ensure that
> the mountpoints are not unmounted by a simultaneously running wic task.

As long as we are not bookkeeping mount and umounts, it's better to
avoid locking of the whole task, only synchronize mounting against
competing runs, and leave umount to the cleanup handler.

Also, the lock is way too broad when it is repo-anchored. Only users of
the same buildchroot compete here.

Jan

> Further, it makes mounting more robust by executing the
> wic task in a try / finally handler so that the mount points are
> unmounted even if the task itself fails.
>
> Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> ---
>  meta/classes/wic-img.bbclass | 34 +++++++++++++++++++++-------------
>  1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/meta/classes/wic-img.bbclass b/meta/classes/wic-img.bbclass
> index d849ad9..cd97fa9 100644
> --- a/meta/classes/wic-img.bbclass
> +++ b/meta/classes/wic-img.bbclass
> @@ -138,23 +138,40 @@ python check_for_wic_warnings() {
>
>  do_wic_image[file-checksums] += "${WKS_FILE_CHECKSUM}"
>  python do_wic_image() {
> -    bb.build.exec_func("generate_wic_image", d)
> +    lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock", shared=False)
> +    bb.build.exec_func("wic_do_mounts", d)
> +    try:
> +        bb.build.exec_func("generate_wic_image", 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
>
> -generate_wic_image() {
> +wic_do_mounts() {
>      buildchroot_do_mounts
>      sudo -s <<'EOSUDO'
> -        ( flock 9
>          for dir in ${BBLAYERS} ${STAGING_DIR} ${SCRIPTSDIR} ${BITBAKEDIR}; do
>              mkdir -p ${BUILDCHROOT_DIR}/$dir
>              if ! mountpoint ${BUILDCHROOT_DIR}/$dir >/dev/null 2>&1; then
>                  mount --bind --make-private $dir ${BUILDCHROOT_DIR}/$dir
>              fi
>          done
> -        ) 9>${MOUNT_LOCKFILE}
>  EOSUDO
> +}
> +
> +wic_undo_mounts() {
> +    sudo -s <<'EOSUDO'
> +        for dir in ${BBLAYERS} ${STAGING_DIR} ${SCRIPTSDIR} ${BITBAKEDIR}; do
> +            if mountpoint -q ${BUILDCHROOT_DIR}/$dir; then
> +                umount ${BUILDCHROOT_DIR}/$dir
> +            fi
> +        done
> +EOSUDO
> +}
> +
> +generate_wic_image() {
>      export FAKEROOTCMD=${FAKEROOTCMD}
>      export BUILDDIR=${BUILDDIR}
>      export MTOOLS_SKIP_CHECK=1
> @@ -200,13 +217,4 @@ EOSUDO
>      done
>      rm -rf ${BUILDCHROOT_DIR}/${WICTMP}
>      rm -rf ${IMAGE_ROOTFS}/../pseudo
> -    sudo -s <<'EOSUDO'
> -        ( flock 9
> -        for dir in ${BBLAYERS} ${STAGING_DIR} ${SCRIPTSDIR} ${BITBAKEDIR}; do
> -            if mountpoint -q ${BUILDCHROOT_DIR}/$dir; then
> -                umount ${BUILDCHROOT_DIR}/$dir
> -            fi
> -        done
> -        ) 9>${MOUNT_LOCKFILE}
> -EOSUDO
>  }
>