[PATCH 0/2] Fix leftover mounts in rootfs.bbclass

[PATCH 0/2] Fix leftover mounts in rootfs.bbclass

['Florian Bezdeka' via isar-users]

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-ad57-b3bf702608b0@siemens.com/

Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Felix Moessbauer <felix.moessbauer@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

-- 
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/20241002203156.149572-1-florian.bezdeka%40siemens.com.

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

['Florian Bezdeka' via isar-users]

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

-- 
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/20241002203156.149572-2-florian.bezdeka%40siemens.com.

[PATCH 2/2] Revert "events: Do not warn on left mounts by default"

['Florian Bezdeka' via isar-users]

This reverts commit 72ec986d9cd3a3913e8592554456d5968d6b659a.

Signed-off-by: Florian Bezdeka <florian.bezdeka@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

-- 
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/20241002203156.149572-3-florian.bezdeka%40siemens.com.

Re: [PATCH 2/2] Revert "events: Do not warn on left mounts by default"

['Jan Kiszka' via isar-users]

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

> Signed-off-by: Florian Bezdeka <florian.bezdeka@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,

-- 
Siemens AG, Technology
Linux Expert Center

-- 
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/dc07e5ec-bc42-4b94-918c-dee3dc4b984f%40siemens.com.

Re: [PATCH 2/2] Revert "events: Do not warn on left mounts by default"

[Anton Mikanovich]

03/10/2024 10:25, 'Jan Kiszka' via isar-users wrote:
> 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
>
>> Signed-off-by: Florian Bezdeka <florian.bezdeka@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,

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

-- 
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/77ecc277-59c5-4094-809c-f0fcc3f48e8e%40ilbers.de.

Re: [PATCH 2/2] Revert "events: Do not warn on left mounts by default"

['Jan Kiszka' via isar-users]

On 03.10.24 09:42, Anton Mikanovich wrote:
> 03/10/2024 10:25, 'Jan Kiszka' via isar-users wrote:
>> 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
>>
>>> Signed-off-by: Florian Bezdeka <florian.bezdeka@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,
> 
> The motivation of original change was discussed here:
> https://groups.google.com/g/isar-users/c/mz-7DWob0O0/m/8hLr6cw4AgAJ
> 

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

-- 
Siemens AG, Technology
Linux Expert Center

-- 
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/82a6c7c9-afc7-42e5-a8ed-44ada344ab85%40siemens.com.