* [PATCH v1 0/5] Restore downstream mounts compatibility
@ 2021-07-07 16:38 Anton Mikanovich
2021-07-07 16:38 ` [PATCH v1 1/5] Revert "dpkg: Make mount buildroot reliable" Anton Mikanovich
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Anton Mikanovich @ 2021-07-07 16:38 UTC (permalink / raw)
To: isar-users; +Cc: Anton Mikanovich
Revert commit d21d49578e5a3b0019075d1946bd93a95914fcca which has broken
compatibility with upstream projects.
Make the same fix by catching bb.build.TaskFailed event and unmount
any lost paths.
Also make undo_mounts functions to be safe for calling in case there is
no do_mounts call before. This will make fail policy more tolerant.
Enable refrence counting for image mounts in case there are several
images using like ubi-ubifs-img does.
This implementation do not call buildchroot_undo_mounts in task fail
handler, because there is hard not to broke other tasks execution. So
it will not fully cover pure buildchroot_do_mounts usage like
wic-img.bbclass does.
Tested with kas-iot2050-example project from:
https://github.com/siemens/meta-iot2050
Anton Mikanovich (5):
Revert "dpkg: Make mount buildroot reliable"
mount: Allow calling unmount on not mounted paths
dpkg: Remove unmount loop
image: Add reference counter
events: Unmount all lost mounts at task fail
meta/classes/buildchroot.bbclass | 20 ++---
meta/classes/dpkg-base.bbclass | 122 +++++++++++--------------------
meta/classes/dpkg-gbp.bbclass | 8 +-
meta/classes/dpkg.bbclass | 14 +---
meta/classes/image.bbclass | 39 +++++++++-
meta/classes/isar-events.bbclass | 27 +++++++
meta/classes/rootfs.bbclass | 20 ++---
7 files changed, 136 insertions(+), 114 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 1/5] Revert "dpkg: Make mount buildroot reliable"
2021-07-07 16:38 [PATCH v1 0/5] Restore downstream mounts compatibility Anton Mikanovich
@ 2021-07-07 16:38 ` Anton Mikanovich
2021-07-08 12:16 ` Jan Kiszka
2021-07-07 16:38 ` [PATCH v1 2/5] mount: Allow calling unmount on not mounted paths Anton Mikanovich
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Anton Mikanovich @ 2021-07-07 16:38 UTC (permalink / raw)
To: isar-users; +Cc: Anton Mikanovich
This reverts commit d21d49578e5a3b0019075d1946bd93a95914fcca.
---
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 ed162b3..97661a6 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,25 +76,21 @@ 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
do_apt_fetch[lockfiles] += "${REPO_ISAR_DIR}/isar.lock"
-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 ' \
@@ -100,25 +101,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
@@ -162,38 +146,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() {
@@ -203,12 +174,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
@@ -252,16 +221,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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 2/5] mount: Allow calling unmount on not mounted paths
2021-07-07 16:38 [PATCH v1 0/5] Restore downstream mounts compatibility Anton Mikanovich
2021-07-07 16:38 ` [PATCH v1 1/5] Revert "dpkg: Make mount buildroot reliable" Anton Mikanovich
@ 2021-07-07 16:38 ` Anton Mikanovich
2021-07-08 12:17 ` Jan Kiszka
2021-07-07 16:38 ` [PATCH v1 3/5] dpkg: Remove unmount loop Anton Mikanovich
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Anton Mikanovich @ 2021-07-07 16:38 UTC (permalink / raw)
To: isar-users; +Cc: Anton Mikanovich
Make buildchroot_undo_mounts and rootfs_undo_mounts functions do not
fail if calling without buildchroot_do_mounts and rootfs_do_mounts.
Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
meta/classes/buildchroot.bbclass | 20 ++++++++++----------
meta/classes/rootfs.bbclass | 20 ++++++++++----------
2 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/meta/classes/buildchroot.bbclass b/meta/classes/buildchroot.bbclass
index 806a29f..9c27711 100644
--- a/meta/classes/buildchroot.bbclass
+++ b/meta/classes/buildchroot.bbclass
@@ -23,16 +23,18 @@ python __anonymous() {
MOUNT_LOCKFILE = "${BUILDCHROOT_DIR}.lock"
+BUILDCHROOT_MOUNT_CNT = "${BUILDCHROOT_DIR}.mount"
+
buildchroot_do_mounts() {
sudo -s <<'EOSUDO'
( flock 9
set -e
count="1"
- if [ -f '${BUILDCHROOT_DIR}.mount' ]; then
- count=$(($(cat '${BUILDCHROOT_DIR}.mount') + 1))
+ if [ -f "${BUILDCHROOT_MOUNT_CNT}" ]; then
+ count=$(($(cat "${BUILDCHROOT_MOUNT_CNT}") + 1))
fi
- echo $count > '${BUILDCHROOT_DIR}.mount'
+ echo $count > "${BUILDCHROOT_MOUNT_CNT}"
if [ $count -gt 1 ]; then
exit 0
fi
@@ -77,17 +79,15 @@ buildchroot_undo_mounts() {
( 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
+ count="0"
+ if [ -f "${BUILDCHROOT_MOUNT_CNT}" ]; then
+ count=$(($(cat "${BUILDCHROOT_MOUNT_CNT}") - 1))
+ echo $count > "${BUILDCHROOT_MOUNT_CNT}"
fi
if [ $count -gt 0 ]; then
exit 0
fi
- rm ${BUILDCHROOT_DIR}.mount
+ rm -f "${BUILDCHROOT_MOUNT_CNT}"
mountpoint -q '${BUILDCHROOT_DIR}/base-apt' && \
umount ${BUILDCHROOT_DIR}/base-apt && \
diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index 50d6408..9ccc8e3 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -28,6 +28,8 @@ export LC_ALL = "C"
MOUNT_LOCKFILE = "${ROOTFSDIR}.lock"
+ROOTFS_MOUNT_CNT = "${ROOTFSDIR}.mount"
+
rootfs_do_mounts[weight] = "3"
rootfs_do_mounts() {
sudo -s <<'EOSUDO'
@@ -35,10 +37,10 @@ rootfs_do_mounts() {
set -e
count="1"
- if [ -f '${ROOTFSDIR}.mount' ]; then
- count=$(($(cat '${ROOTFSDIR}.mount') + 1))
+ if [ -f "${ROOTFS_MOUNT_CNT}" ]; then
+ count=$(($(cat "${ROOTFS_MOUNT_CNT}") + 1))
fi
- echo $count > '${ROOTFSDIR}.mount'
+ echo $count > "${ROOTFS_MOUNT_CNT}"
if [ $count -gt 1 ]; then
exit 0
fi
@@ -79,17 +81,15 @@ rootfs_undo_mounts() {
( 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
+ count="0"
+ if [ -f "${ROOTFS_MOUNT_CNT}" ]; then
+ count=$(($(cat "${ROOTFS_MOUNT_CNT}") - 1))
+ echo $count > "${ROOTFS_MOUNT_CNT}"
fi
if [ $count -gt 0 ]; then
exit 0
fi
- rm ${ROOTFSDIR}.mount
+ rm -f "${ROOTFS_MOUNT_CNT}"
mountpoint -q '${ROOTFSDIR}/base-apt' && \
umount ${ROOTFSDIR}/base-apt && \
--
2.25.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 3/5] dpkg: Remove unmount loop
2021-07-07 16:38 [PATCH v1 0/5] Restore downstream mounts compatibility Anton Mikanovich
2021-07-07 16:38 ` [PATCH v1 1/5] Revert "dpkg: Make mount buildroot reliable" Anton Mikanovich
2021-07-07 16:38 ` [PATCH v1 2/5] mount: Allow calling unmount on not mounted paths Anton Mikanovich
@ 2021-07-07 16:38 ` Anton Mikanovich
2021-07-08 12:21 ` Jan Kiszka
2021-07-07 16:38 ` [PATCH v1 4/5] image: Add reference counter Anton Mikanovich
2021-07-07 16:38 ` [PATCH v1 5/5] events: Unmount all lost mounts at task fail Anton Mikanovich
4 siblings, 1 reply; 16+ messages in thread
From: Anton Mikanovich @ 2021-07-07 16:38 UTC (permalink / raw)
To: isar-users; +Cc: Anton Mikanovich
Remove while loop when trying to unmount WORKDIR. Any failures of
umount should be investigated to fix the root cause.
Also make dpkg_undo_mounts do not fail if dpkg_do_mounts was not
called or it was called twice.
Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
meta/classes/dpkg-base.bbclass | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 97661a6..6c84f3a 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -154,16 +154,11 @@ dpkg_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}
+ buildchroot_undo_mounts
+
+ mountpoint -q '${BUILDROOT}' && \
+ sudo umount ${BUILDROOT} && \
+ sudo rmdir ${BUILDROOT} || true
}
# Placeholder for actual dpkg_runbuild() implementation
--
2.25.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 4/5] image: Add reference counter
2021-07-07 16:38 [PATCH v1 0/5] Restore downstream mounts compatibility Anton Mikanovich
` (2 preceding siblings ...)
2021-07-07 16:38 ` [PATCH v1 3/5] dpkg: Remove unmount loop Anton Mikanovich
@ 2021-07-07 16:38 ` Anton Mikanovich
2021-07-07 16:38 ` [PATCH v1 5/5] events: Unmount all lost mounts at task fail Anton Mikanovich
4 siblings, 0 replies; 16+ messages in thread
From: Anton Mikanovich @ 2021-07-07 16:38 UTC (permalink / raw)
To: isar-users; +Cc: Anton Mikanovich
Image generation can be also made 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 5/5] events: Unmount all lost mounts at task fail
2021-07-07 16:38 [PATCH v1 0/5] Restore downstream mounts compatibility Anton Mikanovich
` (3 preceding siblings ...)
2021-07-07 16:38 ` [PATCH v1 4/5] image: Add reference counter Anton Mikanovich
@ 2021-07-07 16:38 ` Anton Mikanovich
2021-07-08 12:24 ` Jan Kiszka
4 siblings, 1 reply; 16+ messages in thread
From: Anton Mikanovich @ 2021-07-07 16:38 UTC (permalink / raw)
To: isar-users; +Cc: Anton Mikanovich
Perform mounts cleanup in case task fail.
Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
meta/classes/isar-events.bbclass | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
index 73419b4..50f5cb3 100644
--- a/meta/classes/isar-events.bbclass
+++ b/meta/classes/isar-events.bbclass
@@ -4,6 +4,25 @@
# Copyright (C) 2015-2017 ilbers GmbH
# Copyright (c) Siemens AG, 2018
+def is_mounted(d, mountpoint, subpath=None):
+ path = d.getVar(mountpoint, True)
+ if path:
+ if subpath:
+ path = os.path.join(path, subpath)
+ path = os.path.realpath(path)
+ with open('/proc/mounts') as f:
+ for line in f.readlines():
+ if path in line:
+ return True
+ return False
+
+def safe_exec(func, d):
+ try:
+ if d.getVar(func, False):
+ bb.build.exec_func(func, d)
+ except Exception:
+ pass
+
addhandler build_started
python build_started() {
@@ -35,6 +54,14 @@ python task_failed() {
# Avoid false positives if a second target depends on this task and retries
# the execution after the first failure.
os.remove(task_once_stamp(d))
+
+ # Cleanup possible lost mounts
+ if is_mounted(d, 'BUILDROOT_DEPLOY'):
+ safe_exec("image_undo_mounts", d)
+ if is_mounted(d, 'BUILDROOT'):
+ safe_exec("dpkg_undo_mounts", d)
+ if is_mounted(d, 'ROOTFSDIR', 'dev'):
+ safe_exec("rootfs_undo_mounts", d)
}
task_failed[eventmask] = "bb.build.TaskFailed"
--
2.25.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 1/5] Revert "dpkg: Make mount buildroot reliable"
2021-07-07 16:38 ` [PATCH v1 1/5] Revert "dpkg: Make mount buildroot reliable" Anton Mikanovich
@ 2021-07-08 12:16 ` Jan Kiszka
0 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2021-07-08 12:16 UTC (permalink / raw)
To: Anton Mikanovich, isar-users
On 07.07.21 18:38, Anton Mikanovich wrote:
> This reverts commit d21d49578e5a3b0019075d1946bd93a95914fcca.
Signed-off and a short reasoning is missing.
Jan
> ---
> 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 ed162b3..97661a6 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,25 +76,21 @@ 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
> do_apt_fetch[lockfiles] += "${REPO_ISAR_DIR}/isar.lock"
>
> -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 ' \
> @@ -100,25 +101,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
> @@ -162,38 +146,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() {
> @@ -203,12 +174,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
> @@ -252,16 +221,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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/5] mount: Allow calling unmount on not mounted paths
2021-07-07 16:38 ` [PATCH v1 2/5] mount: Allow calling unmount on not mounted paths Anton Mikanovich
@ 2021-07-08 12:17 ` Jan Kiszka
0 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2021-07-08 12:17 UTC (permalink / raw)
To: Anton Mikanovich, isar-users
On 07.07.21 18:38, Anton Mikanovich wrote:
> Make buildchroot_undo_mounts and rootfs_undo_mounts functions do not
> fail if calling without buildchroot_do_mounts and rootfs_do_mounts.
>
"Why" is missing, you only have the "what" here.
Jan
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
> meta/classes/buildchroot.bbclass | 20 ++++++++++----------
> meta/classes/rootfs.bbclass | 20 ++++++++++----------
> 2 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/meta/classes/buildchroot.bbclass b/meta/classes/buildchroot.bbclass
> index 806a29f..9c27711 100644
> --- a/meta/classes/buildchroot.bbclass
> +++ b/meta/classes/buildchroot.bbclass
> @@ -23,16 +23,18 @@ python __anonymous() {
>
> MOUNT_LOCKFILE = "${BUILDCHROOT_DIR}.lock"
>
> +BUILDCHROOT_MOUNT_CNT = "${BUILDCHROOT_DIR}.mount"
> +
> buildchroot_do_mounts() {
> sudo -s <<'EOSUDO'
> ( flock 9
> set -e
>
> count="1"
> - if [ -f '${BUILDCHROOT_DIR}.mount' ]; then
> - count=$(($(cat '${BUILDCHROOT_DIR}.mount') + 1))
> + if [ -f "${BUILDCHROOT_MOUNT_CNT}" ]; then
> + count=$(($(cat "${BUILDCHROOT_MOUNT_CNT}") + 1))
> fi
> - echo $count > '${BUILDCHROOT_DIR}.mount'
> + echo $count > "${BUILDCHROOT_MOUNT_CNT}"
> if [ $count -gt 1 ]; then
> exit 0
> fi
> @@ -77,17 +79,15 @@ buildchroot_undo_mounts() {
> ( 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
> + count="0"
> + if [ -f "${BUILDCHROOT_MOUNT_CNT}" ]; then
> + count=$(($(cat "${BUILDCHROOT_MOUNT_CNT}") - 1))
> + echo $count > "${BUILDCHROOT_MOUNT_CNT}"
> fi
> if [ $count -gt 0 ]; then
> exit 0
> fi
> - rm ${BUILDCHROOT_DIR}.mount
> + rm -f "${BUILDCHROOT_MOUNT_CNT}"
>
> mountpoint -q '${BUILDCHROOT_DIR}/base-apt' && \
> umount ${BUILDCHROOT_DIR}/base-apt && \
> diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
> index 50d6408..9ccc8e3 100644
> --- a/meta/classes/rootfs.bbclass
> +++ b/meta/classes/rootfs.bbclass
> @@ -28,6 +28,8 @@ export LC_ALL = "C"
>
> MOUNT_LOCKFILE = "${ROOTFSDIR}.lock"
>
> +ROOTFS_MOUNT_CNT = "${ROOTFSDIR}.mount"
> +
> rootfs_do_mounts[weight] = "3"
> rootfs_do_mounts() {
> sudo -s <<'EOSUDO'
> @@ -35,10 +37,10 @@ rootfs_do_mounts() {
> set -e
>
> count="1"
> - if [ -f '${ROOTFSDIR}.mount' ]; then
> - count=$(($(cat '${ROOTFSDIR}.mount') + 1))
> + if [ -f "${ROOTFS_MOUNT_CNT}" ]; then
> + count=$(($(cat "${ROOTFS_MOUNT_CNT}") + 1))
> fi
> - echo $count > '${ROOTFSDIR}.mount'
> + echo $count > "${ROOTFS_MOUNT_CNT}"
> if [ $count -gt 1 ]; then
> exit 0
> fi
> @@ -79,17 +81,15 @@ rootfs_undo_mounts() {
> ( 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
> + count="0"
> + if [ -f "${ROOTFS_MOUNT_CNT}" ]; then
> + count=$(($(cat "${ROOTFS_MOUNT_CNT}") - 1))
> + echo $count > "${ROOTFS_MOUNT_CNT}"
> fi
> if [ $count -gt 0 ]; then
> exit 0
> fi
> - rm ${ROOTFSDIR}.mount
> + rm -f "${ROOTFS_MOUNT_CNT}"
>
> mountpoint -q '${ROOTFSDIR}/base-apt' && \
> umount ${ROOTFSDIR}/base-apt && \
>
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/5] dpkg: Remove unmount loop
2021-07-07 16:38 ` [PATCH v1 3/5] dpkg: Remove unmount loop Anton Mikanovich
@ 2021-07-08 12:21 ` Jan Kiszka
2021-07-08 14:35 ` Anton Mikanovich
2021-08-17 13:02 ` Anton Mikanovich
0 siblings, 2 replies; 16+ messages in thread
From: Jan Kiszka @ 2021-07-08 12:21 UTC (permalink / raw)
To: Anton Mikanovich, isar-users
On 07.07.21 18:38, Anton Mikanovich wrote:
> Remove while loop when trying to unmount WORKDIR. Any failures of
> umount should be investigated to fix the root cause.
> Also make dpkg_undo_mounts do not fail if dpkg_do_mounts was not
> called or it was called twice.
>
This is very likely wrong: We know from the past (should be in the git
history, e.g 17d0842d) that not all subprocesses started inside the
chroot may have been terminated when we left it. That can cause
significant delays between returning from chroot and finding non-busy
mount points to complete all umounts. Now, if you silently fail umounts,
you may find inconsistently mounted chroots on the next recipe that
tries to remount them. Big fat warning sign...!
Jan
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
> meta/classes/dpkg-base.bbclass | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
> index 97661a6..6c84f3a 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -154,16 +154,11 @@ dpkg_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}
> + buildchroot_undo_mounts
> +
> + mountpoint -q '${BUILDROOT}' && \
> + sudo umount ${BUILDROOT} && \
> + sudo rmdir ${BUILDROOT} || true
> }
>
> # Placeholder for actual dpkg_runbuild() implementation
>
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 5/5] events: Unmount all lost mounts at task fail
2021-07-07 16:38 ` [PATCH v1 5/5] events: Unmount all lost mounts at task fail Anton Mikanovich
@ 2021-07-08 12:24 ` Jan Kiszka
2021-07-08 14:41 ` Anton Mikanovich
0 siblings, 1 reply; 16+ messages in thread
From: Jan Kiszka @ 2021-07-08 12:24 UTC (permalink / raw)
To: Anton Mikanovich, isar-users
On 07.07.21 18:38, Anton Mikanovich wrote:
> Perform mounts cleanup in case task fail.
>
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
> meta/classes/isar-events.bbclass | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
> index 73419b4..50f5cb3 100644
> --- a/meta/classes/isar-events.bbclass
> +++ b/meta/classes/isar-events.bbclass
> @@ -4,6 +4,25 @@
> # Copyright (C) 2015-2017 ilbers GmbH
> # Copyright (c) Siemens AG, 2018
>
> +def is_mounted(d, mountpoint, subpath=None):
> + path = d.getVar(mountpoint, True)
> + if path:
> + if subpath:
> + path = os.path.join(path, subpath)
> + path = os.path.realpath(path)
> + with open('/proc/mounts') as f:
> + for line in f.readlines():
> + if path in line:
> + return True
> + return False
> +
> +def safe_exec(func, d):
> + try:
> + if d.getVar(func, False):
> + bb.build.exec_func(func, d)
> + except Exception:
> + pass
> +
> addhandler build_started
>
> python build_started() {
> @@ -35,6 +54,14 @@ python task_failed() {
> # Avoid false positives if a second target depends on this task and retries
> # the execution after the first failure.
> os.remove(task_once_stamp(d))
> +
> + # Cleanup possible lost mounts
> + if is_mounted(d, 'BUILDROOT_DEPLOY'):
> + safe_exec("image_undo_mounts", d)
> + if is_mounted(d, 'BUILDROOT'):
> + safe_exec("dpkg_undo_mounts", d)
> + if is_mounted(d, 'ROOTFSDIR', 'dev'):
> + safe_exec("rootfs_undo_mounts", d)
> }
> task_failed[eventmask] = "bb.build.TaskFailed"
>
>
No looping, so everything you miss here will continue to be cleaned up
by build_completed() - so what's the point of these changes?
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/5] dpkg: Remove unmount loop
2021-07-08 12:21 ` Jan Kiszka
@ 2021-07-08 14:35 ` Anton Mikanovich
2021-07-08 16:16 ` Jan Kiszka
2021-08-17 13:02 ` Anton Mikanovich
1 sibling, 1 reply; 16+ messages in thread
From: Anton Mikanovich @ 2021-07-08 14:35 UTC (permalink / raw)
To: Jan Kiszka, isar-users
08.07.2021 15:21, Jan Kiszka wrote:
> This is very likely wrong: We know from the past (should be in the git
> history, e.g 17d0842d) that not all subprocesses started inside the
> chroot may have been terminated when we left it. That can cause
> significant delays between returning from chroot and finding non-busy
> mount points to complete all umounts. Now, if you silently fail umounts,
> you may find inconsistently mounted chroots on the next recipe that
> tries to remount them. Big fat warning sign...!
>
> Jan
Is that really correct to left background processes running from chroot?
Even if it is, we need to control it not to let it run forever.
Not sure how to deal with that. Set maximum run time maybe?
--
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 5/5] events: Unmount all lost mounts at task fail
2021-07-08 12:24 ` Jan Kiszka
@ 2021-07-08 14:41 ` Anton Mikanovich
2021-07-08 16:28 ` Jan Kiszka
0 siblings, 1 reply; 16+ messages in thread
From: Anton Mikanovich @ 2021-07-08 14:41 UTC (permalink / raw)
To: Jan Kiszka, isar-users
08.07.2021 15:24, Jan Kiszka wrote:
> No looping, so everything you miss here will continue to be cleaned up
> by build_completed() - so what's the point of these changes?
>
> Jan
>
This is where WORKDIR will be unmounted to prevent double mount.
It in necessary to fix the original issue fixed by
d21d49578e5a3b0019075d1946bd93a95914fcca and explained in previous mails.
--
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/5] dpkg: Remove unmount loop
2021-07-08 14:35 ` Anton Mikanovich
@ 2021-07-08 16:16 ` Jan Kiszka
0 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2021-07-08 16:16 UTC (permalink / raw)
To: Anton Mikanovich, isar-users
On 08.07.21 16:35, Anton Mikanovich wrote:
> 08.07.2021 15:21, Jan Kiszka wrote:
>> This is very likely wrong: We know from the past (should be in the git
>> history, e.g 17d0842d) that not all subprocesses started inside the
>> chroot may have been terminated when we left it. That can cause
>> significant delays between returning from chroot and finding non-busy
>> mount points to complete all umounts. Now, if you silently fail umounts,
>> you may find inconsistently mounted chroots on the next recipe that
>> tries to remount them. Big fat warning sign...!
>>
>> Jan
>
> Is that really correct to left background processes running from chroot?
> Even if it is, we need to control it not to let it run forever.
> Not sure how to deal with that. Set maximum run time maybe?
>
See the commit I cited on how these issues were reproducible. The only
way to get rid of all pending processes - besides waiting for them - was
and likely still is putting them in containers.
If you can confirm that those issues are gone and explain, why, we can
obviously change the umounting scheme. But none of the patches so far
acknowledge these issues, thus have a high potential to kicking us back
to the point before the current lazy umounting scheme.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 5/5] events: Unmount all lost mounts at task fail
2021-07-08 14:41 ` Anton Mikanovich
@ 2021-07-08 16:28 ` Jan Kiszka
2021-07-12 16:08 ` Baurzhan Ismagulov
0 siblings, 1 reply; 16+ messages in thread
From: Jan Kiszka @ 2021-07-08 16:28 UTC (permalink / raw)
To: Anton Mikanovich, isar-users
On 08.07.21 16:41, Anton Mikanovich wrote:
> 08.07.2021 15:24, Jan Kiszka wrote:
>> No looping, so everything you miss here will continue to be cleaned up
>> by build_completed() - so what's the point of these changes?
>>
>> Jan
>>
> This is where WORKDIR will be unmounted to prevent double mount.
> It in necessary to fix the original issue fixed by
> d21d49578e5a3b0019075d1946bd93a95914fcca and explained in previous mails.
>
I do get the idea of a perfect world where a single attempt to umount
works and cleanly pairs with the regular umount when the task succeeds.
The problem is that I cannot see that this has been validate to truly
catch the corner cases that the lazy, late-triggered scheme addresses so
far. If it does not handle that, these umounts are pointless. If it
does, we can drop the build_completed() handler. Having both indicates
no progress, just more complexity.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 5/5] events: Unmount all lost mounts at task fail
2021-07-08 16:28 ` Jan Kiszka
@ 2021-07-12 16:08 ` Baurzhan Ismagulov
0 siblings, 0 replies; 16+ messages in thread
From: Baurzhan Ismagulov @ 2021-07-12 16:08 UTC (permalink / raw)
To: isar-users; +Cc: Anton Mikanovich, Jan Kiszka
On Thu, Jul 08, 2021 at 06:28:38PM +0200, Jan Kiszka wrote:
> >> No looping, so everything you miss here will continue to be cleaned up
> >> by build_completed() - so what's the point of these changes?
...
> I do get the idea of a perfect world where a single attempt to umount
> works and cleanly pairs with the regular umount when the task succeeds.
> The problem is that I cannot see that this has been validate to truly
> catch the corner cases that the lazy, late-triggered scheme addresses so
> far. If it does not handle that, these umounts are pointless. If it
> does, we can drop the build_completed() handler. Having both indicates
> no progress, just more complexity.
Thanks Jan for the review and the use cases,
I agree that some patches could be refined and better documented.
We'll work on the topics you've raised and respond. I'm afraid we'd need your
help regarding reproducing and analysis of the old issues, because in general,
many issues so far have been rarely reproducible across different build hosts.
With kind regards,
Baurzhan.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/5] dpkg: Remove unmount loop
2021-07-08 12:21 ` Jan Kiszka
2021-07-08 14:35 ` Anton Mikanovich
@ 2021-08-17 13:02 ` Anton Mikanovich
1 sibling, 0 replies; 16+ messages in thread
From: Anton Mikanovich @ 2021-08-17 13:02 UTC (permalink / raw)
To: Jan Kiszka, isar-users
08.07.2021 15:21, Jan Kiszka wrote:
> On 07.07.21 18:38, Anton Mikanovich wrote:
>> Remove while loop when trying to unmount WORKDIR. Any failures of
>> umount should be investigated to fix the root cause.
>> Also make dpkg_undo_mounts do not fail if dpkg_do_mounts was not
>> called or it was called twice.
>>
> This is very likely wrong: We know from the past (should be in the git
> history, e.g 17d0842d) that not all subprocesses started inside the
> chroot may have been terminated when we left it. That can cause
> significant delays between returning from chroot and finding non-busy
> mount points to complete all umounts. Now, if you silently fail umounts,
> you may find inconsistently mounted chroots on the next recipe that
> tries to remount them. Big fat warning sign...!
>
> Jan
We've reproduced the issue fixed by 17d0842d on the old code (pre-17d0842d).
The issue was caused by unmounting buildchroot mounts without care of
their usage.
The latest v5 patchset is not affected, because there is no global task
fail handling, and per-task unmount is protected by the reference counting.
Moreover, buildchroot_undo_mounts was moved after the while loop in
dpkg_undo_mounts in v5, so it will not fail immediately like we had it
with '/sys/fs/cgroups is busy' failures (the real fix of
'/sys/fs/cgroups is busy' is still in progress).
--
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
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-08-17 13:02 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07 16:38 [PATCH v1 0/5] Restore downstream mounts compatibility Anton Mikanovich
2021-07-07 16:38 ` [PATCH v1 1/5] Revert "dpkg: Make mount buildroot reliable" Anton Mikanovich
2021-07-08 12:16 ` Jan Kiszka
2021-07-07 16:38 ` [PATCH v1 2/5] mount: Allow calling unmount on not mounted paths Anton Mikanovich
2021-07-08 12:17 ` Jan Kiszka
2021-07-07 16:38 ` [PATCH v1 3/5] dpkg: Remove unmount loop Anton Mikanovich
2021-07-08 12:21 ` Jan Kiszka
2021-07-08 14:35 ` Anton Mikanovich
2021-07-08 16:16 ` Jan Kiszka
2021-08-17 13:02 ` Anton Mikanovich
2021-07-07 16:38 ` [PATCH v1 4/5] image: Add reference counter Anton Mikanovich
2021-07-07 16:38 ` [PATCH v1 5/5] events: Unmount all lost mounts at task fail Anton Mikanovich
2021-07-08 12:24 ` Jan Kiszka
2021-07-08 14:41 ` Anton Mikanovich
2021-07-08 16:28 ` Jan Kiszka
2021-07-12 16:08 ` Baurzhan Ismagulov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox