[PATCH v2 2/3] rootfs: Provide rootfs_do_umounts

[Jan Kiszka]

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

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

Signed-off-by: Jan Kiszka <jan.k...@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 ef25df49..1414a3ee 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -393,6 +393,8 @@ python do_deploy() {
addtask deploy before do_build after do_image

do_rootfs_finalize() {
+ rootfs_do_umounts
+
sudo -s <<'EOSUDO'
set -e

@@ -406,32 +408,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.43.0

[Jan Kiszka]

From: Jan Kiszka <jan.k...@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.k...@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 c29d9e26..ef25df49 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -406,24 +406,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.43.0

[Jan Kiszka]

Changes in v2:
- break up the previously single patch into refactoring + fix
- add a fix for prematurely failing rootfs_do_unounts

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

meta/classes/image.bbclass | 21 ++-------------------
meta/classes/initramfs.bbclass | 2 ++
meta/classes/rootfs.bbclass | 32 ++++++++++++++++++++++++++++++++
3 files changed, 36 insertions(+), 19 deletions(-)

--
2.43.0

[Florian Bezdeka]

On Tue, 2024-10-01 at 17:13 +0200, Jan Kiszka wrote:
> Changes in v2:
> - break up the previously single patch into refactoring + fix
> - add a fix for prematurely failing rootfs_do_unounts

For v2:
Tested-by: Florian Bezdeka <florian...@siemens.com>

But:
We're not done yet. There are two more calls to rootfs_do_mounts that
are not yet pared with rootfs_do_unmounts calls. The one in
do_rootfs_postprocess is the problematic one from the other thread on
the ML. (Anton is now in CC as well)

I'm currently testing (on top of v2):

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