[PATCH v3 0/8] Hanging mount fixes

[PATCH v3 0/8] Hanging mount fixes

[Anton Mikanovich]

Consolidate the patches to address the hanging mount issues.

Addresses the following issues:

 1. Cleanup handler is called multiple times, unmounting any hanging
    mounts.

 2. Hanging mount warnings are discarded in the current setup.

 3. Incorrect asymmetric mount-umount logic inherited from buildchroot
    times.

In this series, cleanup is done in reverse /proc/mounts order to
unmount e.g. /dev/shm before /dev. This happens to work for us today
but we could perform stricter semantic ordering if necessary.

ISAR_FAIL_ON_CLEANUP=1 can be set in the environment to fail builds if
mounts are left behind. We will use this for Isar CI. The default is 0.

p1 comes from 'Additional CI improvements' series.
Patches p4-p6 come from v2 of 'Start to address umount problems' series
by Jan.
p7 comes from 'Fix leftover mounts in rootfs.bbclass' by Florian.
p2, p3 and p8 are new one.

Anton Mikanovich (4):
  CI: Do not lose output on bitbake / qemu exit
  isar-events: Unhide mounts left behind
  CI: Pass ISAR_FAIL_ON_CLEANUP from environment to bitbake
  image: Do not call rootfs_do_umounts twice

Florian Bezdeka (1):
  rootfs: Add missing umounts in rootfs_postprocess() and
    rootfs_install()

Jan Kiszka (3):
  image: Avoid breaking the build when mounts are no longer present on
    umount
  rootfs: Provide rootfs_do_umounts
  initramfs: Add missing umounts after generation

 RECIPE-API-CHANGELOG.md          | 15 ++++++++
 meta/classes/image.bbclass       | 19 ----------
 meta/classes/initramfs.bbclass   |  2 +
 meta/classes/isar-events.bbclass | 40 ++++++++++++++++----
 meta/classes/rootfs.bbclass      | 63 ++++++++++++++++++++++++++------
 testsuite/cibuilder.py           | 13 +++++--
 6 files changed, 110 insertions(+), 42 deletions(-)

-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20241011100050.322686-1-amikan%40ilbers.de.

[PATCH v3 1/8] CI: Do not lose output on bitbake / qemu exit

[Anton Mikanovich]

If bitbake or qemu exit before we completely read their stdout/stderr,
messages are lost. With this change, we read the subprocess output
before terminating ourselves.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 testsuite/cibuilder.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/testsuite/cibuilder.py b/testsuite/cibuilder.py
index 4a605f50..0d0de99e 100755
--- a/testsuite/cibuilder.py
+++ b/testsuite/cibuilder.py
@@ -257,7 +257,7 @@ class CIBuilder(Test):
             poller = select.poll()
             poller.register(p1.stdout, select.POLLIN)
             poller.register(p1.stderr, select.POLLIN)
-            while p1.poll() is None:
+            while True:
                 events = poller.poll(1000)
                 for fd, event in events:
                     if event != select.POLLIN:
@@ -266,6 +266,8 @@ class CIBuilder(Test):
                         self.log.info(p1.stdout.readline().rstrip())
                     if fd == p1.stderr.fileno():
                         app_log.error(p1.stderr.readline().rstrip())
+                if p1.poll() is not None:
+                    break
             p1.wait()
             if p1.returncode:
                 self.fail("Bitbake failed")
@@ -533,7 +535,7 @@ class CIBuilder(Test):
         databuf = bytearray(b'')
         databuf_size = 1024 * 2 + len(login_prompt)
 
-        while time.time() < timeout and p1.poll() is None:
+        while time.time() < timeout:
             events = poller.poll(1000 * (timeout - time.time()))
             for fd, event in events:
                 if event != select.POLLIN:
@@ -547,6 +549,8 @@ class CIBuilder(Test):
                         return 0
                 if fd == p1.stderr.fileno():
                     app_log.error(p1.stderr.readline().rstrip())
+            if p1.poll() is not None:
+                break
 
         self.log.error("Didn't get login prompt")
         return 1
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20241011100050.322686-2-amikan%40ilbers.de.

[PATCH v3 2/8] isar-events: Unhide mounts left behind

[Anton Mikanovich]

Fail the existing build testcases on mounts left behind by calling the
cleanup handler once per build (controlled by a new variable).

