[PATCH v4 0/5] Restore downstream mounts compatibility

[PATCH v4 0/5] 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 enable reference counting for image mounts for several image tasks
running in parallel (like e.g. ubi-ubifs-img).

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

---
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 (5):
  Revert "dpkg: Make mount buildroot reliable"
  dpkg: Limit unmount loop
  image: Add reference counter
  dpkg-base: Clean up unmounting in do_dpkg_build()
  events: Do not warn on left mounts by default

 meta-isar/conf/local.conf.sample |   3 +
 meta/classes/dpkg-base.bbclass   | 125 +++++++++++++------------------
 meta/classes/dpkg-gbp.bbclass    |   8 +-
 meta/classes/dpkg.bbclass        |  14 +---
 meta/classes/image.bbclass       |  39 +++++++++-
 meta/classes/isar-events.bbclass |   3 +-
 6 files changed, 101 insertions(+), 91 deletions(-)

-- 
2.25.1

[PATCH v4 1/5] 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 v4 2/5] 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..105a1e6 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 -gt 10000 ]; then
+            bbfatal "${BUILDROOT}: Couldn't unmount after timeout"
         fi
+        i=`expr $i + 1`
     done
     sudo rmdir ${BUILDROOT}
 }
-- 
2.25.1

[PATCH v4 3/5] image: Add reference counter

[Anton Mikanovich]

Image generation can be also executed in parallel, so we need to control
the usage of image_do_mounts / image_undo_mounts.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/image.bbclass | 39 ++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 12d1616..b2a828c 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -52,23 +52,54 @@ DEPENDS += "${IMAGE_INSTALL}"
 ISAR_RELEASE_CMD_DEFAULT = "git -C ${LAYERDIR_core} describe --tags --dirty --match 'v[0-9].[0-9]*'"
 ISAR_RELEASE_CMD ?= "${ISAR_RELEASE_CMD_DEFAULT}"
 
+IMG_MOUNT_CNT = "${BUILDCHROOT_DIR}_image.mount"
+
 image_do_mounts() {
-    sudo flock ${MOUNT_LOCKFILE} -c ' \
+    sudo -s <<'EOSUDO'
+        ( flock 9
+        set -e
+
+        count="1"
+        if [ -f "${IMG_MOUNT_CNT}" ]; then
+            count=$(($(cat "${IMG_MOUNT_CNT}") + 1))
+        fi
+        echo $count > "${IMG_MOUNT_CNT}"
+        if [ $count -gt 1 ]; then
+            exit 0
+        fi
+
         mkdir -p "${BUILDROOT_DEPLOY}" "${BUILDROOT_ROOTFS}" "${BUILDROOT_WORK}"
         mount --bind "${DEPLOY_DIR_IMAGE}" "${BUILDROOT_DEPLOY}"
         mount --bind "${IMAGE_ROOTFS}" "${BUILDROOT_ROOTFS}"
         mount --bind "${WORKDIR}" "${BUILDROOT_WORK}"
-    '
+
+        ) 9>'${MOUNT_LOCKFILE}'
+EOSUDO
     buildchroot_do_mounts
 }
 
 image_undo_mounts() {
     buildchroot_undo_mounts
-    sudo flock ${MOUNT_LOCKFILE} -c ' \
+    sudo -s <<'EOSUDO'
+        ( flock 9
+        set -e
+
+        count="0"
+        if [ -f "${IMG_MOUNT_CNT}" ]; then
+            count=$(($(cat "${IMG_MOUNT_CNT}") - 1))
+            echo $count > "${IMG_MOUNT_CNT}"
+        fi
+        if [ $count -gt 0 ]; then
+            exit 0
+        fi
+        rm -f "${IMG_MOUNT_CNT}"
+
         umount "${BUILDROOT_DEPLOY}"
         umount "${BUILDROOT_ROOTFS}"
         umount "${BUILDROOT_WORK}"
-    '
+
+        ) 9>'${MOUNT_LOCKFILE}'
+EOSUDO
 }
 
 ROOTFSDIR = "${IMAGE_ROOTFS}"
-- 
2.25.1

[PATCH v4 4/5] 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 105a1e6..fe814e0 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 v4 5/5] 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.

Logging can be enabled by setting ISAR_MOUNTS_CLEANUP_DEBUG to 1.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta-isar/conf/local.conf.sample | 3 +++
 meta/classes/isar-events.bbclass | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/meta-isar/conf/local.conf.sample b/meta-isar/conf/local.conf.sample
index 6cf1656..ca43261 100644
--- a/meta-isar/conf/local.conf.sample
+++ b/meta-isar/conf/local.conf.sample
@@ -219,3 +219,6 @@ USER_isar[flags] = "system create-home"
 
 # Uncomment the below line to debug WIC.
 # WIC_CREATE_EXTRA_ARGS += "-D"
