[PATCH v3 0/5] Rebuild mount logic

[PATCH v3 0/5] Rebuild mount logic

[Anton Mikanovich]

Mounting is perfomed very inconsistent currently in Isar. A lot of mount
points are left after task execution and stays mounted until cleanup in
build_completed hook.

We already experienced issues with stuck forever on unmounting at
"Couldn't unmount, retrying..." error.
It is also quite easy to face with a lot of ramaining mounts after the
failed or interrupted manual builds.

This patchset implements unmounting paths after usage. In the cases of
mount sharing corresponding request counting is performed, so real mount
will be done only at first request, real unmount will be done only after
the last task will try to unmount the path.

Meantime build_completed will use emergency only, so following warning
added to show something goes wrong.

---
Changes since v2:
- Prevent race conditions.
- Fix building qemuarm64-stretch.
- Minimize locks.
- Cleanup counter files.
- Unmount /proc/sys/fs/binfmt_misc also.
Changes since v1:
- Make all mounts be unmounted after use.
- Warn about lost mounts after finish.

Anton Mikanovich (5):
  dpkg: Make mount buildroot reliable
  buildchroot: Unmount buildchroot mounts if not needed
  rootfs: Unmount rootfs mounts if not needed
  wic: Unmount dirs after usage
  events: Warn if mounted paths left

 meta/classes/buildchroot.bbclass              |  47 +++++++
 meta/classes/dpkg-base.bbclass                | 127 +++++++++++-------
 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              |   1 +
 meta/classes/rootfs.bbclass                   |  62 ++++++++-
 meta/classes/ubi-img.bbclass                  |   2 +
 meta/classes/ubifs-img.bbclass                |   2 +
 meta/classes/wic-img.bbclass                  |  10 ++
 .../buildchroot/buildchroot.inc               |   8 ++
 15 files changed, 232 insertions(+), 81 deletions(-)

-- 
2.25.1

[PATCH v3 1/5] dpkg: Make mount buildroot reliable

[Anton Mikanovich]

We use mounting for several tasks. Mounting and unmounting is performed
in shell scripts. If a command fails before unmounting, the directories
remain mounted.

Implement exception handling around the scripts to ensure that
unmounting is performed also in case of script failure. The number of
unmounting attempts has been limited to avoid infinite loops.

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

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 97661a6..d5e70f1 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -61,12 +61,7 @@ addtask patch before do_adjust_git
 
 SRC_APT ?= ""
 
-do_apt_fetch() {
-    if [ -z "${@d.getVar("SRC_APT", True).strip()}" ]; then
-        return 0
-    fi
-    dpkg_do_mounts
-    E="${@ isar_export_proxies(d)}"
+fetch_apt() {
     sudo -E chroot ${BUILDCHROOT_DIR} /usr/bin/apt-get update \
         -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" \
         -o Dir::Etc::SourceParts="-" \
@@ -76,21 +71,25 @@ do_apt_fetch() {
         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_undo_mounts
+    dpkg_do_mounts(d)
+    try:
+        isar_export_proxies(d)
+        bb.build.exec_func("fetch_apt", d)
+    finally:
+        dpkg_undo_mounts(d)
 }
 
 addtask apt_fetch after do_unpack before do_apt_unpack
 do_apt_fetch[lockfiles] += "${REPO_ISAR_DIR}/isar.lock"
 
-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)}"
-
+unpack_apt() {
     for uri in "${SRC_APT}"; do
         sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} \
             sh -c ' \
@@ -101,8 +100,25 @@ do_apt_unpack() {
                 dpkg-source -x "${dscfile}" "${PPS}"' \
                     my_script "${DISTRO}" "${uri}"
     done
+}
 
-    dpkg_undo_mounts
+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)
 }
 
 addtask apt_unpack after do_apt_fetch before do_patch
@@ -146,25 +162,37 @@ do_prepare_build[deptask] = "do_deploy_deb"
 
 BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
 
-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}
-}
+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):
+    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...")
 
 # Placeholder for actual dpkg_runbuild() implementation
 dpkg_runbuild() {
@@ -174,10 +202,12 @@ dpkg_runbuild() {
 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)
+    dpkg_do_mounts(d)
+    try:
+        bb.build.exec_func("dpkg_runbuild", d)
+    finally:
+        dpkg_undo_mounts(d)
+        bb.utils.unlockfile(lock)
 }
 
 addtask dpkg_build before do_build
@@ -221,16 +251,16 @@ python do_devshell() {
     oe_lib_path = os.path.join(d.getVar('LAYERDIR_core'), 'lib')
     sys.path.insert(0, oe_lib_path)
 
-    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)
+    dpkg_do_mounts(d)
+    try:
+        isar_export_proxies(d)
 
-    bb.build.exec_func('dpkg_undo_mounts', 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)
 }
 
 addtask devshell after do_prepare_build
diff --git a/meta/classes/dpkg-gbp.bbclass b/meta/classes/dpkg-gbp.bbclass
index d956e8c..20d2d4c 100644
--- a/meta/classes/dpkg-gbp.bbclass
+++ b/meta/classes/dpkg-gbp.bbclass
@@ -12,12 +12,7 @@ PATCHTOOL ?= "git"
 GBP_DEPENDS ?= "git-buildpackage pristine-tar"
 GBP_EXTRA_OPTIONS ?= "--git-pristine-tar"
 