Specifically:

1. Execute cleanup handler only once per build.
2. Output error messages in case of umount failure.
3. Unmount /proc/mounts in reverse order to unmount subdirectories first.
4. Introduce ISAR_FAIL_ON_CLEANUP bitbake variable:
   - 0 or unset: Output a warning, unmount, build succeeds (default).
   - 1: Output a warning, keep mounts left behind, build fails.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 RECIPE-API-CHANGELOG.md          |  9 +++++++
 meta/classes/isar-events.bbclass | 40 +++++++++++++++++++++++++-------
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/RECIPE-API-CHANGELOG.md b/RECIPE-API-CHANGELOG.md
index 8eeaf325..608d0cc3 100644
--- a/RECIPE-API-CHANGELOG.md
+++ b/RECIPE-API-CHANGELOG.md
@@ -665,3 +665,12 @@ package on an `amd64` host: you would expect the -native variant to produce
 an `amd64` package and -compat an 'armhf` package: it will however remain
 `arm64` and build of dependent recipes (image or dpkg) may fail because of
 the architecture mismatch.
+
+### Changes in cleanup handler
+
+Bitbake BuildCompleted event handler is now executed only once per build and
+always outputs a warning if mounts are left behind after the build.
+
+Bitbake exit status depends on ISAR_FAIL_ON_CLEANUP bitbake variable:
+ - 0 or unset: Output a warning, unmount, build succeeds (default).
+ - 1: Output a warning, keep mounts left behind, build fails.
diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
index f5061a8b..76b713c7 100644
--- a/meta/classes/isar-events.bbclass
+++ b/meta/classes/isar-events.bbclass
@@ -4,6 +4,10 @@
 # Copyright (C) 2015-2017 ilbers GmbH
 # Copyright (c) Siemens AG, 2018
 
