[PATCH 1/2] rootfs: Add missing umounts in rootfs_postprocess() and rootfs_install()

[Florian Bezdeka]

Calls to rootfs_do_mounts should always be paired with calls to
rootfs_do_umounts.

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.

Signed-off-by: Florian Bezdeka <florian...@siemens.com>
---
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 c67d3bb8..e359d529 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -235,18 +235,21 @@ python do_rootfs_install() {

progress_reporter = bb.progress.MultiStageProgressReporter(d, stage_weights)

- 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

@@ -366,9 +369,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.39.5

[Florian Bezdeka]

Hi all,

this is a follow up of the umount fixes provided by Jan earlier this
week. This series depends on the series sent by Jan to be appied first.

See [1] for details.

In my case there were mount points in
build/tmp/work/debian-bookworm-amd64/sbuild-chroot-target/1.0-r0/rootfs/dev
left after the build completed successfully.

We have some cleanup handlers that should detect and report such issues.
It turned out that for some reason the warnings were silenced by using
bb.debug(), which is disabled by default.

Best regards,
Florian

[1] https://lists.isar-build.org/isar-users/1646de44-c244-43b3...@siemens.com/

Cc: Jan Kiszka <jan.k...@siemens.com>
Cc: Felix Moessbauer <felix.mo...@siemens.com>

Florian Bezdeka (2):
rootfs: Add missing umounts in rootfs_postprocess() and
rootfs_install()
Revert "events: Do not warn on left mounts by default"

meta/classes/isar-events.bbclass | 2 +-
meta/classes/rootfs.bbclass | 31 +++++++++++++++++++------------
2 files changed, 20 insertions(+), 13 deletions(-)

--
2.39.5

[Florian Bezdeka]

This reverts commit 72ec986d9cd3a3913e8592554456d5968d6b659a.

Signed-off-by: Florian Bezdeka <florian...@siemens.com>
---

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 f5061a8b..77dfecee 100644
--- a/meta/classes/isar-events.bbclass
+++ b/meta/classes/isar-events.bbclass
@@ -53,7 +53,7 @@ python build_completed() {
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])
+ bb.warn('%s left mounted, unmounting...' % line.split()[1])
subprocess.call(
["sudo", "umount", line.split()[1]],
stdout=subprocess.DEVNULL,
--
2.39.5

[Jan Kiszka]

On 02.10.24 22:31, 'Florian Bezdeka' via isar-users wrote:
> This reverts commit 72ec986d9cd3a3913e8592554456d5968d6b659a.
>

This needs reasoning why the original motivation for this change no
longer applies or was wrong.

Jan

Siemens AG, Technology
Linux Expert Center

[Anton Mikanovich]

The motivation of original change was discussed here:
https://groups.google.com/g/isar-users/c/mz-7DWob0O0/m/8hLr6cw4AgAJ

[Jan Kiszka]

Exactly. I wonder if we cannot limit bb.warn to error-free build
completion, somehow. On build errors, this remains a cleanup that should
not warn.

Jan