public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
* [PATCH] initramfs: Add missing umounts after generation
@ 2024-09-30 19:42 'Jan Kiszka' via isar-users
  2024-10-01  7:38 ` 'MOESSBAUER, Felix' via isar-users
  0 siblings, 1 reply; 8+ messages in thread
From: 'Jan Kiszka' via isar-users @ 2024-09-30 19:42 UTC (permalink / raw)
  To: 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/image.bbclass     | 21 ++-------------------
 meta/classes/initramfs.bbclass |  2 ++
 meta/classes/rootfs.bbclass    | 25 +++++++++++++++++++++++++
 3 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index c29d9e26..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,25 +408,6 @@ do_rootfs_finalize() {
                 -maxdepth 1 -name 'qemu-*-static' -type f -delete
         fi
 
-        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
-            umount '${ROOTFSDIR}/isar-apt' && \
-            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
-
-        mountpoint -q '${ROOTFSDIR}/base-apt' && \
-            umount '${ROOTFSDIR}/base-apt' && \
-            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
-
-        mountpoint -q '${ROOTFSDIR}/dev/pts' && \
-            umount '${ROOTFSDIR}/dev/pts'
-        mountpoint -q '${ROOTFSDIR}/dev/shm' && \
-            umount '${ROOTFSDIR}/dev/shm'
-        mountpoint -q '${ROOTFSDIR}/dev' && \
-            umount '${ROOTFSDIR}/dev'
-        mountpoint -q '${ROOTFSDIR}/proc' && \
-            umount '${ROOTFSDIR}/proc'
-        mountpoint -q '${ROOTFSDIR}/sys' && \
-            umount '${ROOTFSDIR}/sys'
-
         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/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
diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index f0abd795..0b4ea061 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -65,6 +65,31 @@ rootfs_do_mounts() {
 EOSUDO
 }
 
+rootfs_do_umounts() {
+    sudo -s <<'EOSUDO'
+        set -e
+        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
+            umount '${ROOTFSDIR}/isar-apt' && \
+            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
+
+        mountpoint -q '${ROOTFSDIR}/base-apt' && \
+            umount '${ROOTFSDIR}/base-apt' && \
+            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
+
+        mountpoint -q '${ROOTFSDIR}/dev/pts' && \
+            umount '${ROOTFSDIR}/dev/pts'
+        mountpoint -q '${ROOTFSDIR}/dev/shm' && \
+            umount '${ROOTFSDIR}/dev/shm'
+        mountpoint -q '${ROOTFSDIR}/dev' && \
+            umount '${ROOTFSDIR}/dev'
+        mountpoint -q '${ROOTFSDIR}/proc' && \
+            umount '${ROOTFSDIR}/proc'
+        mountpoint -q '${ROOTFSDIR}/sys' && \
+            umount '${ROOTFSDIR}/sys'
+
+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/1646de44-c244-43b3-ad57-b3bf702608b0%40siemens.com.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] initramfs: Add missing umounts after generation
  2024-09-30 19:42 [PATCH] initramfs: Add missing umounts after generation 'Jan Kiszka' via isar-users
@ 2024-10-01  7:38 ` 'MOESSBAUER, Felix' via isar-users
  2024-10-01 10:04   ` 'Jan Kiszka' via isar-users
  0 siblings, 1 reply; 8+ messages in thread
From: 'MOESSBAUER, Felix' via isar-users @ 2024-10-01  7:38 UTC (permalink / raw)
  To: isar-users, Kiszka, Jan

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

Felix

> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  meta/classes/image.bbclass     | 21 ++-------------------
>  meta/classes/initramfs.bbclass |  2 ++
>  meta/classes/rootfs.bbclass    | 25 +++++++++++++++++++++++++
>  3 files changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index c29d9e26..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,25 +408,6 @@ do_rootfs_finalize() {
>                  -maxdepth 1 -name 'qemu-*-static' -type f -delete
>          fi
>  
> -        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
> -            umount '${ROOTFSDIR}/isar-apt' && \
> -            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
> -
> -        mountpoint -q '${ROOTFSDIR}/base-apt' && \
> -            umount '${ROOTFSDIR}/base-apt' && \
> -            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
> -
> -        mountpoint -q '${ROOTFSDIR}/dev/pts' && \
> -            umount '${ROOTFSDIR}/dev/pts'
> -        mountpoint -q '${ROOTFSDIR}/dev/shm' && \
> -            umount '${ROOTFSDIR}/dev/shm'
> -        mountpoint -q '${ROOTFSDIR}/dev' && \
> -            umount '${ROOTFSDIR}/dev'
> -        mountpoint -q '${ROOTFSDIR}/proc' && \
> -            umount '${ROOTFSDIR}/proc'
> -        mountpoint -q '${ROOTFSDIR}/sys' && \
> -            umount '${ROOTFSDIR}/sys'
> -
>          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/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
> diff --git a/meta/classes/rootfs.bbclass
> b/meta/classes/rootfs.bbclass
> index f0abd795..0b4ea061 100644
> --- a/meta/classes/rootfs.bbclass
> +++ b/meta/classes/rootfs.bbclass
> @@ -65,6 +65,31 @@ rootfs_do_mounts() {
>  EOSUDO
>  }
>  
> +rootfs_do_umounts() {
> +    sudo -s <<'EOSUDO'
> +        set -e
> +        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
> +            umount '${ROOTFSDIR}/isar-apt' && \
> +            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
> +
> +        mountpoint -q '${ROOTFSDIR}/base-apt' && \
> +            umount '${ROOTFSDIR}/base-apt' && \
> +            rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
> +
> +        mountpoint -q '${ROOTFSDIR}/dev/pts' && \
> +            umount '${ROOTFSDIR}/dev/pts'
> +        mountpoint -q '${ROOTFSDIR}/dev/shm' && \
> +            umount '${ROOTFSDIR}/dev/shm'
> +        mountpoint -q '${ROOTFSDIR}/dev' && \
> +            umount '${ROOTFSDIR}/dev'
> +        mountpoint -q '${ROOTFSDIR}/proc' && \
> +            umount '${ROOTFSDIR}/proc'
> +        mountpoint -q '${ROOTFSDIR}/sys' && \
> +            umount '${ROOTFSDIR}/sys'
> +
> +EOSUDO
> +}
> +
>  rootfs_do_qemu() {
>      if [ '${@repr(d.getVar('ROOTFS_ARCH') ==
> d.getVar('HOST_ARCH'))}' = 'False' ]
>      then
> -- 
> 2.43.0
> 

-- 
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/855415c409c8a24b672ed64dba8f3227edf23355.camel%40siemens.com.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] initramfs: Add missing umounts after generation
  2024-10-01  7:38 ` 'MOESSBAUER, Felix' via isar-users