-do_install_builddeps_append() {
-    dpkg_do_mounts
-    distro="${DISTRO}"
-    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
-       distro="${HOST_DISTRO}"
-    fi
+builddeps_install_append() {
     deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
     sudo -E chroot ${BUILDCHROOT_DIR} \
         apt-get install -y -o Debug::pkgProblemResolver=yes \
@@ -26,7 +21,6 @@ do_install_builddeps_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 4e7c2f7..29e2b89 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -6,9 +6,7 @@ inherit dpkg-base
 PACKAGE_ARCH ?= "${DISTRO_ARCH}"
 
 # Install build dependencies for package
-do_install_builddeps() {
-    dpkg_do_mounts
-    E="${@ isar_export_proxies(d)}"
+builddeps_install() {
     distro="${DISTRO}"
     if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
        distro="${HOST_DISTRO}"
@@ -19,7 +17,15 @@ do_install_builddeps() {
     deb_dl_dir_export "${BUILDCHROOT_DIR}" "${distro}"
     sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
         ${PP}/${PPS} ${PACKAGE_ARCH}
-    dpkg_undo_mounts
+}
+
+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)
 }
 
 addtask install_builddeps after do_prepare_build before do_dpkg_build
-- 
2.25.1

[PATCH v3 2/5] buildchroot: Unmount buildchroot mounts if not needed

[Anton Mikanovich]

Count the usage of BUILDCHROOT_DIR mounts and trigger unmount of them
after all tasks using chroot are finished.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/buildchroot.bbclass              | 47 +++++++++++++++++++
 meta/classes/dpkg-base.bbclass                |  1 +
 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, 76 insertions(+)

diff --git a/meta/classes/buildchroot.bbclass b/meta/classes/buildchroot.bbclass
index 1a8ee13..97d5301 100644
--- a/meta/classes/buildchroot.bbclass
+++ b/meta/classes/buildchroot.bbclass
@@ -27,8 +27,20 @@ buildchroot_do_mounts() {
     sudo -s <<'EOSUDO'
         ( flock 9
         set -e
+
+        count="1"
+        if [ -f '${BUILDCHROOT_DIR}.mount' ]; then
+            count=$(($(< '${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' ||
@@ -59,3 +71,38 @@ buildchroot_do_mounts() {
         ) 9>'${MOUNT_LOCKFILE}'
 EOSUDO
 }
+
+buildchroot_undo_mounts() {
+    sudo -s <<'EOSUDO'
+        ( flock 9
+        set -e
+
+        if [ -f '${BUILDCHROOT_DIR}.mount' ]; then
+            count=$(($(< '${BUILDCHROOT_DIR}.mount') - 1))
+            echo $count > '${BUILDCHROOT_DIR}.mount'
+        else
+            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 d5e70f1..ed162b3 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -181,6 +181,7 @@ def dpkg_do_mounts(d):
     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)
diff --git a/meta/classes/ext4-img.bbclass b/meta/classes/ext4-img.bbclass
index 334dc64..26e0137 100644
--- a/meta/classes/ext4-img.bbclass
+++ b/meta/classes/ext4-img.bbclass
@@ -17,6 +17,8 @@ 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 221ac3f..da8e1da 100644
--- a/meta/classes/fit-img.bbclass
+++ b/meta/classes/fit-img.bbclass
@@ -23,5 +23,7 @@ do_fit_image() {
     # Create fit image using buildchroot tools
     sudo chroot ${BUILDCHROOT_DIR} /usr/bin/mkimage ${MKIMAGE_ARGS} \
                 -f '${PP_WORK}/${FIT_IMAGE_SOURCE}' '${PP_DEPLOY}/${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 0b067ff..4738479 100644
--- a/meta/classes/image-tools-extension.bbclass
+++ b/meta/classes/image-tools-extension.bbclass
@@ -40,5 +40,7 @@ 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 ec93cab..9f9b3f8 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -62,6 +62,15 @@ 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 2178b50..0f4c4cb 100644
--- a/meta/classes/ubi-img.bbclass
+++ b/meta/classes/ubi-img.bbclass
@@ -26,5 +26,7 @@ do_ubi_image() {
     # Create ubi image using buildchroot tools
     sudo chroot ${BUILDCHROOT_DIR} /usr/sbin/ubinize ${UBINIZE_ARGS} \
                 -o '${PP_DEPLOY}/${UBI_IMAGE_FILE}' '${PP_WORK}/${UBINIZE_CFG}'
+
+    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 78926f6..e422b46 100644
--- a/meta/classes/ubifs-img.bbclass
+++ b/meta/classes/ubifs-img.bbclass
@@ -21,6 +21,8 @@ do_ubifs_image() {
     # Create ubifs image using buildchroot tools
     sudo chroot ${BUILDCHROOT_DIR} /usr/sbin/mkfs.ubifs ${MKUBIFS_ARGS} \
                 -r '${PP_ROOTFS}' '${PP_DEPLOY}/${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 042a546..bb3a3ee 100644
--- a/meta/classes/wic-img.bbclass
+++ b/meta/classes/wic-img.bbclass
@@ -200,4 +200,5 @@ EOSUDO
     done
     rm -rf ${BUILDCHROOT_DIR}/${WICTMP}
     rm -rf ${IMAGE_ROOTFS}/../pseudo
+    buildchroot_undo_mounts
 }
diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
index 5a2befb..0c1fb7f 100644
--- a/meta/recipes-devtools/buildchroot/buildchroot.inc
+++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
@@ -49,6 +49,14 @@ 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

[PATCH v3 3/5] rootfs: Unmount rootfs mounts if not needed

[Anton Mikanovich]

Count the usage of rootfs mounts and trigger unmount only after all
corresponded tasks are finished.

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

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 9f9b3f8..12d1616 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -199,21 +199,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}/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 10a642b..e895afa 100644
--- a/meta/classes/initramfs.bbclass
+++ b/meta/classes/initramfs.bbclass
@@ -38,5 +38,7 @@ 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 f9151c5..0458c7c 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -26,9 +26,23 @@ export LANG = "C"
 export LANGUAGE = "C"
 export LC_ALL = "C"
 
+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=$(($(< '${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'
@@ -56,6 +70,39 @@ rootfs_do_mounts() {
                 mount --bind '${REPO_BASE_DIR}' '${ROOTFSDIR}/base-apt'
         fi
 
+        ) 9>'${MOUNT_LOCKFILE}'
+EOSUDO
+}
+
+rootfs_undo_mounts() {
+    sudo -s <<'EOSUDO'
+        ( flock 9
+        set -e
+
+        if [ -f '${ROOTFSDIR}.mount' ]; then
+            count=$(($(< '${ROOTFSDIR}.mount') - 1))
+            echo $count > '${ROOTFSDIR}.mount'
+        else
+            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>'${MOUNT_LOCKFILE}'
 EOSUDO
 }
 
@@ -159,7 +206,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
+    cmds = ['rootfs_prepare'] + configure_cmds + ['rootfs_do_mounts'] + install_cmds  + ['rootfs_undo_mounts']
 
     # 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.
@@ -241,12 +288,13 @@ python do_rootfs_postprocess() {
     progress_reporter.update(0)
 
     cmds = d.getVar("ROOTFS_POSTPROCESS_COMMAND")
-    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))
+    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)
 }
 addtask rootfs_postprocess before do_rootfs
 
-- 
2.25.1

[PATCH v3 4/5] wic: Unmount dirs after usage

[Anton Mikanovich]

Unmount dirs mounted at generate_wic_image task after usage.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/wic-img.bbclass | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/meta/classes/wic-img.bbclass b/meta/classes/wic-img.bbclass
index bb3a3ee..f641312 100644
--- a/meta/classes/wic-img.bbclass
+++ b/meta/classes/wic-img.bbclass
@@ -200,5 +200,14 @@ 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
     buildchroot_undo_mounts
 }
-- 
2.25.1

[PATCH v3 5/5] events: Warn if mounted paths left

[Anton Mikanovich]

Output warnings in case any mounts are left after build.

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

diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
index 4c49635..73419b4 100644
--- a/meta/classes/isar-events.bbclass
+++ b/meta/classes/isar-events.bbclass
@@ -52,6 +52,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])
                 subprocess.call(
                     ["sudo", "umount", "-l", line.split()[1]],
                     stdout=subprocess.DEVNULL,
-- 
2.25.1

Re: [PATCH v3 0/5] Rebuild mount logic

[Henning Schild]

This series makes things not build in containers anymore, which we
heavily rely on for CI. Exceptions in the mount logic.

I would suggest to revert and investigate.

Henning

Am Sat,  8 May 2021 09:25:13 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> Mounting is perfomed very inconsistent currently in Isar. A lot of
> mount points are left after task execution and stays mounted until
> cleanup in build_completed hook.
> 
> We already experienced issues with stuck forever on unmounting at
> "Couldn't unmount, retrying..." error.
> It is also quite easy to face with a lot of ramaining mounts after the
> failed or interrupted manual builds.
> 
> This patchset implements unmounting paths after usage. In the cases of
> mount sharing corresponding request counting is performed, so real
> mount will be done only at first request, real unmount will be done
> only after the last task will try to unmount the path.
> 
> Meantime build_completed will use emergency only, so following warning
> added to show something goes wrong.
> 
> ---
> Changes since v2:
> - Prevent race conditions.
> - Fix building qemuarm64-stretch.
> - Minimize locks.
> - Cleanup counter files.
> - Unmount /proc/sys/fs/binfmt_misc also.
> Changes since v1:
> - Make all mounts be unmounted after use.
> - Warn about lost mounts after finish.
> 
> Anton Mikanovich (5):
>   dpkg: Make mount buildroot reliable
>   buildchroot: Unmount buildchroot mounts if not needed
>   rootfs: Unmount rootfs mounts if not needed
>   wic: Unmount dirs after usage
>   events: Warn if mounted paths left
> 
>  meta/classes/buildchroot.bbclass              |  47 +++++++
>  meta/classes/dpkg-base.bbclass                | 127
> +++++++++++------- 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              |   1 +
>  meta/classes/rootfs.bbclass                   |  62 ++++++++-
>  meta/classes/ubi-img.bbclass                  |   2 +
>  meta/classes/ubifs-img.bbclass                |   2 +
>  meta/classes/wic-img.bbclass                  |  10 ++
>  .../buildchroot/buildchroot.inc               |   8 ++
>  15 files changed, 232 insertions(+), 81 deletions(-)
> 

Re: [PATCH v3 1/5] dpkg: Make mount buildroot reliable

[Henning Schild]

Am Sat,  8 May 2021 09:25:14 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> We use mounting for several tasks. Mounting and unmounting is
> performed in shell scripts. If a command fails before unmounting, the
> directories remain mounted.
> 
> Implement exception handling around the scripts to ensure that
> unmounting is performed also in case of script failure. The number of
> unmounting attempts has been limited to avoid infinite loops.
> 
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
>  meta/classes/dpkg-base.bbclass | 126
> ++++++++++++++++++++------------- meta/classes/dpkg-gbp.bbclass  |
> 8 +-- meta/classes/dpkg.bbclass      |  14 ++--
>  3 files changed, 89 insertions(+), 59 deletions(-)
> 
> diff --git a/meta/classes/dpkg-base.bbclass
> b/meta/classes/dpkg-base.bbclass index 97661a6..d5e70f1 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -61,12 +61,7 @@ addtask patch before do_adjust_git
>  
>  SRC_APT ?= ""
>  
> -do_apt_fetch() {
> -    if [ -z "${@d.getVar("SRC_APT", True).strip()}" ]; then
> -        return 0
> -    fi
> -    dpkg_do_mounts
> -    E="${@ isar_export_proxies(d)}"

Sure that the new pattern will make this shell have the variables?
Building behind proxies is unfortunately something we need. Could be
tried with BB_NO_NETWORK which should make you fail because of
deadend_proxy

Henning

> +fetch_apt() {
>      sudo -E chroot ${BUILDCHROOT_DIR} /usr/bin/apt-get update \
>          -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" \
>          -o Dir::Etc::SourceParts="-" \
> @@ -76,21 +71,25 @@ do_apt_fetch() {
>          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_undo_mounts
> +    dpkg_do_mounts(d)
> +    try:
> +        isar_export_proxies(d)
> +        bb.build.exec_func("fetch_apt", d)
> +    finally:
> +        dpkg_undo_mounts(d)
>  }
>  
>  addtask apt_fetch after do_unpack before do_apt_unpack
>  do_apt_fetch[lockfiles] += "${REPO_ISAR_DIR}/isar.lock"
>  
> -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)}"
> -
> +unpack_apt() {
>      for uri in "${SRC_APT}"; do
>          sudo -E chroot --userspec=$( id -u ):$( id -g )
> ${BUILDCHROOT_DIR} \ sh -c ' \
> @@ -101,8 +100,25 @@ do_apt_unpack() {
>                  dpkg-source -x "${dscfile}" "${PPS}"' \
>                      my_script "${DISTRO}" "${uri}"
>      done
> +}
>  
> -    dpkg_undo_mounts
> +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)
>  }
>  
>  addtask apt_unpack after do_apt_fetch before do_patch
> @@ -146,25 +162,37 @@ do_prepare_build[deptask] = "do_deploy_deb"
>  
>  BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
>  
> -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}
> -}
> +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):
> +    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...")
>  
>  # Placeholder for actual dpkg_runbuild() implementation
>  dpkg_runbuild() {
> @@ -174,10 +202,12 @@ dpkg_runbuild() {
>  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)
> +    dpkg_do_mounts(d)
> +    try:
> +        bb.build.exec_func("dpkg_runbuild", d)
> +    finally:
> +        dpkg_undo_mounts(d)
> +        bb.utils.unlockfile(lock)
>  }
>  
>  addtask dpkg_build before do_build
> @@ -221,16 +251,16 @@ python do_devshell() {
>      oe_lib_path = os.path.join(d.getVar('LAYERDIR_core'), 'lib')
>      sys.path.insert(0, oe_lib_path)
>  
> -    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)
> +    dpkg_do_mounts(d)
> +    try:
> +        isar_export_proxies(d)
>  
> -    bb.build.exec_func('dpkg_undo_mounts', 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)
>  }
>  
>  addtask devshell after do_prepare_build
> diff --git a/meta/classes/dpkg-gbp.bbclass
> b/meta/classes/dpkg-gbp.bbclass index d956e8c..20d2d4c 100644
> --- a/meta/classes/dpkg-gbp.bbclass
> +++ b/meta/classes/dpkg-gbp.bbclass
> @@ -12,12 +12,7 @@ PATCHTOOL ?= "git"
>  GBP_DEPENDS ?= "git-buildpackage pristine-tar"
>  GBP_EXTRA_OPTIONS ?= "--git-pristine-tar"
>  
> -do_install_builddeps_append() {
> -    dpkg_do_mounts
> -    distro="${DISTRO}"
> -    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
> -       distro="${HOST_DISTRO}"
> -    fi
> +builddeps_install_append() {
>      deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
>      sudo -E chroot ${BUILDCHROOT_DIR} \
>          apt-get install -y -o Debug::pkgProblemResolver=yes \
> @@ -26,7 +21,6 @@ do_install_builddeps_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 4e7c2f7..29e2b89 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -6,9 +6,7 @@ inherit dpkg-base
>  PACKAGE_ARCH ?= "${DISTRO_ARCH}"
>  
>  # Install build dependencies for package
> -do_install_builddeps() {
> -    dpkg_do_mounts
> -    E="${@ isar_export_proxies(d)}"
> +builddeps_install() {
>      distro="${DISTRO}"
>      if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
>         distro="${HOST_DISTRO}"
> @@ -19,7 +17,15 @@ do_install_builddeps() {
>      deb_dl_dir_export "${BUILDCHROOT_DIR}" "${distro}"
>      sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
>          ${PP}/${PPS} ${PACKAGE_ARCH}
> -    dpkg_undo_mounts
> +}
> +
> +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)
>  }
>  
>  addtask install_builddeps after do_prepare_build before do_dpkg_build

Re: [PATCH v3 0/5] Rebuild mount logic

[Anton Mikanovich]

08.06.2021 10:58, Henning Schild wrote:
> This series makes things not build in containers anymore, which we
> heavily rely on for CI. Exceptions in the mount logic.
>
> I would suggest to revert and investigate.
>
> Henning

There is no any build issues with container targets at ci.isar-build.org.
Do you have any logs?

-- 
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 v3 1/5] dpkg: Make mount buildroot reliable

[Anton Mikanovich]

08.06.2021 11:09, Henning Schild wrote:
> Sure that the new pattern will make this shell have the variables?
> Building behind proxies is unfortunately something we need. Could be
> tried with BB_NO_NETWORK which should make you fail because of
> deadend_proxy
>
> Henning

This code was not removed but just moved from bash to python.
So there is still check for SRC_APT and isar_export_proxy call there.
Please look at meta/classes/dpkg-base.bbclass

-- 
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 v3 0/5] Rebuild mount logic

[Henning Schild]

[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]

Am Tue, 8 Jun 2021 11:15:26 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 08.06.2021 10:58, Henning Schild wrote:
> > This series makes things not build in containers anymore, which we
> > heavily rely on for CI. Exceptions in the mount logic.
> >
> > I would suggest to revert and investigate.
> >
> > Henning  
> 
> There is no any build issues with container targets at
> ci.isar-build.org. Do you have any logs?

Not container targets, rather isar running inside a container. Our CI
does not run native because if severe security issues that would result
in. (need for sudo) And VMs are hard to set up.

Find the logs attached as a zip file.

I see the first error around 

ERROR: mc:qemuarm64-focal:hello-1.0-r0 do_apt_unpack:

This is a plain run of .gitlab-ci.yml, so using the container
"ghcr.io/siemens/kas/kas-isar:latest"

I guess can be reproduced with
cd isar
mkdir build
docker run --rm -t -i -v $(pwd):/this/:rw --privileged ghcr.io/siemens/kas/kas-isar:latest
sudo chown builder:builder build
scripts/ci_build.sh -q -f


regards,
Henning

[-- Attachment #2: logs-47819947.zip --]
[-- Type: application/zip, Size: 1367189 bytes --]

Re: [PATCH v3 1/5] dpkg: Make mount buildroot reliable

[Henning Schild]

Am Tue, 8 Jun 2021 11:31:03 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 08.06.2021 11:09, Henning Schild wrote:
> > Sure that the new pattern will make this shell have the variables?
> > Building behind proxies is unfortunately something we need. Could be
> > tried with BB_NO_NETWORK which should make you fail because of
> > deadend_proxy
> >
> > Henning  
> 
> This code was not removed but just moved from bash to python.
> So there is still check for SRC_APT and isar_export_proxy call there.
> Please look at meta/classes/dpkg-base.bbclass

I did see that it was moved. But shell functions in bitbake often open
a new shell and do not inherit all env, that is why we need to export
the variables all over the place instead of just doing it once. And i
am afraid doing in python and later forking a fresh shell might make
you not have the variables in the shell.

It should be double checked that shell functions opened with the
pattern of "isar_export_proxies; bb.build.exec_func" do receive the
variables as they used to. Maybe it is already the case, just asking
and pointing out.

Henning

Re: [PATCH v3 0/5] Rebuild mount logic

[Henning Schild]

Am Tue, 8 Jun 2021 11:15:26 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 08.06.2021 10:58, Henning Schild wrote:
> > This series makes things not build in containers anymore, which we
> > heavily rely on for CI. Exceptions in the mount logic.
> >
> > I would suggest to revert and investigate.
> >
> > Henning  
> 
> There is no any build issues with container targets at
> ci.isar-build.org. Do you have any logs?

I think the core of the problem is that "umount" can fail and that
failure is not handled. Before we maybe ignored it ...

In the logs i sent it failed with "target is busy"

Henning

Re: [PATCH v3 0/5] Rebuild mount logic

[Anton Mikanovich]

08.06.2021 11:54, Henning Schild wrote:
> I think the core of the problem is that "umount" can fail and that
> failure is not handled. Before we maybe ignored it ...
>
> In the logs i sent it failed with "target is busy"
>
> Henning

I've found the reason and it was quite strange:
The construction `$(($(< filename) + 1))` with `1` inside the file was 
returning 1, while equivalent `$(($(cat filename) + 1))` was working 
correctly.
So I've changed it to little bit slower `cat` version and now there is 
no issues when building Isar inside the container.
Please look at the patch: "[PATCH] buildchroot: Remove bashism when 
reading file content"

-- 
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 v3 0/5] Rebuild mount logic

[Henning Schild]

Am Thu, 10 Jun 2021 10:12:24 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 08.06.2021 11:54, Henning Schild wrote:
> > I think the core of the problem is that "umount" can fail and that
> > failure is not handled. Before we maybe ignored it ...
> >
> > In the logs i sent it failed with "target is busy"
> >
> > Henning  
> 
> I've found the reason and it was quite strange:
> The construction `$(($(< filename) + 1))` with `1` inside the file
> was returning 1, while equivalent `$(($(cat filename) + 1))` was
> working correctly.
> So I've changed it to little bit slower `cat` version and now there
> is no issues when building Isar inside the container.
> Please look at the patch: "[PATCH] buildchroot: Remove bashism when 
> reading file content"

Cool, reason is that bitbake is not using bash. It might be using
whatever the default shell is. At least i think so.

Thanks!
Henning

Re: [PATCH v3 0/5] Rebuild mount logic

[Jan Kiszka]

On 08.05.21 08:25, Anton Mikanovich wrote:
> Mounting is perfomed very inconsistent currently in Isar. A lot of mount
> points are left after task execution and stays mounted until cleanup in
> build_completed hook.
> 
> We already experienced issues with stuck forever on unmounting at
> "Couldn't unmount, retrying..." error.
> It is also quite easy to face with a lot of ramaining mounts after the
> failed or interrupted manual builds.
> 
> This patchset implements unmounting paths after usage. In the cases of
> mount sharing corresponding request counting is performed, so real mount
> will be done only at first request, real unmount will be done only after
> the last task will try to unmount the path.
> 
> Meantime build_completed will use emergency only, so following warning
> added to show something goes wrong.
> 
> ---
> Changes since v2:
> - Prevent race conditions.
> - Fix building qemuarm64-stretch.
> - Minimize locks.
> - Cleanup counter files.
> - Unmount /proc/sys/fs/binfmt_misc also.
> Changes since v1:
> - Make all mounts be unmounted after use.
> - Warn about lost mounts after finish.
> 
> Anton Mikanovich (5):
>   dpkg: Make mount buildroot reliable
>   buildchroot: Unmount buildchroot mounts if not needed
>   rootfs: Unmount rootfs mounts if not needed
>   wic: Unmount dirs after usage
>   events: Warn if mounted paths left
> 
>  meta/classes/buildchroot.bbclass              |  47 +++++++
>  meta/classes/dpkg-base.bbclass                | 127 +++++++++++-------
>  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              |   1 +
>  meta/classes/rootfs.bbclass                   |  62 ++++++++-
>  meta/classes/ubi-img.bbclass                  |   2 +
>  meta/classes/ubifs-img.bbclass                |   2 +
>  meta/classes/wic-img.bbclass                  |  10 ++
>  .../buildchroot/buildchroot.inc               |   8 ++
>  15 files changed, 232 insertions(+), 81 deletions(-)
> 

This one is still not stable (reverting my claim "current next builds 
fine"):

NOTE: recipe buildchroot-target-1.0-r0: task do_rootfs_postprocess: Started
ERROR: mc:rpi-stretch:buildchroot-target-1.0-r0 do_rootfs_postprocess: Error executing a python function in exec_python_func() autogenerated:

The stack trace of python calls that resulted in this exception/failure was:
File: 'exec_python_func() autogenerated', lineno: 2, function: <module>
     0001:
 *** 0002:do_rootfs_postprocess(d)
     0003:
File: '/builds/Vs1BETDQ/0/ebsy/debian/isar/meta/classes/rootfs.bbclass', lineno: 298, function: do_rootfs_postprocess
     0294:        for i, cmd in enumerate(cmds):
     0295:            bb.build.exec_func(cmd, d)
     0296:            progress_reporter.update(int(i / len(cmds) * 100))
     0297:
 *** 0298:    bb.build.exec_func('rootfs_undo_mounts', d)
     0299:}
     0300:addtask rootfs_postprocess before do_rootfs
     0301:
     0302:python do_rootfs() {
File: '/builds/Vs1BETDQ/0/ebsy/debian/isar/bitbake/lib/bb/build.py', lineno: 251, function: exec_func
     0247:    with bb.utils.fileslocked(lockfiles):
     0248:        if ispython:
     0249:            exec_func_python(func, d, runfile, cwd=adir)
     0250:        else:
 *** 0251:            exec_func_shell(func, d, runfile, cwd=adir)
     0252:
     0253:    try:
     0254:        curcwd = os.getcwd()
     0255:    except:
File: '/builds/Vs1BETDQ/0/ebsy/debian/isar/bitbake/lib/bb/build.py', lineno: 452, function: exec_func_shell
     0448:    with open(fifopath, 'r+b', buffering=0) as fifo:
     0449:        try:
     0450:            bb.debug(2, "Executing shell function %s" % func)
     0451:            with open(os.devnull, 'r+') as stdin, logfile:
 *** 0452:                bb.process.run(cmd, shell=False, stdin=stdin, log=logfile, extrafiles=[(fifo,readfifo)])
     0453:        finally:
     0454:            os.unlink(fifopath)
     0455:
     0456:    bb.debug(2, "Shell function %s finished" % func)
File: '/builds/Vs1BETDQ/0/ebsy/debian/isar/bitbake/lib/bb/process.py', lineno: 182, function: run
     0178:        if not stderr is None:
     0179:            stderr = stderr.decode("utf-8")
     0180:
     0181:    if pipe.returncode != 0:
 *** 0182:        raise ExecutionError(cmd, pipe.returncode, stdout, stderr)
     0183:    return stdout, stderr
Exception: bb.process.ExecutionError: Execution of '/builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/temp/run.rootfs_undo_mounts.19080' failed with exit code 32:
umount: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/sys/fs/cgroup: target is busy.
WARNING: exit code 32 from a shell command.


ERROR: Logfile of failure stored in: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/temp/log.do_rootfs_postprocess.19080
NOTE: recipe buildchroot-target-1.0-r0: task do_rootfs_postprocess: Failed
ERROR: Task (mc:rpi-stretch:/builds/Vs1BETDQ/0/ebsy/debian/isar/meta/recipes-dev


And then also 

NOTE: Tasks Summary: Attempted 1181 tasks of which 214 didn't need to be rerun and 1 failed.
WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/dev left mounted, unmounting...
WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/dev/pts left mounted, unmounting...
WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/dev/mqueue left mounted, unmounting...
WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/dev/termination-log left mounted, unmounting...
WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/dev/shm left mounted, unmounting...
WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/proc left mounted, unmounting...
WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/sys left mounted, unmounting...
WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/sys/fs/cgroup left mounted, unmounting...
WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/sys/fs/cgroup/cpuset left mounted, unmounting...
WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/downloads left mounted, unmounting...

Summary: 1 task failed:
  mc:rpi-stretch:/builds/Vs1BETDQ/0/ebsy/debian/isar/meta/recipes-devtools/buildchroot/buildchroot-target.bb:do_rootfs_postprocess


Though I suspect and hope the latter is related to the former.

Retrying, to see if we have a race (likely) or something stable.

Jan

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

Re: [PATCH v3 0/5] Rebuild mount logic

[Anton Mikanovich]

21.06.2021 18:54, Jan Kiszka wrote:
> This one is still not stable (reverting my claim "current next builds
> fine"):
>
> NOTE: recipe buildchroot-target-1.0-r0: task do_rootfs_postprocess: Started
> ERROR: mc:rpi-stretch:buildchroot-target-1.0-r0 do_rootfs_postprocess: Error executing a python function in exec_python_func() autogenerated:
>
> The stack trace of python calls that resulted in this exception/failure was:
> File: 'exec_python_func() autogenerated', lineno: 2, function: <module>
>       0001:
>   *** 0002:do_rootfs_postprocess(d)
>       0003:
> File: '/builds/Vs1BETDQ/0/ebsy/debian/isar/meta/classes/rootfs.bbclass', lineno: 298, function: do_rootfs_postprocess
>       0294:        for i, cmd in enumerate(cmds):
>       0295:            bb.build.exec_func(cmd, d)
>       0296:            progress_reporter.update(int(i / len(cmds) * 100))
>       0297:
>   *** 0298:    bb.build.exec_func('rootfs_undo_mounts', d)
>       0299:}
>       0300:addtask rootfs_postprocess before do_rootfs
>       0301:
>       0302:python do_rootfs() {
> File: '/builds/Vs1BETDQ/0/ebsy/debian/isar/bitbake/lib/bb/build.py', lineno: 251, function: exec_func
>       0247:    with bb.utils.fileslocked(lockfiles):
>       0248:        if ispython:
>       0249:            exec_func_python(func, d, runfile, cwd=adir)
>       0250:        else:
>   *** 0251:            exec_func_shell(func, d, runfile, cwd=adir)
>       0252:
>       0253:    try:
>       0254:        curcwd = os.getcwd()
>       0255:    except:
> File: '/builds/Vs1BETDQ/0/ebsy/debian/isar/bitbake/lib/bb/build.py', lineno: 452, function: exec_func_shell
>       0448:    with open(fifopath, 'r+b', buffering=0) as fifo:
>       0449:        try:
>       0450:            bb.debug(2, "Executing shell function %s" % func)
>       0451:            with open(os.devnull, 'r+') as stdin, logfile:
>   *** 0452:                bb.process.run(cmd, shell=False, stdin=stdin, log=logfile, extrafiles=[(fifo,readfifo)])
>       0453:        finally:
>       0454:            os.unlink(fifopath)
>       0455:
>       0456:    bb.debug(2, "Shell function %s finished" % func)
> File: '/builds/Vs1BETDQ/0/ebsy/debian/isar/bitbake/lib/bb/process.py', lineno: 182, function: run
>       0178:        if not stderr is None:
>       0179:            stderr = stderr.decode("utf-8")
>       0180:
>       0181:    if pipe.returncode != 0:
>   *** 0182:        raise ExecutionError(cmd, pipe.returncode, stdout, stderr)
>       0183:    return stdout, stderr
> Exception: bb.process.ExecutionError: Execution of '/builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/temp/run.rootfs_undo_mounts.19080' failed with exit code 32:
> umount: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/sys/fs/cgroup: target is busy.
> WARNING: exit code 32 from a shell command.
>
>
> ERROR: Logfile of failure stored in: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/temp/log.do_rootfs_postprocess.19080
> NOTE: recipe buildchroot-target-1.0-r0: task do_rootfs_postprocess: Failed
> ERROR: Task (mc:rpi-stretch:/builds/Vs1BETDQ/0/ebsy/debian/isar/meta/recipes-dev
Looks like a race.
It was not a problem when we left mounts till the end of the build.
I will try to look for any possible reasons.
> And then also
>
> NOTE: Tasks Summary: Attempted 1181 tasks of which 214 didn't need to be rerun and 1 failed.
> WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/dev left mounted, unmounting...
> WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/dev/pts left mounted, unmounting...
> WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/dev/mqueue left mounted, unmounting...
> WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/dev/termination-log left mounted, unmounting...
> WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/dev/shm left mounted, unmounting...
> WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/proc left mounted, unmounting...
> WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/sys left mounted, unmounting...
> WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/sys/fs/cgroup left mounted, unmounting...
> WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/sys/fs/cgroup/cpuset left mounted, unmounting...
> WARNING: /builds/Vs1BETDQ/0/ebsy/debian/isar/build/tmp/work/raspbian-stretch-armhf/buildchroot-target/1.0-r0/rootfs/downloads left mounted, unmounting...
>
> Summary: 1 task failed:
>    mc:rpi-stretch:/builds/Vs1BETDQ/0/ebsy/debian/isar/meta/recipes-devtools/buildchroot/buildchroot-target.bb:do_rootfs_postprocess
>
>
> Though I suspect and hope the latter is related to the former.
Yes, it's just a 'visualization' of mount points cleanup at finalize 
task, called just after the build was failed.
It supposed to be empty for successful builds, but not for failed ones.
> Retrying, to see if we have a race (likely) or something stable.
>
> Jan

-- 
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 v3 5/5] events: Warn if mounted paths left

[Jan Kiszka]

On 08.05.21 08:25, Anton Mikanovich wrote:
> Output warnings in case any mounts are left after build.
> 
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
>  meta/classes/isar-events.bbclass | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
> index 4c49635..73419b4 100644
> --- a/meta/classes/isar-events.bbclass
> +++ b/meta/classes/isar-events.bbclass
> @@ -52,6 +52,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])
>                  subprocess.call(
>                      ["sudo", "umount", "-l", line.split()[1]],
>                      stdout=subprocess.DEVNULL,
> 

This is misplaced: If you want to warn about imbalanced mount/umount,
check that all mounts are reverted after a task class successfully
finished. We should not spam the user with additional warnings when the
build is already failing, specifically as only dpkg implements a
try-finally pattern (in way that breaks existing users...), the rest not.

Jan

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

Re: [PATCH v3 1/5] dpkg: Make mount buildroot reliable

[Jan Kiszka]

On 08.05.21 08:25, Anton Mikanovich wrote:
> We use mounting for several tasks. Mounting and unmounting is performed
> in shell scripts. If a command fails before unmounting, the directories
> remain mounted.
> 
> Implement exception handling around the scripts to ensure that
> unmounting is performed also in case of script failure. The number of
> unmounting attempts has been limited to avoid infinite loops.
> 
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
>  meta/classes/dpkg-base.bbclass | 126 ++++++++++++++++++++-------------
>  meta/classes/dpkg-gbp.bbclass  |   8 +--
>  meta/classes/dpkg.bbclass      |  14 ++--
>  3 files changed, 89 insertions(+), 59 deletions(-)
> 
> diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
> index 97661a6..d5e70f1 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -61,12 +61,7 @@ addtask patch before do_adjust_git
>  
>  SRC_APT ?= ""
>  
> -do_apt_fetch() {
> -    if [ -z "${@d.getVar("SRC_APT", True).strip()}" ]; then
> -        return 0
> -    fi
> -    dpkg_do_mounts
> -    E="${@ isar_export_proxies(d)}"
> +fetch_apt() {
>      sudo -E chroot ${BUILDCHROOT_DIR} /usr/bin/apt-get update \
>          -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" \
>          -o Dir::Etc::SourceParts="-" \
> @@ -76,21 +71,25 @@ do_apt_fetch() {
>          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_undo_mounts
> +    dpkg_do_mounts(d)
> +    try:
> +        isar_export_proxies(d)
> +        bb.build.exec_func("fetch_apt", d)
> +    finally:
> +        dpkg_undo_mounts(d)
>  }
>  
>  addtask apt_fetch after do_unpack before do_apt_unpack
>  do_apt_fetch[lockfiles] += "${REPO_ISAR_DIR}/isar.lock"
>  
> -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)}"
> -
> +unpack_apt() {
>      for uri in "${SRC_APT}"; do
>          sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} \
>              sh -c ' \
> @@ -101,8 +100,25 @@ do_apt_unpack() {
>                  dpkg-source -x "${dscfile}" "${PPS}"' \
>                      my_script "${DISTRO}" "${uri}"
>      done
> +}
>  
> -    dpkg_undo_mounts
> +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)
>  }
>  
>  addtask apt_unpack after do_apt_fetch before do_patch
> @@ -146,25 +162,37 @@ do_prepare_build[deptask] = "do_deploy_deb"
>  
>  BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
>  
> -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}
> -}
> +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):
> +    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...")
>  
>  # Placeholder for actual dpkg_runbuild() implementation
>  dpkg_runbuild() {
> @@ -174,10 +202,12 @@ dpkg_runbuild() {
>  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)
> +    dpkg_do_mounts(d)
> +    try:
> +        bb.build.exec_func("dpkg_runbuild", d)
> +    finally:
> +        dpkg_undo_mounts(d)
> +        bb.utils.unlockfile(lock)
>  }
>  
>  addtask dpkg_build before do_build
> @@ -221,16 +251,16 @@ python do_devshell() {
>      oe_lib_path = os.path.join(d.getVar('LAYERDIR_core'), 'lib')
>      sys.path.insert(0, oe_lib_path)
>  
> -    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)
> +    dpkg_do_mounts(d)
> +    try:
> +        isar_export_proxies(d)
>  
> -    bb.build.exec_func('dpkg_undo_mounts', 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)
>  }
>  
>  addtask devshell after do_prepare_build
> diff --git a/meta/classes/dpkg-gbp.bbclass b/meta/classes/dpkg-gbp.bbclass
> index d956e8c..20d2d4c 100644
> --- a/meta/classes/dpkg-gbp.bbclass
> +++ b/meta/classes/dpkg-gbp.bbclass
> @@ -12,12 +12,7 @@ PATCHTOOL ?= "git"
>  GBP_DEPENDS ?= "git-buildpackage pristine-tar"
>  GBP_EXTRA_OPTIONS ?= "--git-pristine-tar"
>  
> -do_install_builddeps_append() {
> -    dpkg_do_mounts
> -    distro="${DISTRO}"
> -    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
> -       distro="${HOST_DISTRO}"
> -    fi
> +builddeps_install_append() {
>      deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
>      sudo -E chroot ${BUILDCHROOT_DIR} \
>          apt-get install -y -o Debug::pkgProblemResolver=yes \
> @@ -26,7 +21,6 @@ do_install_builddeps_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 4e7c2f7..29e2b89 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -6,9 +6,7 @@ inherit dpkg-base
>  PACKAGE_ARCH ?= "${DISTRO_ARCH}"
>  
>  # Install build dependencies for package
> -do_install_builddeps() {
> -    dpkg_do_mounts
> -    E="${@ isar_export_proxies(d)}"
> +builddeps_install() {
>      distro="${DISTRO}"
>      if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
>         distro="${HOST_DISTRO}"
> @@ -19,7 +17,15 @@ do_install_builddeps() {
>      deb_dl_dir_export "${BUILDCHROOT_DIR}" "${distro}"
>      sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
>          ${PP}/${PPS} ${PACKAGE_ARCH}
> -    dpkg_undo_mounts
> +}
> +
> +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)
>  }
>  
>  addtask install_builddeps after do_prepare_build before do_dpkg_build
> 

As written before, this is the only class applying try-finally for
cleaning up mounts on failing task. One price is API breakage because
dpkg_do_mounts and dpkg_undo_mounts are converted to python functions.
Given that all other mount/unmout services do not do this extra effort,
I would consider that price too high. Just let the build_completed
handler clean up, that's what it's for.

Jan

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