+
+# Uncomment this to enable warn on all left mounts after build complete
+#ISAR_MOUNTS_CLEANUP_DEBUG ?= "1"
diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
index 2f8bf6e..60bb1a9 100644
--- a/meta/classes/isar-events.bbclass
+++ b/meta/classes/isar-events.bbclass
@@ -54,7 +54,8 @@ 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])
+                if d.getVar('ISAR_MOUNTS_CLEANUP_DEBUG', True) == '1':
+                    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 v4 5/5] events: Do not warn on left mounts by default

[Henning Schild]

Am Mon, 16 Aug 2021 18:37:56 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> Disable warnings printing for all the mount points left after build
> completed to do not flood build logs too much in case build fail.
> 
> Logging can be enabled by setting ISAR_MOUNTS_CLEANUP_DEBUG to 1.

Can we find something more generic to attach to? Something that
represents verbose logging / debug tracing in general? We can not
invent a new variable for every aspect we want to debug. Could also
just comment that out ... to have it back handy for someone having to
debug.

Henning

> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
>  meta-isar/conf/local.conf.sample | 3 +++
>  meta/classes/isar-events.bbclass | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/meta-isar/conf/local.conf.sample
> b/meta-isar/conf/local.conf.sample index 6cf1656..ca43261 100644
> --- a/meta-isar/conf/local.conf.sample
> +++ b/meta-isar/conf/local.conf.sample
> @@ -219,3 +219,6 @@ USER_isar[flags] = "system create-home"
>  
>  # Uncomment the below line to debug WIC.
>  # WIC_CREATE_EXTRA_ARGS += "-D"
> +
> +# Uncomment this to enable warn on all left mounts after build
> complete +#ISAR_MOUNTS_CLEANUP_DEBUG ?= "1"
> diff --git a/meta/classes/isar-events.bbclass
> b/meta/classes/isar-events.bbclass index 2f8bf6e..60bb1a9 100644
> --- a/meta/classes/isar-events.bbclass
> +++ b/meta/classes/isar-events.bbclass
> @@ -54,7 +54,8 @@ 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])
> +                if d.getVar('ISAR_MOUNTS_CLEANUP_DEBUG', True) ==
> '1':
> +                    bb.warn('%s left mounted, unmounting...' %
> line.split()[1]) subprocess.call(
>                      ["sudo", "umount", "-l", line.split()[1]],
>                      stdout=subprocess.DEVNULL,

Re: [PATCH v4 2/5] dpkg: Limit unmount loop

[Henning Schild]

Am Mon, 16 Aug 2021 18:37:53 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 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..105a1e6 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 -gt 10000 ]; then
> +            bbfatal "${BUILDROOT}: Couldn't unmount after timeout"
>          fi
> +        i=`expr $i + 1`

IMHO it would be easier to read how long that will take in total ... if
it was "sleep 1" ... "% 10" and "-gt 1000" ... in fact i think it
should be "-ge 1000" now that you start with 0. But on off-by-one in a
timeout it not the biggest problem.

Henning

>      done
>      sudo rmdir ${BUILDROOT}
>  }

Re: [PATCH v4 5/5] events: Do not warn on left mounts by default

[Jan Kiszka]

On 16.08.21 17:53, Henning Schild wrote:
> Am Mon, 16 Aug 2021 18:37:56 +0300
> schrieb Anton Mikanovich <amikan@ilbers.de>:
> 
>> Disable warnings printing for all the mount points left after build
>> completed to do not flood build logs too much in case build fail.
>>
>> Logging can be enabled by setting ISAR_MOUNTS_CLEANUP_DEBUG to 1.
> 
> Can we find something more generic to attach to? Something that
> represents verbose logging / debug tracing in general? We can not
> invent a new variable for every aspect we want to debug. Could also
> just comment that out ... to have it back handy for someone having to
> debug.

The goal should be that this warning can remain always on. But that
implies having resolved all issues where it is not pointing to a
separate problem but rather only prolongs the dump of the actual issue.
Where are we in that journey?

Jan

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

Re: [PATCH v4 5/5] events: Do not warn on left mounts by default

[Anton Mikanovich]

16.08.2021 18:53, Henning Schild wrote:
> Can we find something more generic to attach to? Something that
> represents verbose logging / debug tracing in general? We can not
> invent a new variable for every aspect we want to debug. Could also
> just comment that out ... to have it back handy for someone having to
> debug.
>
> Henning

As Jan already mentioned, originally this supposed to be something to be 
always on to take care about the right mount API usage.
But currently we have other places where covering mounts with 
try-finally require huge rewriting all that tasks in Python which will 
break the compatibility. That's why additional debug messages will 
appear for all the failures outside do_dpkg_build but inside mount sections.

Commenting this code out is not a good way if we need to have it 
configured from outside (f.e. by CI). I've moved those printouts from 
warning to debug in v5, so it will be available with all other debug 
information (enabled by -D).

-- 
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