[PATCH v6 0/6] Restore downstream mounts compatibility

[PATCH v6 0/6] Restore downstream mounts compatibility

[Anton Mikanovich]

Revert commit d21d49578e5a3b0019075d1946bd93a95914fcca which has broken
compatibility with downstream projects. Move try-finally from
dpkg_runbuild(), etc. to the caller, do_dpkg_build().

Also revert e438c8f62b9aacb2c9d5120e93f81637f091a83c and
3360b7d9ff85172dc3d2275c35caec71149b35ef commits to prevent 'umount:
target is busy' issues caused by /dev and /sys mounts stayed locked
even after task finish in some cases. Those two commits should be
applied back after this issue is found and fixed.

Move mounts left printing from warnings to debug output bacause
per-task unmounting was reverted.

Tested with kas-iot2050-example project from:
https://github.com/siemens/meta-iot2050

---
Changes sinse v5:
- Revert buildchroot per-task unmounts.
- Revert rootfs per-task unmounts.
- Remove imager per-task unmounts.
Changes sinse v4:
- Fix while loop counts.
- Move unmount warnings under debug.
Changes since v3:
- Warn on first entering in umount while loop.
- Do not warn on left mounts by default.
- Remote incorrect dpkg unmount usage protection.
- Rebase on next.
- Improve commit messages.
Changes since v2:
- Remove incorrect buildchroot/rootfs mount/unmount usage protection.
- Remove task fail handler.
- Implement double mount protection by try-finally.
Changes since v1:
- Get back while loop in dpkg_undo_mounts.
- Exit dpkg_undo_mounts if already mounted.
- Do not ignore exceptions in safe_exec.

Anton Mikanovich (6):
  Revert "dpkg: Make mount buildroot reliable"
  dpkg: Limit unmount loop
  dpkg-base: Clean up unmounting in do_dpkg_build()
  events: Do not warn on left mounts by default
  Revert "rootfs: Unmount rootfs mounts if not needed"
  Revert "buildchroot: Unmount buildchroot mounts if not needed"

 meta/classes/buildchroot.bbclass              |  47 -------
 meta/classes/dpkg-base.bbclass                | 123 +++++++-----------
 meta/classes/dpkg-gbp.bbclass                 |   8 +-
 meta/classes/dpkg.bbclass                     |  14 +-
 meta/classes/ext4-img.bbclass                 |   2 -
 meta/classes/fit-img.bbclass                  |   2 -
 meta/classes/image-tools-extension.bbclass    |   2 -
 meta/classes/image.bbclass                    |  24 ++--
 meta/classes/initramfs.bbclass                |   2 -
 meta/classes/isar-events.bbclass              |   2 +-
 meta/classes/rootfs.bbclass                   |  63 +--------
 meta/classes/ubi-img.bbclass                  |   2 -
 meta/classes/ubifs-img.bbclass                |   2 -
 meta/classes/wic-img.bbclass                  |   1 -
 .../buildchroot/buildchroot.inc               |   8 --
 15 files changed, 82 insertions(+), 220 deletions(-)

-- 
2.25.1

[PATCH v6 1/6] Revert "dpkg: Make mount buildroot reliable"

[Anton Mikanovich]

d21d49578e5a "dpkg: Make mount buildroot reliable" has broken API
compatibility with downstream layers: all functions using dpkg_do_mounts
and dpkg_undo_mounts had to be rewritten in Python.

This reverts commit d21d49578e5a3b0019075d1946bd93a95914fcca to retain
the possibility to call dpkg_do_mounts and dpkg_undo_mounts from shell
functions.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/dpkg-base.bbclass | 127 +++++++++++++--------------------
 meta/classes/dpkg-gbp.bbclass  |   8 ++-
 meta/classes/dpkg.bbclass      |  14 ++--
 3 files changed, 59 insertions(+), 90 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index ec8fbc1..4b189f1 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -61,7 +61,12 @@ addtask patch before do_adjust_git
 
 SRC_APT ?= ""
 
-fetch_apt() {
+do_apt_fetch() {
+    if [ -z "${@d.getVar("SRC_APT", True).strip()}" ]; then
+        return 0
+    fi
+    dpkg_do_mounts
+    E="${@ isar_export_proxies(d)}"
     sudo -E chroot ${BUILDCHROOT_DIR} /usr/bin/apt-get update \
         -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" \
         -o Dir::Etc::SourceParts="-" \
@@ -71,19 +76,8 @@ fetch_apt() {
         sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} \
             sh -c 'mkdir -p /downloads/deb-src/"$1"/"$2" && cd /downloads/deb-src/"$1"/"$2" && apt-get -y --download-only --only-source source "$2"' my_script "${DISTRO}" "${uri}"
     done
-}
-
-python do_apt_fetch() {
-    src_apt = d.getVar("SRC_APT", True)
-    if not src_apt:
-        return 0
 
-    dpkg_do_mounts(d)
-    try:
-        isar_export_proxies(d)
-        bb.build.exec_func("fetch_apt", d)
-    finally:
-        dpkg_undo_mounts(d)
+    dpkg_undo_mounts
 }
 
 addtask apt_fetch after do_unpack before do_apt_unpack
@@ -92,7 +86,14 @@ do_apt_fetch[lockfiles] += "${REPO_ISAR_DIR}/isar.lock"
 # Add dependency from the correct buildchroot: host or target
 do_apt_fetch[depends] = "${BUILDCHROOT_DEP}"
 