@ 2024-10-01 10:04   ` 'Jan Kiszka' via isar-users
  2024-10-02  8:31     ` 'Florian Bezdeka' via isar-users
  0 siblings, 1 reply; 8+ messages in thread
From: 'Jan Kiszka' via isar-users @ 2024-10-01 10:04 UTC (permalink / raw)
  To: Moessbauer, Felix (T CED OES-DE), isar-users

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.

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/4271f8a5-45d8-4e75-9a39-7c11a3411ef6%40siemens.com.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] initramfs: Add missing umounts after generation
  2024-10-01 10:04   ` 'Jan Kiszka' via isar-users
@ 2024-10-02  8:31     ` 'Florian Bezdeka' via isar-users
  2024-10-02 10:28       ` 'Jan Kiszka' via isar-users
  0 siblings, 1 reply; 8+ messages in thread
From: 'Florian Bezdeka' via isar-users @ 2024-10-02  8:31 UTC (permalink / raw)
  To: Jan Kiszka, Moessbauer, Felix (T CED OES-DE), isar-users

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

Florian

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] initramfs: Add missing umounts after generation
  2024-10-02  8:31     ` 'Florian Bezdeka' via isar-users
@ 2024-10-02 10:28       ` 'Jan Kiszka' via isar-users
  2024-10-02 15:10         ` 'Florian Bezdeka' via isar-users
  2024-10-03 16:15         ` Baurzhan Ismagulov
  0 siblings, 2 replies; 8+ messages in thread
From: 'Jan Kiszka' via isar-users @ 2024-10-02 10:28 UTC (permalink / raw)
  To: Florian Bezdeka, Moessbauer, Felix (T CED OES-DE), isar-users

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] initramfs: Add missing umounts after generation
  2024-10-02 10:28       ` 'Jan Kiszka' via isar-users
@ 2024-10-02 15:10         ` 'Florian Bezdeka' via isar-users
  2024-10-03 16:15         ` Baurzhan Ismagulov
  1 sibling, 0 replies; 8+ messages in thread
From: 'Florian Bezdeka' via isar-users @ 2024-10-02 15:10 UTC (permalink / raw)
  To: Jan Kiszka, Moessbauer, Felix (T CED OES-DE), isar-users

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] initramfs: Add missing umounts after generation
  2024-10-02 10:28       ` 'Jan Kiszka' via isar-users
  2024-10-02 15:10         ` 'Florian Bezdeka' via isar-users
@ 2024-10-03 16:15         ` Baurzhan Ismagulov
  2024-10-04  7:47           ` 'Jan Kiszka' via isar-users
  1 sibling, 1 reply; 8+ messages in thread
From: Baurzhan Ismagulov @ 2024-10-03 16:15 UTC (permalink / raw)
  To: isar-users; +Cc: Florian Bezdeka, Moessbauer, Felix (T CED OES-DE), Jan Kiszka

Hello Jan,

On 2024-10-02 12:28, 'Jan Kiszka' via isar-users wrote:
> 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.

I'll share a bit of background on this: This is not about assumptions, we knew
there were issues (and worked on them -- e.g., [1]). The occurrence rate was
several times per year. After migrating to sbuild, mounts moved into
per-package schroots, and the errors became even more rare. Debugging cycles of
weeks to months are not practical because you can dump the mounts or similar
stuff, but the necessary number of iterations is not reachable, not in
reasonable time.

Before sending the lazy umount removal patch, we've checked the archives why
"umount -l", "umount || true" were introduced but couldn't reproduce any of
those (I personally found the problem descriptions and root cause analysis
quite spare and often not actionable -- I think we can improve here). Even now,
the issue seems to occur more frequently downstream than in meta-isar CI. Given
that the downstreams have control of meta-isar revision and can also apply
patches, the question for me was whether to keep the problems under the rug or
work with downstreams to analyze them. If you have specific suggestions what we
could do better the next time, please share.

For this specific occurrence, if we don't have the races with package building
left after the sbuild introduction (this is why mounts and umounts were
asymmetric with buildchroot), the latest patches might be enough without
central mount / umount tracking or moving the mounts to individual schroots
altogether. We'll need to understand the failure mode because otherwise proper
testing is challenging.

1. https://lists.isar-build.org/isar-users/20210421145855.66257-1-amikan@ilbers.de/#b

With kind regards,
Baurzhan

-- 
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/Zv7DB35UdWbBLpW2%40abai.m.ilbers.de.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] initramfs: Add missing umounts after generation
  2024-10-03 16:15         ` Baurzhan Ismagulov
@ 2024-10-04  7:47           ` 'Jan Kiszka' via isar-users
  0 siblings, 0 replies; 8+ messages in thread
From: 'Jan Kiszka' via isar-users @ 2024-10-04  7:47 UTC (permalink / raw)
  To: isar-users, Florian Bezdeka, Moessbauer, Felix (T CED OES-DE)

On 03.10.24 18:15, Baurzhan Ismagulov wrote:
> Hello Jan,
> 
> On 2024-10-02 12:28, 'Jan Kiszka' via isar-users wrote:
>> 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.
> 
> I'll share a bit of background on this: This is not about assumptions, we knew
> there were issues (and worked on them -- e.g., [1]). The occurrence rate was
> several times per year. After migrating to sbuild, mounts moved into
> per-package schroots, and the errors became even more rare. Debugging cycles of
> weeks to months are not practical because you can dump the mounts or similar
> stuff, but the necessary number of iterations is not reachable, not in
> reasonable time.
> 
> Before sending the lazy umount removal patch, we've checked the archives why
> "umount -l", "umount || true" were introduced but couldn't reproduce any of
> those (I personally found the problem descriptions and root cause analysis
> quite spare and often not actionable -- I think we can improve here). Even now,
> the issue seems to occur more frequently downstream than in meta-isar CI. Given
> that the downstreams have control of meta-isar revision and can also apply
> patches, the question for me was whether to keep the problems under the rug or
> work with downstreams to analyze them. If you have specific suggestions what we
> could do better the next time, please share.
> 
> For this specific occurrence, if we don't have the races with package building
> left after the sbuild introduction (this is why mounts and umounts were
> asymmetric with buildchroot), the latest patches might be enough without
> central mount / umount tracking or moving the mounts to individual schroots
> altogether. We'll need to understand the failure mode because otherwise proper
> testing is challenging.
> 
> 1. https://lists.isar-build.org/isar-users/20210421145855.66257-1-amikan@ilbers.de/#b
> 

There are multiple things:
 - mounts left behind - several, easily to review manually once it was
   clear that those still exist (I wasn't expecting that)
 - left-behind mounts detection - apparently broken, not properly tested
 - races and other conditions actually stumbling over those mounts -
   rare, specifically with the few cases of isar itself; and that's why
   the lazy umount removal appeared to be safe

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/b2b32e26-c713-408a-8422-1354a48dc517%40siemens.com.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-10-04  7:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-30 19:42 [PATCH] initramfs: Add missing umounts after generation 'Jan Kiszka' via isar-users
2024-10-01  7:38 ` 'MOESSBAUER, Felix' via isar-users
2024-10-01 10:04   ` 'Jan Kiszka' via isar-users
2024-10-02  8:31     ` 'Florian Bezdeka' via isar-users
2024-10-02 10:28       ` 'Jan Kiszka' via isar-users
2024-10-02 15:10         ` 'Florian Bezdeka' via isar-users
2024-10-03 16:15         ` Baurzhan Ismagulov
2024-10-04  7:47           ` 'Jan Kiszka' via isar-users

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox