[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

[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 <ami...@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

[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 <ami...@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

[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 <ami...@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

[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 <ami...@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

[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 <ami...@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

[Henning Schild]

Am Mon, 16 Aug 2021 18:37:56 +0300
schrieb Anton Mikanovich <ami...@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

[Henning Schild]

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

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

[Jan Kiszka]

On 16.08.21 17:53, Henning Schild wrote:
> Am Mon, 16 Aug 2021 18:37:56 +0300
> schrieb Anton Mikanovich <ami...@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

[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

[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 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 (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/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 | 2 +-
5 files changed, 97 insertions(+), 91 deletions(-)

--
2.25.1

[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 <ami...@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

+ return 0
+ fi

+ 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)
}

[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 <ami...@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

[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 <ami...@ilbers.de>
---

+ exit 0
+ fi

[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 <ami...@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

[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 <ami...@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

[Jan Kiszka]

I've pushed that to our CI as well, and if that runs fine, I can give a
try in one or two downstream layers (which are not on any mount change
so far). But from reading things, it looks good and fairly consistent now.

[Jan Kiszka]

Unfortunately:

Setting up git-buildpackage (0.8.12.2) ...
umount: /builds/BLKE9aZ_/2/ebsy/debian/isar/build/tmp/work/debian-stretch-amd64/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/BLKE9aZ_/2/ebsy/debian/isar/build/tmp/work/debian-stretch-amd64/cowsay/git-r0/temp/log.do_install_builddeps.32372

[Anton Mikanovich]

18.08.2021 20:39, Jan Kiszka wrote:
> Unfortunately:
>
> Setting up git-buildpackage (0.8.12.2) ...
> umount: /builds/BLKE9aZ_/2/ebsy/debian/isar/build/tmp/work/debian-stretch-amd64/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/BLKE9aZ_/2/ebsy/debian/isar/build/tmp/work/debian-stretch-amd64/cowsay/git-r0/temp/log.do_install_builddeps.32372
>
> Jan
>

As I've already mentioned before, working on the '/sys/fs/cgroups is
busy' issue is still in progress.
It was not supposed to be covered with the current patchset to not waste
a time in 'broken next API' state, because it looks like fixing 'target
is busy' will take more time.

[Jan Kiszka]

On 20.08.21 10:12, Anton Mikanovich wrote:
> 18.08.2021 20:39, Jan Kiszka wrote:
>> Unfortunately:
>>
>> Setting up git-buildpackage (0.8.12.2) ...
>> umount:
>> /builds/BLKE9aZ_/2/ebsy/debian/isar/build/tmp/work/debian-stretch-amd64/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/BLKE9aZ_/2/ebsy/debian/isar/build/tmp/work/debian-stretch-amd64/cowsay/git-r0/temp/log.do_install_builddeps.32372
>>
>>
>> Jan
>>
> As I've already mentioned before, working on the '/sys/fs/cgroups is
> busy' issue is still in progress.
> It was not supposed to be covered with the current patchset to not waste
> a time in 'broken next API' state, because it looks like fixing 'target
> is busy' will take more time.
>

Then, again, as we have broken next now for several months, please roll
everything back that caused this (all mount-related changes), create a
wip branch to debug the regression while allowing features to progress
on next.

Thanks,

[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

[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 <ami...@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

+ return 0
+ fi

+ 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)
}

[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 <ami...@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

[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 <ami...@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

[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 <ami...@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:

[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 <ami...@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

[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 <ami...@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

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

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

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

[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 <ami...@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

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

[Henning Schild]

Am Thu, 9 Sep 2021 11:13:37 +0300
schrieb Anton Mikanovich <ami...@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

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