+# If set to 1, the build will fail on mounts found during cleanup,
+# keeping those mounts left behind
+ISAR_FAIL_ON_CLEANUP ?= "0"
+
 addhandler build_started
 
 python build_started() {
@@ -48,17 +52,37 @@ python build_completed() {
     if not tmpdir:
         return
 
+    # bitbake calls cleanup for every multiconfig listed in BBMULTICONFIG plus
+    # one for the entire build. E.g., if BBMULTICONFIG="mc1 mc2 mc3", we call
+    # "bitbake mc1 mc2", the following cleanups would be called:
+    # "c1 c2 c3 cdefault".
+    # Skip running cleanup for additional multiconfigs
+    mc = d.getVar('BB_CURRENT_MC')
+    if mc != 'default':
+        return
+
+    fail_on_cleanup = bb.utils.to_boolean(d.getVar('ISAR_FAIL_ON_CLEANUP'))
+
     basepath = tmpdir + '/work/'
 
-    with open('/proc/mounts') as f:
-        for line in f.readlines():
-            if basepath in line:
-                bb.debug(1, '%s left mounted, unmounting...' % line.split()[1])
-                subprocess.call(
-                    ["sudo", "umount", line.split()[1]],
-                    stdout=subprocess.DEVNULL,
-                    stderr=subprocess.DEVNULL,
+    for line in reversed(list(open('/proc/mounts'))):
+        if basepath not in line:
+            continue
+        msg_line = f"{line.split()[1]} left mounted"
+        # If bitbake is started manually, bb.warn and bb.error go to stdout;
+        # with bb.error, bitbake additionally fails the build. Under CI,
+        # bb.warn and bb.error currently go to a file.
+        if fail_on_cleanup:
+            bb.error(msg_line)
+        else:
+            msg_line += ', unmounting...'
+            bb.warn(msg_line)
+            try:
+                subprocess.run(
+                    f"sudo umount {line.split()[1]}", shell=True, check=True
                 )
+            except subprocess.CalledProcessError as e:
+                bb.error(str(e))
 
     # Cleanup build UUID, the next bitbake run will generate new one
     bb.persist_data.persist('BB_ISAR_UUID_DATA', d).clear()
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20241011100050.322686-3-amikan%40ilbers.de.

[PATCH v3 3/8] CI: Pass ISAR_FAIL_ON_CLEANUP from environment to bitbake

[Anton Mikanovich]

Setting the variable to 1 in the CI environment would make the job
fail if any mounts are left behind after building.

To test:

$ ISAR_FAIL_ON_CLEANUP=1 ../scripts/ci_build.sh -T dev

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 testsuite/cibuilder.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/testsuite/cibuilder.py b/testsuite/cibuilder.py
index 0d0de99e..9fa3d86c 100755
--- a/testsuite/cibuilder.py
+++ b/testsuite/cibuilder.py
@@ -129,6 +129,7 @@ class CIBuilder(Test):
 
         # get parameters from environment
         distro_apt_premir = os.getenv('DISTRO_APT_PREMIRRORS')
+        fail_on_cleanup = os.getenv('ISAR_FAIL_ON_CLEANUP')
 
         self.log.info(
             f"===================================================\n"
@@ -204,7 +205,9 @@ class CIBuilder(Test):
             if sstate_dir:
                 f.write('SSTATE_DIR = "%s"\n' % sstate_dir)
             if image_install is not None:
-                f.write('IMAGE_INSTALL = "%s"' % image_install)
+                f.write('IMAGE_INSTALL = "%s"\n' % image_install)
+            if fail_on_cleanup == '1':
+                f.write('ISAR_FAIL_ON_CLEANUP = "1"\n')
 
         # include ci_build.conf in local.conf
         with open(self.build_dir + '/conf/local.conf', 'r+') as f:
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20241011100050.322686-4-amikan%40ilbers.de.

[PATCH v3 4/8] image: Avoid breaking the build when mounts are no longer present on umount

[Anton Mikanovich]

From: Jan Kiszka <jan.kiszka@siemens.com>

This does not seem to trigger yet because we always have those
mountpoints present. But if that is no longer the case, we will bail out
when mountpoint fails due to the set -e.

Fixes: 165519a7b314 ("sudo: Fail on the first error")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 meta/classes/image.bbclass | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 0c162aa3..9d5b782a 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -418,24 +418,31 @@ do_rootfs_finalize() {
                 -maxdepth 1 -name 'qemu-*-static' -type f -delete
         fi
 
-        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
-            umount '${ROOTFSDIR}/isar-apt' && \
+        if mountpoint -q '${ROOTFSDIR}/isar-apt'; then
+            umount '${ROOTFSDIR}/isar-apt'
             rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
+        fi
 
-        mountpoint -q '${ROOTFSDIR}/base-apt' && \
-            umount '${ROOTFSDIR}/base-apt' && \
+        if mountpoint -q '${ROOTFSDIR}/base-apt'; then
+            umount '${ROOTFSDIR}/base-apt'
             rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
+        fi
 
-        mountpoint -q '${ROOTFSDIR}/dev/pts' && \
+        if mountpoint -q '${ROOTFSDIR}/dev/pts'; then
             umount '${ROOTFSDIR}/dev/pts'
-        mountpoint -q '${ROOTFSDIR}/dev/shm' && \
+        fi
+        if mountpoint -q '${ROOTFSDIR}/dev/shm'; then
             umount '${ROOTFSDIR}/dev/shm'
-        mountpoint -q '${ROOTFSDIR}/dev' && \
+        fi
+        if mountpoint -q '${ROOTFSDIR}/dev'; then
             umount '${ROOTFSDIR}/dev'
-        mountpoint -q '${ROOTFSDIR}/proc' && \
+        fi
+        if mountpoint -q '${ROOTFSDIR}/proc'; then
             umount '${ROOTFSDIR}/proc'
-        mountpoint -q '${ROOTFSDIR}/sys' && \
+        fi
+        if mountpoint -q '${ROOTFSDIR}/sys'; then
             umount '${ROOTFSDIR}/sys'
+        fi
 
         if [ -e "${ROOTFSDIR}/etc/apt/sources-list" ]; then
             mv "${ROOTFSDIR}/etc/apt/sources-list" \
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20241011100050.322686-5-amikan%40ilbers.de.

[PATCH v3 5/8] rootfs: Provide rootfs_do_umounts

[Anton Mikanovich]

From: Jan Kiszka <jan.kiszka@siemens.com>

This will be used more frequently soon to avoid dangling mounts.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 meta/classes/image.bbclass  | 28 ++--------------------------
 meta/classes/rootfs.bbclass | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 9d5b782a..1eb974e8 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -405,6 +405,8 @@ def apt_list_files(d):
 IMAGE_LISTS = "${@ ' '.join(apt_list_files(d)) }"
 
 do_rootfs_finalize() {
+    rootfs_do_umounts
+
     sudo -s <<'EOSUDO'
         set -e
 
@@ -418,32 +420,6 @@ do_rootfs_finalize() {
                 -maxdepth 1 -name 'qemu-*-static' -type f -delete
         fi
 
-        if mountpoint -q '${ROOTFSDIR}/isar-apt'; then
-            umount '${ROOTFSDIR}/isar-apt'
-            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
-        fi
-
-        if mountpoint -q '${ROOTFSDIR}/base-apt'; then
-            umount '${ROOTFSDIR}/base-apt'
-            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
-        fi
-
-        if mountpoint -q '${ROOTFSDIR}/dev/pts'; then
-            umount '${ROOTFSDIR}/dev/pts'
-        fi
-        if mountpoint -q '${ROOTFSDIR}/dev/shm'; then
-            umount '${ROOTFSDIR}/dev/shm'
-        fi
-        if mountpoint -q '${ROOTFSDIR}/dev'; then
-            umount '${ROOTFSDIR}/dev'
-        fi
-        if mountpoint -q '${ROOTFSDIR}/proc'; then
-            umount '${ROOTFSDIR}/proc'
-        fi
-        if mountpoint -q '${ROOTFSDIR}/sys'; then
-            umount '${ROOTFSDIR}/sys'
-        fi
-
         if [ -e "${ROOTFSDIR}/etc/apt/sources-list" ]; then
             mv "${ROOTFSDIR}/etc/apt/sources-list" \
                 "${ROOTFSDIR}/etc/apt/sources.list.d/bootstrap.list"
diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index f0abd795..ef40cbdf 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -65,6 +65,38 @@ rootfs_do_mounts() {
 EOSUDO
 }
 
+rootfs_do_umounts() {
+    sudo -s <<'EOSUDO'
+        set -e
+        if mountpoint -q '${ROOTFSDIR}/isar-apt'; then
+            umount '${ROOTFSDIR}/isar-apt'
+            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
+        fi
+
+        if mountpoint -q '${ROOTFSDIR}/base-apt'; then
+            umount '${ROOTFSDIR}/base-apt'
+            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
+        fi
+
+        if mountpoint -q '${ROOTFSDIR}/dev/pts'; then
+            umount '${ROOTFSDIR}/dev/pts'
+        fi
+        if mountpoint -q '${ROOTFSDIR}/dev/shm'; then
+            umount '${ROOTFSDIR}/dev/shm'
+        fi
+        if mountpoint -q '${ROOTFSDIR}/dev'; then
+            umount '${ROOTFSDIR}/dev'
+        fi
+        if mountpoint -q '${ROOTFSDIR}/proc'; then
+            umount '${ROOTFSDIR}/proc'
+        fi
+        if mountpoint -q '${ROOTFSDIR}/sys'; then
+            umount '${ROOTFSDIR}/sys'
+        fi
+
+EOSUDO
+}
+
 rootfs_do_qemu() {
     if [ '${@repr(d.getVar('ROOTFS_ARCH') == d.getVar('HOST_ARCH'))}' = 'False' ]
     then
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20241011100050.322686-6-amikan%40ilbers.de.

[PATCH v3 6/8] initramfs: Add missing umounts after generation

[Anton Mikanovich]

From: Jan Kiszka <jan.kiszka@siemens.com>

Failing to unmount what was mounted via rootfs_do_mounts can cause
troubles on rebuilds.

To reproduce:

. ./isar-init-build-env newbuild
mount >01.txt
cat >conf/local.conf <<EOF
MACHINE = "qemuamd64"
DISTRO = "debian-bookworm"
DISTRO_ARCH = "amd64"
EOF
bitbake isar-initramfs
mount >02.txt
diff -Naurp 01.txt 02.txt
+tmpfs on newbuild/tmp/work/debian-bookworm-amd64/sbuild-chroot-target/1.0-r0/rootfs/dev type tmpfs (rw,nosuid,size=4096k,nr_inodes=65536,mode=755,inode64)
+tmpfs on newbuild/tmp/work/debian-bookworm-amd64/isar-initramfs-qemuamd64/1.0-r0/rootfs/dev type tmpfs (rw,nosuid,size=4096k,nr_inodes=65536,mode=755,inode64)

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/initramfs.bbclass | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/classes/initramfs.bbclass b/meta/classes/initramfs.bbclass
index 6886b95a..42013356 100644
--- a/meta/classes/initramfs.bbclass
+++ b/meta/classes/initramfs.bbclass
@@ -45,6 +45,8 @@ do_generate_initramfs() {
           update-initramfs -u -v ;  \
         fi'
 
+    rootfs_do_umounts
+
     if [ ! -e "${INITRAMFS_ROOTFS}/initrd.img" ]; then
         bberror "No initramfs was found after generation!"
     fi
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20241011100050.322686-7-amikan%40ilbers.de.

[PATCH v3 7/8] rootfs: Add missing umounts in rootfs_postprocess() and rootfs_install()

[Anton Mikanovich]

From: Florian Bezdeka <florian.bezdeka@siemens.com>

Calls to rootfs_do_mounts should always be paired with calls to
rootfs_do_umounts. With this change, it is done in do_rootfs_install
and do_rootfs_postprocess.

In case there was an exception thrown within the try blocks they will be
re-raised after the finally block has been processed. This way we try to
avoid leaking mounts but unmounting might still fail. In any case we
tried our best to clean up.

To reproduce:

. ./isar-init-build-env newbuild
mount >01.txt
cat >conf/local.conf <<EOF
MACHINE = "qemuamd64"
DISTRO = "debian-bookworm"
DISTRO_ARCH = "amd64"
EOF
bitbake sbuild-chroot-target
mount >02.txt
diff -Naurp 01.txt 02.txt
+tmpfs on newbuild/tmp/work/debian-bookworm-amd64/sbuild-chroot-target/1.0-r0/rootfs/dev type tmpfs (rw,nosuid,size=4096k,nr_inodes=65536,mode=755,inode64)

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/rootfs.bbclass | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index ef40cbdf..c7011508 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -248,18 +248,21 @@ python do_rootfs_install() {
     progress_reporter = bb.progress.MultiStageProgressReporter(d, stage_weights)
     d.rootfs_progress = progress_reporter
 
-    for cmd in cmds:
-        progress_reporter.next_stage()
+    try:
+        for cmd in cmds:
+            progress_reporter.next_stage()
 
-        if (d.getVarFlag(cmd, 'isar-apt-lock') or "") == "acquire-before":
-            lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock",
-                                     shared=True)
+            if (d.getVarFlag(cmd, 'isar-apt-lock') or "") == "acquire-before":
+                lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock",
+                                         shared=True)
 
-        bb.build.exec_func(cmd, d)
+            bb.build.exec_func(cmd, d)
 
-        if (d.getVarFlag(cmd, 'isar-apt-lock') or "") == "release-after":
-            bb.utils.unlockfile(lock)
-    progress_reporter.finish()
+            if (d.getVarFlag(cmd, 'isar-apt-lock') or "") == "release-after":
+                bb.utils.unlockfile(lock)
+            progress_reporter.finish()
+    finally:
+        bb.build.exec_func('rootfs_do_umounts', d)
 }
 addtask rootfs_install before do_rootfs_postprocess after do_unpack
 
@@ -379,9 +382,13 @@ python do_rootfs_postprocess() {
     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))
+
+    try:
+        for i, cmd in enumerate(cmds):
+            bb.build.exec_func(cmd, d)
+            progress_reporter.update(int(i / len(cmds) * 100))
+    finally:
+        bb.build.exec_func('rootfs_do_umounts', d)
 }
 addtask rootfs_postprocess before do_rootfs after do_unpack
 
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20241011100050.322686-8-amikan%40ilbers.de.

[PATCH v3 8/8] image: Do not call rootfs_do_umounts twice

[Anton Mikanovich]

As now every rootfs_do_mounts should be followed by rootfs_do_umounts
in the same task, there is no need in calling umount inside
do_rootfs_finalize.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 RECIPE-API-CHANGELOG.md    | 6 ++++++
 meta/classes/image.bbclass | 2 --
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/RECIPE-API-CHANGELOG.md b/RECIPE-API-CHANGELOG.md
index 608d0cc3..56ebfeb7 100644
--- a/RECIPE-API-CHANGELOG.md
+++ b/RECIPE-API-CHANGELOG.md
@@ -674,3 +674,9 @@ always outputs a warning if mounts are left behind after the build.
 Bitbake exit status depends on ISAR_FAIL_ON_CLEANUP bitbake variable:
  - 0 or unset: Output a warning, unmount, build succeeds (default).
  - 1: Output a warning, keep mounts left behind, build fails.
+
+### Stricter rootfs mounts management
+
+rootfs_do_umounts is not called from do_rootfs_finalize anymore.
+
+Every individual task that does mounting must also do the umounting at its end.
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 1eb974e8..472df3cf 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -405,8 +405,6 @@ def apt_list_files(d):
 IMAGE_LISTS = "${@ ' '.join(apt_list_files(d)) }"
 
 do_rootfs_finalize() {
-    rootfs_do_umounts
-
     sudo -s <<'EOSUDO'
         set -e
 
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20241011100050.322686-9-amikan%40ilbers.de.

Re: [PATCH v3 0/8] Hanging mount fixes

[Baurzhan Ismagulov]

On 2024-10-11 13:00, Anton Mikanovich wrote:
> Consolidate the patches to address the hanging mount issues.
> 
> Addresses the following issues:
> 
>  1. Cleanup handler is called multiple times, unmounting any hanging
>     mounts.
> 
>  2. Hanging mount warnings are discarded in the current setup.
> 
>  3. Incorrect asymmetric mount-umount logic inherited from buildchroot
>     times.

The cases being fixed are covered with this patchset in Isar CI as well. There
are open cases of failed tasks and aborted builds which we'll be covering in
the patchsets to follow. If there are no objections, we'd like to apply this
tomorrow.

With kind regards,
Baurzhan

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/Zw40d3L3ew53UyN1%40abai.m.ilbers.de.

Re: [PATCH v3 0/8] Hanging mount fixes

[Uladzimir Bely]

On Fri, 2024-10-11 at 13:00 +0300, Anton Mikanovich wrote:
> Consolidate the patches to address the hanging mount issues.
> 
> Addresses the following issues:
> 
>  1. Cleanup handler is called multiple times, unmounting any hanging
>     mounts.
> 
>  2. Hanging mount warnings are discarded in the current setup.
> 
>  3. Incorrect asymmetric mount-umount logic inherited from
> buildchroot
>     times.
> 
> In this series, cleanup is done in reverse /proc/mounts order to
> unmount e.g. /dev/shm before /dev. This happens to work for us today
> but we could perform stricter semantic ordering if necessary.
> 
> ISAR_FAIL_ON_CLEANUP=1 can be set in the environment to fail builds
> if
> mounts are left behind. We will use this for Isar CI. The default is
> 0.
> 
> p1 comes from 'Additional CI improvements' series.
> Patches p4-p6 come from v2 of 'Start to address umount problems'
> series
> by Jan.
> p7 comes from 'Fix leftover mounts in rootfs.bbclass' by Florian.
> p2, p3 and p8 are new one.
> 
> Anton Mikanovich (4):
>   CI: Do not lose output on bitbake / qemu exit
>   isar-events: Unhide mounts left behind
>   CI: Pass ISAR_FAIL_ON_CLEANUP from environment to bitbake
>   image: Do not call rootfs_do_umounts twice
> 
> Florian Bezdeka (1):
>   rootfs: Add missing umounts in rootfs_postprocess() and
>     rootfs_install()
> 
> Jan Kiszka (3):
>   image: Avoid breaking the build when mounts are no longer present
> on
>     umount
>   rootfs: Provide rootfs_do_umounts
>   initramfs: Add missing umounts after generation
> 
>  RECIPE-API-CHANGELOG.md          | 15 ++++++++
>  meta/classes/image.bbclass       | 19 ----------
>  meta/classes/initramfs.bbclass   |  2 +
>  meta/classes/isar-events.bbclass | 40 ++++++++++++++++----
>  meta/classes/rootfs.bbclass      | 63 ++++++++++++++++++++++++++----
> --
>  testsuite/cibuilder.py           | 13 +++++--
>  6 files changed, 110 insertions(+), 42 deletions(-)
> 
> -- 
> 2.34.1
> 

Applied to next, thanks all.

-- 
Best regards,
Uladzimir.



-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/96f3c61318d93964c175ebd9e3406aa3b3e9712b.camel%40ilbers.de.