-unpack_apt() {
+do_apt_unpack() {
+    if [ -z "${@d.getVar("SRC_APT", True).strip()}" ]; then
+        return 0
+    fi
+    rm -rf ${S}
+    dpkg_do_mounts
+    E="${@ isar_export_proxies(d)}"
+
     for uri in "${SRC_APT}"; do
         sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} \
             sh -c ' \
@@ -103,25 +104,8 @@ unpack_apt() {
                 dpkg-source -x "${dscfile}" "${PPS}"' \
                     my_script "${DISTRO}" "${uri}"
     done
-}
 
-python do_apt_unpack() {
-    import shutil
-
-    src_apt = d.getVar("SRC_APT", True)
-    if not src_apt:
-        return 0
-
-    srcsubdir = d.getVar('S', True)
-    if os.path.exists(srcsubdir):
-        shutil.rmtree(srcsubdir)
-
-    dpkg_do_mounts(d)
-    try:
-        isar_export_proxies(d)
-        bb.build.exec_func("unpack_apt", d)
-    finally:
-        dpkg_undo_mounts(d)
+    dpkg_undo_mounts
 }
 
 addtask apt_unpack after do_apt_fetch before do_patch
@@ -165,38 +149,25 @@ do_prepare_build[deptask] = "do_deploy_deb"
 
 BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
 
-def ismount(path):
-    real = os.path.realpath(path)
-    with open('/proc/mounts') as f:
-        for line in f.readlines():
-            if len(line.split()) > 2 and real == line.split()[1]:
-                return True
-    return False
-
-def dpkg_do_mounts(d):
-    buildroot = d.getVar('BUILDROOT', True)
-    if ismount(buildroot):
-        bb.warn('Path %s already mounted!' % buildroot)
-        return
-    workdir = d.getVar('WORKDIR', True)
-    os.makedirs(buildroot, exist_ok=True)
-    os.system('sudo mount --bind %s %s' % (workdir, buildroot))
-    bb.build.exec_func("buildchroot_do_mounts", d)
-
-def dpkg_undo_mounts(d):
-    bb.build.exec_func("buildchroot_undo_mounts", d)
-    buildroot = d.getVar('BUILDROOT', True)
-    if not ismount(buildroot):
-        bb.warn('Path %s not mounted!' % buildroot)
-        return
-    for i in range(200):
-        if not os.system('sudo umount %s' % buildroot):
-            os.rmdir(buildroot)
-            return
-        if i % 100 == 0:
-            bb.warn("%s: Couldn't unmount, retrying..." % buildroot)
-        time.sleep(0.1)
-    bb.fatal("Couldn't unmount, exiting...")
+dpkg_do_mounts() {
+    mkdir -p ${BUILDROOT}
+    sudo mount --bind ${WORKDIR} ${BUILDROOT}
+
+    buildchroot_do_mounts
+}
+
+dpkg_undo_mounts() {
+    i=1
+    while ! sudo umount ${BUILDROOT}; do
+        sleep 0.1
+        i=`expr $i + 1`
+        if [ $i -gt 100 ]; then
+            bbwarn "${BUILDROOT}: Couldn't unmount, retrying..."
+            i=1
+        fi
+    done
+    sudo rmdir ${BUILDROOT}
+}
 
 # Placeholder for actual dpkg_runbuild() implementation
 dpkg_runbuild() {
@@ -206,12 +177,10 @@ dpkg_runbuild() {
 python do_dpkg_build() {
     lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock",
                              shared=True)
-    dpkg_do_mounts(d)
-    try:
-        bb.build.exec_func("dpkg_runbuild", d)
-    finally:
-        dpkg_undo_mounts(d)
-        bb.utils.unlockfile(lock)
+    bb.build.exec_func("dpkg_do_mounts", d)
+    bb.build.exec_func("dpkg_runbuild", d)
+    bb.build.exec_func("dpkg_undo_mounts", d)
+    bb.utils.unlockfile(lock)
 }
 
 addtask dpkg_build before do_build
@@ -255,16 +224,16 @@ python do_devshell() {
     oe_lib_path = os.path.join(d.getVar('LAYERDIR_core'), 'lib')
     sys.path.insert(0, oe_lib_path)
 
-    dpkg_do_mounts(d)
-    try:
-        isar_export_proxies(d)
+    bb.build.exec_func('dpkg_do_mounts', d)
+
+    isar_export_proxies(d)
+
+    buildchroot = d.getVar('BUILDCHROOT_DIR')
+    pp_pps = os.path.join(d.getVar('PP'), d.getVar('PPS'))
+    termcmd = "sudo -E chroot {0} sh -c 'cd {1}; $SHELL -i'"
+    oe_terminal(termcmd.format(buildchroot, pp_pps), "Isar devshell", d)
 
-        buildchroot = d.getVar('BUILDCHROOT_DIR')
-        pp_pps = os.path.join(d.getVar('PP'), d.getVar('PPS'))
-        termcmd = "sudo -E chroot {0} sh -c 'cd {1}; $SHELL -i'"
-        oe_terminal(termcmd.format(buildchroot, pp_pps), "Isar devshell", d)
-    finally:
-        dpkg_undo_mounts(d)
+    bb.build.exec_func('dpkg_undo_mounts', d)
 }
 
 addtask devshell after do_prepare_build
diff --git a/meta/classes/dpkg-gbp.bbclass b/meta/classes/dpkg-gbp.bbclass
index 20d2d4c..d956e8c 100644
--- a/meta/classes/dpkg-gbp.bbclass
+++ b/meta/classes/dpkg-gbp.bbclass
@@ -12,7 +12,12 @@ PATCHTOOL ?= "git"
 GBP_DEPENDS ?= "git-buildpackage pristine-tar"
 GBP_EXTRA_OPTIONS ?= "--git-pristine-tar"
 
-builddeps_install_append() {
+do_install_builddeps_append() {
+    dpkg_do_mounts
+    distro="${DISTRO}"
+    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
+       distro="${HOST_DISTRO}"
+    fi
     deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
     sudo -E chroot ${BUILDCHROOT_DIR} \
         apt-get install -y -o Debug::pkgProblemResolver=yes \
@@ -21,6 +26,7 @@ builddeps_install_append() {
     sudo -E chroot ${BUILDCHROOT_DIR} \
         apt-get install -y -o Debug::pkgProblemResolver=yes \
                         --no-install-recommends ${GBP_DEPENDS}
+    dpkg_undo_mounts
 }
 
 dpkg_runbuild_prepend() {
diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index 29e2b89..4e7c2f7 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -6,7 +6,9 @@ inherit dpkg-base
 PACKAGE_ARCH ?= "${DISTRO_ARCH}"
 
 # Install build dependencies for package
-builddeps_install() {
+do_install_builddeps() {
+    dpkg_do_mounts
+    E="${@ isar_export_proxies(d)}"
     distro="${DISTRO}"
     if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
        distro="${HOST_DISTRO}"
@@ -17,15 +19,7 @@ builddeps_install() {
     deb_dl_dir_export "${BUILDCHROOT_DIR}" "${distro}"
     sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
         ${PP}/${PPS} ${PACKAGE_ARCH}
-}
-
-python do_install_builddeps() {
-    dpkg_do_mounts(d)
-    isar_export_proxies(d)
-    try:
-        bb.build.exec_func("builddeps_install", d)
-    finally:
-        dpkg_undo_mounts(d)
+    dpkg_undo_mounts
 }
 
 addtask install_builddeps after do_prepare_build before do_dpkg_build
-- 
2.25.1

[PATCH v6 2/6] dpkg: Limit unmount loop

[Anton Mikanovich]

In the past, unmounting WORKDIR caused infinite looping with the
"Couldn't unmount, retrying..." warning if the first linux-mainline task
failed and the second one succeeded (it is built for de0-nano-soc-buster
and stm32mp15x-buster).

This change limits the loop to 1000 s.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/dpkg-base.bbclass | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 4b189f1..46b9700 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -157,14 +157,16 @@ dpkg_do_mounts() {
 }
 
 dpkg_undo_mounts() {
-    i=1
+    i=0
     while ! sudo umount ${BUILDROOT}; do
         sleep 0.1
-        i=`expr $i + 1`
-        if [ $i -gt 100 ]; then
-            bbwarn "${BUILDROOT}: Couldn't unmount, retrying..."
-            i=1
+        if [ `expr $i % 100` -eq 0 ] ; then
+            bbwarn "${BUILDROOT}: Couldn't unmount ($i), retrying..."
+        fi
+        if [ $i -ge 10000 ]; then
+            bbfatal "${BUILDROOT}: Couldn't unmount after timeout"
         fi
+        i=`expr $i + 1`
     done
     sudo rmdir ${BUILDROOT}
 }
-- 
2.25.1

[PATCH v6 3/6] dpkg-base: Clean up unmounting in do_dpkg_build()

[Anton Mikanovich]

Reverting d21d49578e5a "dpkg: Make mount buildroot reliable" brings back
two problems:

* The filesystems mounted under buildchroot (dev, proc, sys, etc.) are
  left mounted.

* If dpkg_runbuild() fails, WORKDIR and the filesystems under
  buildchroot are left mounted.

Request unmounting the filesystems under buildchroot after every package
build. The actual unmounting will be done after the last job due to the
reference counting.

Also request unmounting in any case, even if building fails.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/dpkg-base.bbclass | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 46b9700..7364a7f 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -169,6 +169,8 @@ dpkg_undo_mounts() {
         i=`expr $i + 1`
     done
     sudo rmdir ${BUILDROOT}
+
+    buildchroot_undo_mounts
 }
 
 # Placeholder for actual dpkg_runbuild() implementation
@@ -180,9 +182,11 @@ python do_dpkg_build() {
     lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock",
                              shared=True)
     bb.build.exec_func("dpkg_do_mounts", d)
-    bb.build.exec_func("dpkg_runbuild", d)
-    bb.build.exec_func("dpkg_undo_mounts", d)
-    bb.utils.unlockfile(lock)
+    try:
+        bb.build.exec_func("dpkg_runbuild", d)
+    finally:
+        bb.build.exec_func("dpkg_undo_mounts", d)
+        bb.utils.unlockfile(lock)
 }
 
 addtask dpkg_build before do_build
-- 
2.25.1

[PATCH v6 4/6] events: Do not warn on left mounts by default

[Anton Mikanovich]

Disable warnings printing for all the mount points left after build
completed to do not flood build logs too much in case build fail.

To enable those debug messages bitbake -D flag can be used.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/isar-events.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
index 2f8bf6e..92aff20 100644
--- a/meta/classes/isar-events.bbclass
+++ b/meta/classes/isar-events.bbclass
@@ -54,7 +54,7 @@ python build_completed() {
     with open('/proc/mounts') as f:
         for line in f.readlines():
             if basepath in line:
-                bb.warn('%s left mounted, unmounting...' % line.split()[1])
+                bb.debug(1, '%s left mounted, unmounting...' % line.split()[1])
                 subprocess.call(
                     ["sudo", "umount", "-l", line.split()[1]],
                     stdout=subprocess.DEVNULL,
-- 
2.25.1

[PATCH v6 5/6] Revert "rootfs: Unmount rootfs mounts if not needed"

[Anton Mikanovich]

Reference counter protected per-task resource unmounting can't yet be
used because some /dev and /sys mounts can stay still locked even after
task finish in CI environment on high jobs load.
This leads to the following error:

Execution of 'buildchroot-host/1.0-r0/temp/run.rootfs_undo_mounts.65491' failed with exit code 32:
umount: buildchroot-host/1.0-r0/rootfs/sys/fs/cgroup: target is busy.

We should revert commit e438c8f62b9aacb2c9d5120e93f81637f091a83c until
the reason of this issue will be found and fixed.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/image.bbclass     | 15 ++++++++
 meta/classes/initramfs.bbclass |  2 --
 meta/classes/rootfs.bbclass    | 63 ++++------------------------------
 3 files changed, 22 insertions(+), 58 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 12d1616..9f9b3f8 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -199,6 +199,21 @@ 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}/sys' && \
+            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"
diff --git a/meta/classes/initramfs.bbclass b/meta/classes/initramfs.bbclass
index e895afa..10a642b 100644
--- a/meta/classes/initramfs.bbclass
+++ b/meta/classes/initramfs.bbclass
@@ -38,7 +38,5 @@ do_generate_initramfs() {
 
     rm -rf "${INITRAMFS_IMAGE_FILE}"
     cp "${INITRAMFS_ROOTFS}/initrd.img" "${INITRAMFS_IMAGE_FILE}"
-
-    rootfs_undo_mounts
 }
 addtask generate_initramfs after do_rootfs before do_build
diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index 6ce2cf9..f9151c5 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -26,23 +26,9 @@ export LANG = "C"
 export LANGUAGE = "C"
 export LC_ALL = "C"
 
-IMAGE_MOUNT_LOCKFILE = "${ROOTFSDIR}.lock"
-
 rootfs_do_mounts[weight] = "3"
 rootfs_do_mounts() {
     sudo -s <<'EOSUDO'
-        ( flock 9
-        set -e
-
-        count="1"
-        if [ -f '${ROOTFSDIR}.mount' ]; then
-            count=$(($(cat '${ROOTFSDIR}.mount') + 1))
-        fi
-        echo $count > '${ROOTFSDIR}.mount'
-        if [ $count -gt 1 ]; then
-            exit 0
-        fi
-
         mountpoint -q '${ROOTFSDIR}/dev' || \
             mount --rbind /dev '${ROOTFSDIR}/dev'
         mount --make-rslave '${ROOTFSDIR}/dev'
@@ -70,40 +56,6 @@ rootfs_do_mounts() {
                 mount --bind '${REPO_BASE_DIR}' '${ROOTFSDIR}/base-apt'
         fi
 
-        ) 9>'${IMAGE_MOUNT_LOCKFILE}'
-EOSUDO
-}
-
-rootfs_undo_mounts() {
-    sudo -s <<'EOSUDO'
-        ( flock 9
-        set -e
-
-        if [ -f '${ROOTFSDIR}.mount' ]; then
-            count=$(($(cat '${ROOTFSDIR}.mount') - 1))
-            echo $count > '${ROOTFSDIR}.mount'
-        else
-            echo "Could not find mount counter"
-            exit 1
-        fi
-        if [ $count -gt 0 ]; then
-            exit 0
-        fi
-        rm ${ROOTFSDIR}.mount
-
-        mountpoint -q '${ROOTFSDIR}/base-apt' && \
-            umount ${ROOTFSDIR}/base-apt && \
-            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
-        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
-            umount ${ROOTFSDIR}/isar-apt && \
-            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
-        mountpoint -q '${ROOTFSDIR}/sys' && \
-            umount -R ${ROOTFSDIR}/sys
-        mountpoint -q '${ROOTFSDIR}/proc' && \
-            umount -R ${ROOTFSDIR}/proc
-        mountpoint -q '${ROOTFSDIR}/dev' && \
-            umount -R ${ROOTFSDIR}/dev
-        ) 9>'${IMAGE_MOUNT_LOCKFILE}'
 EOSUDO
 }
 
@@ -207,7 +159,7 @@ python do_rootfs_install() {
 
     # Mount after configure commands, so that they have time to copy
     # 'isar-apt' (sdkchroot):
-    cmds = ['rootfs_prepare'] + configure_cmds + ['rootfs_do_mounts'] + install_cmds  + ['rootfs_undo_mounts']
+    cmds = ['rootfs_prepare'] + configure_cmds + ['rootfs_do_mounts'] + install_cmds
 
     # NOTE: The weights specify how long each task takes in seconds and are used
     # by the MultiStageProgressReporter to render a progress bar for this task.
@@ -289,13 +241,12 @@ python do_rootfs_postprocess() {
     progress_reporter.update(0)
 
     cmds = d.getVar("ROOTFS_POSTPROCESS_COMMAND")
-    if cmds is not None and cmds.strip():
-        cmds = cmds.split()
-        for i, cmd in enumerate(cmds):
-            bb.build.exec_func(cmd, d)
-            progress_reporter.update(int(i / len(cmds) * 100))
-
-    bb.build.exec_func('rootfs_undo_mounts', d)
+    if cmds is None or not cmds.strip():
+        return
+    cmds = cmds.split()
+    for i, cmd in enumerate(cmds):
+        bb.build.exec_func(cmd, d)
+        progress_reporter.update(int(i / len(cmds) * 100))
 }
 addtask rootfs_postprocess before do_rootfs
 
-- 
2.25.1

[PATCH v6 6/6] Revert "buildchroot: Unmount buildchroot mounts if not needed"

[Anton Mikanovich]

Reference counter protected per-task resource unmounting can't yet be
used because some /dev and /sys mounts can stay still locked even after
task finish in CI environment on high jobs load.
This leads to the following error:

Execution of 'enable-fsck/1.0-r0/temp/run.buildchroot_undo_mounts.43890' failed with exit code 32:
umount: buildchroot-host/1.0-r0/rootfs/sys/fs/cgroup: target is busy.

We should revert commit 3360b7d9ff85172dc3d2275c35caec71149b35ef until
the reason of this issue will be found and fixed.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/buildchroot.bbclass              | 47 -------------------
 meta/classes/dpkg-base.bbclass                |  2 -
 meta/classes/ext4-img.bbclass                 |  2 -
 meta/classes/fit-img.bbclass                  |  2 -
 meta/classes/image-tools-extension.bbclass    |  2 -
 meta/classes/image.bbclass                    |  9 ----
 meta/classes/ubi-img.bbclass                  |  2 -
 meta/classes/ubifs-img.bbclass                |  2 -
 meta/classes/wic-img.bbclass                  |  1 -
 .../buildchroot/buildchroot.inc               |  8 ----
 10 files changed, 77 deletions(-)

diff --git a/meta/classes/buildchroot.bbclass b/meta/classes/buildchroot.bbclass
index f77e4f0..e9eb9af 100644
--- a/meta/classes/buildchroot.bbclass
+++ b/meta/classes/buildchroot.bbclass
@@ -28,19 +28,8 @@ buildchroot_do_mounts() {
         ( flock 9
         set -e
 
-        count="1"
-        if [ -f '${BUILDCHROOT_DIR}.mount' ]; then
-            count=$(($(cat '${BUILDCHROOT_DIR}.mount') + 1))
-        fi
-        echo $count > '${BUILDCHROOT_DIR}.mount'
-        if [ $count -gt 1 ]; then
-            exit 0
-        fi
-
-        mkdir -p '${BUILDCHROOT_DIR}/isar-apt'
         mountpoint -q '${BUILDCHROOT_DIR}/isar-apt' ||
             mount --bind '${REPO_ISAR_DIR}/${DISTRO}' '${BUILDCHROOT_DIR}/isar-apt'
-        mkdir -p '${BUILDCHROOT_DIR}/downloads'
         mountpoint -q '${BUILDCHROOT_DIR}/downloads' ||
             mount --bind '${DL_DIR}' '${BUILDCHROOT_DIR}/downloads'
         mountpoint -q '${BUILDCHROOT_DIR}/dev' ||
@@ -71,39 +60,3 @@ buildchroot_do_mounts() {
         ) 9>'${MOUNT_LOCKFILE}'
 EOSUDO
 }
-
-buildchroot_undo_mounts() {
-    sudo -s <<'EOSUDO'
-        ( flock 9
-        set -e
-
-        if [ -f '${BUILDCHROOT_DIR}.mount' ]; then
-            count=$(($(cat '${BUILDCHROOT_DIR}.mount') - 1))
-            echo $count > '${BUILDCHROOT_DIR}.mount'
-        else
-            echo "Could not find mount counter"
-            exit 1
-        fi
-        if [ $count -gt 0 ]; then
-            exit 0
-        fi
-        rm ${BUILDCHROOT_DIR}.mount
-
-        mountpoint -q '${BUILDCHROOT_DIR}/base-apt' && \
-            umount ${BUILDCHROOT_DIR}/base-apt && \
-            rmdir --ignore-fail-on-non-empty ${BUILDCHROOT_DIR}/base-apt
-        mountpoint -q '${BUILDCHROOT_DIR}/sys' && \
-            umount -R ${BUILDCHROOT_DIR}/sys
-        mountpoint -q '${BUILDCHROOT_DIR}/proc' && \
-            umount -R ${BUILDCHROOT_DIR}/proc
-        mountpoint -q '${BUILDCHROOT_DIR}/dev' && \
-            umount -R ${BUILDCHROOT_DIR}/dev
-        mountpoint -q '${BUILDCHROOT_DIR}/downloads' && \
-            umount ${BUILDCHROOT_DIR}/downloads && \
-            rmdir --ignore-fail-on-non-empty ${BUILDCHROOT_DIR}/downloads
-        mountpoint -q '${BUILDCHROOT_DIR}/isar-apt' && \
-            umount ${BUILDCHROOT_DIR}/isar-apt && \
-            rmdir --ignore-fail-on-non-empty ${BUILDCHROOT_DIR}/isar-apt
-        ) 9>'${MOUNT_LOCKFILE}'
-EOSUDO
-}
diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 7364a7f..aa529ca 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -169,8 +169,6 @@ dpkg_undo_mounts() {
         i=`expr $i + 1`
     done
     sudo rmdir ${BUILDROOT}
-
-    buildchroot_undo_mounts
 }
 
 # Placeholder for actual dpkg_runbuild() implementation
diff --git a/meta/classes/ext4-img.bbclass b/meta/classes/ext4-img.bbclass
index 26e0137..334dc64 100644
--- a/meta/classes/ext4-img.bbclass
+++ b/meta/classes/ext4-img.bbclass
@@ -17,8 +17,6 @@ do_ext4_image() {
 
     sudo chroot ${BUILDCHROOT_DIR} /sbin/mke2fs ${MKE2FS_ARGS} \
                 -F -d '${PP_ROOTFS}' '${PP_DEPLOY}/${EXT4_IMAGE_FILE}'
-
-    image_undo_mounts
 }
 
 addtask ext4_image before do_image after do_image_tools
diff --git a/meta/classes/fit-img.bbclass b/meta/classes/fit-img.bbclass
index 98ed8eb..82b96d8 100644
--- a/meta/classes/fit-img.bbclass
+++ b/meta/classes/fit-img.bbclass
@@ -24,7 +24,5 @@ do_fit_image() {
     sudo chroot ${BUILDCHROOT_DIR} /usr/bin/mkimage ${MKIMAGE_ARGS} \
                 -f '${PP_WORK}/${FIT_IMAGE_SOURCE}' '${PP_DEPLOY}/${FIT_IMAGE_FILE}'
     sudo chown $(id -u):$(id -g) '${DEPLOY_DIR_IMAGE}/${FIT_IMAGE_FILE}'
-
-    image_undo_mounts
 }
 addtask fit_image before do_image after do_image_tools do_transform_template
diff --git a/meta/classes/image-tools-extension.bbclass b/meta/classes/image-tools-extension.bbclass
index 9646a14..9f28800 100644
--- a/meta/classes/image-tools-extension.bbclass
+++ b/meta/classes/image-tools-extension.bbclass
@@ -40,7 +40,5 @@ do_install_imager_deps() {
         apt-get -o Debug::pkgProblemResolver=yes --no-install-recommends -y \
             --allow-unauthenticated --allow-downgrades install \
             ${IMAGER_INSTALL}'
-
-    buildchroot_undo_mounts
 }
 addtask install_imager_deps before do_image_tools
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 9f9b3f8..ec93cab 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -62,15 +62,6 @@ image_do_mounts() {
     buildchroot_do_mounts
 }
 
-image_undo_mounts() {
-    buildchroot_undo_mounts
-    sudo flock ${MOUNT_LOCKFILE} -c ' \
-        umount "${BUILDROOT_DEPLOY}"
-        umount "${BUILDROOT_ROOTFS}"
-        umount "${BUILDROOT_WORK}"
-    '
-}
-
 ROOTFSDIR = "${IMAGE_ROOTFS}"
 ROOTFS_FEATURES += "clean-package-cache generate-manifest export-dpkg-status"
 ROOTFS_PACKAGES += "${IMAGE_PREINSTALL} ${IMAGE_INSTALL}"
diff --git a/meta/classes/ubi-img.bbclass b/meta/classes/ubi-img.bbclass
index 5ce7686..c69ac4d 100644
--- a/meta/classes/ubi-img.bbclass
+++ b/meta/classes/ubi-img.bbclass
@@ -27,7 +27,5 @@ do_ubi_image() {
     sudo chroot ${BUILDCHROOT_DIR} /usr/sbin/ubinize ${UBINIZE_ARGS} \
                 -o '${PP_DEPLOY}/${UBI_IMAGE_FILE}' '${PP_WORK}/${UBINIZE_CFG}'
     sudo chown $(id -u):$(id -g) '${DEPLOY_DIR_IMAGE}/${UBI_IMAGE_FILE}'
-
-    image_undo_mounts
 }
 addtask ubi_image before do_image after do_image_tools do_transform_template
diff --git a/meta/classes/ubifs-img.bbclass b/meta/classes/ubifs-img.bbclass
index ea06561..fcbfbbc 100644
--- a/meta/classes/ubifs-img.bbclass
+++ b/meta/classes/ubifs-img.bbclass
@@ -22,8 +22,6 @@ do_ubifs_image() {
     sudo chroot ${BUILDCHROOT_DIR} /usr/sbin/mkfs.ubifs ${MKUBIFS_ARGS} \
                 -r '${PP_ROOTFS}' '${PP_DEPLOY}/${UBIFS_IMAGE_FILE}'
     sudo chown $(id -u):$(id -g) '${DEPLOY_DIR_IMAGE}/${UBIFS_IMAGE_FILE}'
-
-    image_undo_mounts
 }
 
 addtask ubifs_image before do_image after do_image_tools
diff --git a/meta/classes/wic-img.bbclass b/meta/classes/wic-img.bbclass
index f641312..170b83e 100644
--- a/meta/classes/wic-img.bbclass
+++ b/meta/classes/wic-img.bbclass
@@ -209,5 +209,4 @@ EOSUDO
         done
         ) 9>${MOUNT_LOCKFILE}
 EOSUDO
-    buildchroot_undo_mounts
 }
diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
index 0c1fb7f..5a2befb 100644
--- a/meta/recipes-devtools/buildchroot/buildchroot.inc
+++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
@@ -49,14 +49,6 @@ rootfs_do_mounts_append() {
 EOSUDO
 }
 
-rootfs_undo_mounts_append() {
-    sudo -s <<'EOSUDO'
-    mountpoint -q '${BUILDCHROOT_DIR}/downloads' && \
-        umount ${BUILDCHROOT_DIR}/downloads && \
-        rmdir --ignore-fail-on-non-empty ${BUILDCHROOT_DIR}/downloads
-EOSUDO
-}
-
 ROOTFS_POSTPROCESS_COMMAND =+ "buildchroot_install_files"
 buildchroot_install_files() {
     sudo mkdir -p "${BUILDCHROOT_DIR}/home/builder"
-- 
2.25.1

Re: [PATCH v6 0/6] Restore downstream mounts compatibility

[Anton Mikanovich]

23.08.2021 13:23, Anton Mikanovich wrote:
> Revert commit d21d49578e5a3b0019075d1946bd93a95914fcca which has broken
> compatibility with downstream projects. Move try-finally from
> dpkg_runbuild(), etc. to the caller, do_dpkg_build().
>
> Also revert e438c8f62b9aacb2c9d5120e93f81637f091a83c and
> 3360b7d9ff85172dc3d2275c35caec71149b35ef commits to prevent 'umount:
> target is busy' issues caused by /dev and /sys mounts stayed locked
> even after task finish in some cases. Those two commits should be
> applied back after this issue is found and fixed.
>
> Move mounts left printing from warnings to debug output bacause
> per-task unmounting was reverted.
>
> Tested with kas-iot2050-example project from:
> https://github.com/siemens/meta-iot2050

If no objection will merge this on Monday.

-- 
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov

Re: [PATCH v6 0/6] Restore downstream mounts compatibility

[Jan Kiszka]

On 27.08.21 19:16, Anton Mikanovich wrote:
> 23.08.2021 13:23, Anton Mikanovich wrote:
>> Revert commit d21d49578e5a3b0019075d1946bd93a95914fcca which has broken
>> compatibility with downstream projects. Move try-finally from
>> dpkg_runbuild(), etc. to the caller, do_dpkg_build().
>>
>> Also revert e438c8f62b9aacb2c9d5120e93f81637f091a83c and
>> 3360b7d9ff85172dc3d2275c35caec71149b35ef commits to prevent 'umount:
>> target is busy' issues caused by /dev and /sys mounts stayed locked
>> even after task finish in some cases. Those two commits should be
>> applied back after this issue is found and fixed.
>>
>> Move mounts left printing from warnings to debug output bacause
>> per-task unmounting was reverted.
>>
>> Tested with kas-iot2050-example project from:
>> https://github.com/siemens/meta-iot2050
> 
> If no objection will merge this on Monday.
> 

>From what I saw this looks fine - but I have trouble here receiving all
emails from this list in the recent weeks. Likely Siemens-specific issues.

Thanks,
Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH v6 0/6] Restore downstream mounts compatibility

[Anton Mikanovich]

23.08.2021 13:23, Anton Mikanovich wrote:
> Revert commit d21d49578e5a3b0019075d1946bd93a95914fcca which has broken
> compatibility with downstream projects. Move try-finally from
> dpkg_runbuild(), etc. to the caller, do_dpkg_build().
>
> Also revert e438c8f62b9aacb2c9d5120e93f81637f091a83c and
> 3360b7d9ff85172dc3d2275c35caec71149b35ef commits to prevent 'umount:
> target is busy' issues caused by /dev and /sys mounts stayed locked
> even after task finish in some cases. Those two commits should be
> applied back after this issue is found and fixed.
>
> Move mounts left printing from warnings to debug output bacause
> per-task unmounting was reverted.
>
> Tested with kas-iot2050-example project from:
> https://github.com/siemens/meta-iot2050

Applied to next.

-- 
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov

Re: [PATCH v6 0/6] Restore downstream mounts compatibility

[Henning Schild]

Is that what was recently applied to next?

We are still having issues some issues with current next and are trying
to understand things.

One thing i did find is that next contains
57b3dc055837 ("Revert "rootfs: Unmount rootfs mounts if not needed"")

which reverts
e438c8f62b9a ("rootfs: Unmount rootfs mounts if not needed")

this guy
5e563ac4b707 ("rootfs: Use separate mounts lock file")

was a fix on top of e438c8f62b9a and now also potentially deserves a
revert.

Henning


Am Mon, 23 Aug 2021 13:23:36 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> Revert commit d21d49578e5a3b0019075d1946bd93a95914fcca which has
> broken compatibility with downstream projects. Move try-finally from
> dpkg_runbuild(), etc. to the caller, do_dpkg_build().
> 
> Also revert e438c8f62b9aacb2c9d5120e93f81637f091a83c and
> 3360b7d9ff85172dc3d2275c35caec71149b35ef commits to prevent 'umount:
> target is busy' issues caused by /dev and /sys mounts stayed locked
> even after task finish in some cases. Those two commits should be
> applied back after this issue is found and fixed.
> 
> Move mounts left printing from warnings to debug output bacause
> per-task unmounting was reverted.
> 
> Tested with kas-iot2050-example project from:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsiemens%2Fmeta-iot2050&amp;data=04%7C01%7Cde173c00-e982-4fda-8644-47edf4671d63%40ad011.siemens.com%7C5185795a8ce7459026e408d966201e24%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637653110414149256%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=3zx9%2Bfri6TR4jMdA7hZvvvmmuYYMtJoowPgRhv3peAg%3D&amp;reserved=0
> 
> ---
> Changes sinse v5:
> - Revert buildchroot per-task unmounts.
> - Revert rootfs per-task unmounts.
> - Remove imager per-task unmounts.
> Changes sinse v4:
> - Fix while loop counts.
> - Move unmount warnings under debug.
> Changes since v3:
> - Warn on first entering in umount while loop.
> - Do not warn on left mounts by default.
> - Remote incorrect dpkg unmount usage protection.
> - Rebase on next.
> - Improve commit messages.
> Changes since v2:
> - Remove incorrect buildchroot/rootfs mount/unmount usage protection.
> - Remove task fail handler.
> - Implement double mount protection by try-finally.
> Changes since v1:
> - Get back while loop in dpkg_undo_mounts.
> - Exit dpkg_undo_mounts if already mounted.
> - Do not ignore exceptions in safe_exec.
> 
> Anton Mikanovich (6):
>   Revert "dpkg: Make mount buildroot reliable"
>   dpkg: Limit unmount loop
>   dpkg-base: Clean up unmounting in do_dpkg_build()
>   events: Do not warn on left mounts by default
>   Revert "rootfs: Unmount rootfs mounts if not needed"
>   Revert "buildchroot: Unmount buildchroot mounts if not needed"
> 
>  meta/classes/buildchroot.bbclass              |  47 -------
>  meta/classes/dpkg-base.bbclass                | 123
> +++++++----------- meta/classes/dpkg-gbp.bbclass                 |
> 8 +- meta/classes/dpkg.bbclass                     |  14 +-
>  meta/classes/ext4-img.bbclass                 |   2 -
>  meta/classes/fit-img.bbclass                  |   2 -
>  meta/classes/image-tools-extension.bbclass    |   2 -
>  meta/classes/image.bbclass                    |  24 ++--
>  meta/classes/initramfs.bbclass                |   2 -
>  meta/classes/isar-events.bbclass              |   2 +-
>  meta/classes/rootfs.bbclass                   |  63 +--------
>  meta/classes/ubi-img.bbclass                  |   2 -
>  meta/classes/ubifs-img.bbclass                |   2 -
>  meta/classes/wic-img.bbclass                  |   1 -
>  .../buildchroot/buildchroot.inc               |   8 --
>  15 files changed, 82 insertions(+), 220 deletions(-)
> 

Re: [PATCH v6 0/6] Restore downstream mounts compatibility

[Anton Mikanovich]

08.09.2021 18:48, Henning Schild wrote:
> Is that what was recently applied to next?
>
> We are still having issues some issues with current next and are trying
> to understand things.
>
> One thing i did find is that next contains
> 57b3dc055837 ("Revert "rootfs: Unmount rootfs mounts if not needed"")
>
> which reverts
> e438c8f62b9a ("rootfs: Unmount rootfs mounts if not needed")
>
> this guy
> 5e563ac4b707 ("rootfs: Use separate mounts lock file")
>
> was a fix on top of e438c8f62b9a and now also potentially deserves a
> revert.
>
> Henning

Yes, that was already applied to next.
What kind of issues do you have, are there any logs?

The lockfile changed by 5e563ac4b707 was gone with revert, because it 
was introduced by e438c8f62b9a.

-- 
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov

Re: [PATCH v6 0/6] Restore downstream mounts compatibility

[Henning Schild]

Am Thu, 9 Sep 2021 11:13:37 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 08.09.2021 18:48, Henning Schild wrote:
> > Is that what was recently applied to next?
> >
> > We are still having issues some issues with current next and are
> > trying to understand things.
> >
> > One thing i did find is that next contains
> > 57b3dc055837 ("Revert "rootfs: Unmount rootfs mounts if not
> > needed"")
> >
> > which reverts
> > e438c8f62b9a ("rootfs: Unmount rootfs mounts if not needed")
> >
> > this guy
> > 5e563ac4b707 ("rootfs: Use separate mounts lock file")
> >
> > was a fix on top of e438c8f62b9a and now also potentially deserves a
> > revert.
> >
> > Henning  
> 
> Yes, that was already applied to next.
> What kind of issues do you have, are there any logs?

I just wrote an email on that. Some builds are getting stuck and it
seems that ROOTFS/isar.lock is part of the problem. The layer does have
a few recipes that use buildchroot_do_mounts but nothing with locking.
Investigation is ongoing, but if you know the problem that builds get
stuck at do_deploy_deb that would be interesting to us.

> The lockfile changed by 5e563ac4b707 was gone with revert, because it 
> was introduced by e438c8f62b9a.

I see 57b3dc05583743781cad3b46bec915a601d2fbe2 in fact reverts multiple
commits.

It would be helpful to revert single commits in reverse order and _not_
remove the part of the commit message that contains the sha of the
reverted commit.

Henning

Re: [PATCH v6 0/6] Restore downstream mounts compatibility

[Anton Mikanovich]

09.09.2021 12:17, Henning Schild wrote:
>
> I just wrote an email on that. Some builds are getting stuck and it
> seems that ROOTFS/isar.lock is part of the problem. The layer does have
> a few recipes that use buildchroot_do_mounts but nothing with locking.
> Investigation is ongoing, but if you know the problem that builds get
> stuck at do_deploy_deb that would be interesting to us.
According to that email isar/isar.lock file is the one which was stuck, 
but not ROOTFS/isar.lock.
So this is not a lockfile used for mounts control.

I've analyzed 3 different stuck builds (54664183, 54679509 and 54679508) 
and see that all three are supposed to stuck on different tasks 
(do_dpkg_build, do_install_builddeps and do_apt_fetch) of three 
different recipes (can share the lists of those recipes if needed). 
There were no CI artifacts saved, so it is hard to say what's going on 
inside.
Suggest looking into appropriate logfile when this happens again until 
the build fail by timeout.

-- 

Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov