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

["'Florian Bezdeka' via isar-users" <isar-users@googlegroups.com>]

On Wed, 2024-10-02 at 12:28 +0200, Jan Kiszka wrote:
> 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.

Technically someone should go and revert 
72ec986d9cd3 ("events: Do not warn on left mounts by default")
then.

https://github.com/ilbers/isar/commit/72ec986d9cd3a3913e8592554456d5968d6b659a

> 
> 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/716633968fcf2babb39125d568c48d69aead3b78.camel%40siemens.com.