Re: [PATCH] initramfs: Add missing umounts after generation

["'Jan Kiszka' via isar-users" <isar-users@googlegroups.com>]

On 02.10.24 10:31, Florian Bezdeka wrote:
> On Tue, 2024-10-01 at 12:04 +0200, 'Jan Kiszka' via isar-users wrote:
>> On 01.10.24 09:38, Moessbauer, Felix (T CED OES-DE) wrote:
>>> On Mon, 2024-09-30 at 21:42 +0200, 'Jan Kiszka' via isar-users wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> Failing to unmount what was mounted via rootfs_do_mounts can cause
>>>> troubles on rebuilds.
>>>
>>> Why is that not caught by the cleanup handler in [1]?
>>> I'm just wondering as the mounting and unmounting does not happen in
>>> the same task, hence we will run into problems on partial rebuilds.
>>>
>>> [1]https://github.com/ilbers/isar/blob/51462e82e4a108c9c94ba4099465f5a19a7125a6/meta/classes/isar-events.bbclass#L53
>>
>> I didn't analyze that yet, but it is a valid question why our diagnostic
>> framework failed as well.
> 
> Taken from the cleanup handler referenced above:
> 
>     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,
>                 )
> 
> Problems:
> 
> - bb.debug() will not make it to stdout as long as the log level is not
>   set accordingly. By default the level is not set to debug, so nobody
>   will notice.
> 
> - The cleanup handler is executed in "build" context, not "task" 
>   context. (Naming might be wrong bitbake wise...). With that there is
>   no log file where bb.debug() will write to. Nobody will notice
>   (again...).
> 
> - subprocess.call() might fail silently, there is no error checking.
>   In my case: "target is busy.\n"
> 
> - The cleanup handler is called twice. Not sure if that is correct...
> 
> At the end our "(u)mount mistake detection mechanism" seems broken. As
> first step I would vote for a bb.debug() -> bb.warn() migration. Manual
> inspection of stdout still required...
> 

Ack. And a test for this handler would be good.

Lazy umount was apparently dropped without review/testing of the safety
mechanisms and the code that is involved in mounting. We all assumed to
world would be fine while things de facto work not that well. On top
comes that most users are in containers and only notice those issues on
rebuilds or due to further special factors.

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/f6fc27b0-a5c5-4bd9-bf40-83b90e702e4c%40siemens.com.