[PATCH v2 0/3] Start to address umount problems

[PATCH v2 0/3] Start to address umount problems

['Jan Kiszka' via isar-users]

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

-- 
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/cover.1727795639.git.jan.kiszka%40siemens.com.

[PATCH v2 1/3] image: Avoid breaking the build when mounts are no longer present on umount

['Jan Kiszka' via isar-users]

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

-- 
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/c84a1a48568e0204b0eb36fedb8af4a2809f199a.1727795639.git.jan.kiszka%40siemens.com.

[PATCH v2 2/3] rootfs: Provide rootfs_do_umounts

['Jan Kiszka' via isar-users]

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

-- 
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/c8fceb7009980251e7191709d808575ee279b499.1727795639.git.jan.kiszka%40siemens.com.

[PATCH v2 3/3] initramfs: Add missing umounts after generation

['Jan Kiszka' via isar-users]

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

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

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 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.43.0

-- 
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/3d2094cf49cca93e49b4e779cd29281f6016f824.1727795639.git.jan.kiszka%40siemens.com.

Re: [PATCH v2 0/3] Start to address umount problems

['Florian Bezdeka' via isar-users]

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.bezdeka@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


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

-- 
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/015cefd1513be9e918baefe50b55c8323065c384.camel%40siemens.com.