[PATCH 0/1] try unmounting all mounts before returning an error

[PATCH 0/1] try unmounting all mounts before returning an error

[Cedric Hombourger]

Isar would loop forever if a mount cannot be unmounted because another
bind mount was done underneath it (e.g. <basedir>/sys/devices on top
of <basedir>/sys). Increase of chances of success by trying to unmount
everything before returning an error. In the <basedir>/sys/dev case,
we may try <basedir>/sys first and fail but would continue to unmount
<basedir>/sys/dev. On the following call, only <basedir>/sys will remain
and the unmount_all should now succeed. We may want to implement a
timeout: e.g. giveup after 30 tries (i.e. approximately 30 seconds)?

Cedric Hombourger (1):
  isar-events: try unmounting all mounts before returning an error

 meta/classes/isar-events.bbclass | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.11.0

[PATCH 1/1] isar-events: try unmounting all mounts before returning an error

[Cedric Hombourger]

Isar would loop forever if a mount cannot be unmounted because another
bind mount was done underneath it (e.g. <basedir>/sys/devices on top
of <basedir>/sys). Increase of chances of success by trying to unmount
everything before returning an error. In the <basedir>/sys/dev case,
we may try <basedir>/sys first and fail but would continue to unmount
<basedir>/sys/dev. On the following call, only <basedir>/sys will remain
and the unmount_all should now succeed. We may want to implement a
timeout: e.g. giveup after 30 tries (i.e. approximately 30 seconds)?

Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
---
 meta/classes/isar-events.bbclass | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
index 29c7437..b3ce434 100644
--- a/meta/classes/isar-events.bbclass
+++ b/meta/classes/isar-events.bbclass
@@ -14,14 +14,15 @@ python isar_handler () {
         # '/proc/mounts' contains all the active mounts, so knowing basepath
         # we can get the list of mounts for the specific multiconfig and
         # clean them.
+        result = True
         with open('/proc/mounts', 'rU') as f:
             for line in f:
                 if basepath in line:
                     if subprocess.call('sudo umount ' + line.split()[1],
                                        stdout=devnull, stderr=devnull,
                                        shell=True) != 0:
-                        return False
-        return True
+                        result = False
+        return result
 
     devnull = open(os.devnull, 'w')
 
-- 
2.11.0

Re: [PATCH 1/1] isar-events: try unmounting all mounts before returning an error

[Jan Kiszka]

On 2018-03-07 12:00, Cedric Hombourger wrote:
> Isar would loop forever if a mount cannot be unmounted because another
> bind mount was done underneath it (e.g. <basedir>/sys/devices on top
> of <basedir>/sys). Increase of chances of success by trying to unmount
> everything before returning an error. In the <basedir>/sys/dev case,
> we may try <basedir>/sys first and fail but would continue to unmount
> <basedir>/sys/dev. On the following call, only <basedir>/sys will remain
> and the unmount_all should now succeed. We may want to implement a
> timeout: e.g. giveup after 30 tries (i.e. approximately 30 seconds)?
> 
> Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
> ---
>  meta/classes/isar-events.bbclass | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
> index 29c7437..b3ce434 100644
> --- a/meta/classes/isar-events.bbclass
> +++ b/meta/classes/isar-events.bbclass
> @@ -14,14 +14,15 @@ python isar_handler () {
>          # '/proc/mounts' contains all the active mounts, so knowing basepath
>          # we can get the list of mounts for the specific multiconfig and
>          # clean them.
> +        result = True
>          with open('/proc/mounts', 'rU') as f:
>              for line in f:
>                  if basepath in line:
>                      if subprocess.call('sudo umount ' + line.split()[1],
>                                         stdout=devnull, stderr=devnull,
>                                         shell=True) != 0:
> -                        return False
> -        return True
> +                        result = False
> +        return result
>  
>      devnull = open(os.devnull, 'w')
>  
> 

This is by design because it gives us more time when waiting for pending
build steps. The problem is that things like multistrap can continue to
run in the background, no longer under the control of cooker daemon, and
we should wait for them to terminate before

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

RE: [PATCH 1/1] isar-events: try unmounting all mounts before returning an error

[Hombourger, Cedric]

Hi Jan,

Ok we can give background jobs plenty of time to run. I believe you may still want my change (and I am glad I decided to put aside the give-up change for now).
In my case, I had no background processes running (Isar had completed everything it needed to do) but would never complete because of the nested mounts done
by a package I was rebuilding.

If we do not implement a give-up mechanism, I would at least suggest logging a warning message every now and then
(e.g. warning: Isar is waiting on background jobs to complete, please be patient...). If the wait becomes really too long,
we could even print the list of children process IDs and names we see. This would give us a clue if we get a problem report
(when I ran bitbake with maximum debugging output, it would not give any hints on what it was doing... this issue was a
nice one to debug; trust me!).

Cedric

-----Original Message-----
From: Jan Kiszka [mailto:jan.kiszka@siemens.com] 
Sent: Wednesday, March 7, 2018 12:24 PM
To: Hombourger, Cedric <Cedric_Hombourger@mentor.com>; isar-users@googlegroups.com
Subject: Re: [PATCH 1/1] isar-events: try unmounting all mounts before returning an error

On 2018-03-07 12:00, Cedric Hombourger wrote:
> Isar would loop forever if a mount cannot be unmounted because another 
> bind mount was done underneath it (e.g. <basedir>/sys/devices on top 
> of <basedir>/sys). Increase of chances of success by trying to unmount 
> everything before returning an error. In the <basedir>/sys/dev case, 
> we may try <basedir>/sys first and fail but would continue to unmount 
> <basedir>/sys/dev. On the following call, only <basedir>/sys will 
> remain and the unmount_all should now succeed. We may want to 
> implement a
> timeout: e.g. giveup after 30 tries (i.e. approximately 30 seconds)?
> 
> Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
> ---
>  meta/classes/isar-events.bbclass | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/isar-events.bbclass 
> b/meta/classes/isar-events.bbclass
> index 29c7437..b3ce434 100644
> --- a/meta/classes/isar-events.bbclass
> +++ b/meta/classes/isar-events.bbclass
> @@ -14,14 +14,15 @@ python isar_handler () {
>          # '/proc/mounts' contains all the active mounts, so knowing basepath
>          # we can get the list of mounts for the specific multiconfig and
>          # clean them.
> +        result = True
>          with open('/proc/mounts', 'rU') as f:
>              for line in f:
>                  if basepath in line:
>                      if subprocess.call('sudo umount ' + line.split()[1],
>                                         stdout=devnull, stderr=devnull,
>                                         shell=True) != 0:
> -                        return False
> -        return True
> +                        result = False
> +        return result
>  
>      devnull = open(os.devnull, 'w')
>  
> 

This is by design because it gives us more time when waiting for pending build steps. The problem is that things like multistrap can continue to run in the background, no longer under the control of cooker daemon, and we should wait for them to terminate before

Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux

Re: [PATCH 1/1] isar-events: try unmounting all mounts before returning an error

[Jan Kiszka]

On 2018-03-07 12:34, Hombourger, Cedric wrote:
> Hi Jan,
> 
> Ok we can give background jobs plenty of time to run. I believe you may still want my change (and I am glad I decided to put aside the give-up change for now).
> In my case, I had no background processes running (Isar had completed everything it needed to do) but would never complete because of the nested mounts done
> by a package I was rebuilding.

Ah, nested mounts, now I got it :). Then this makes sense, true.

Jan

> 
> If we do not implement a give-up mechanism, I would at least suggest logging a warning message every now and then
> (e.g. warning: Isar is waiting on background jobs to complete, please be patient...). If the wait becomes really too long,
> we could even print the list of children process IDs and names we see. This would give us a clue if we get a problem report
> (when I ran bitbake with maximum debugging output, it would not give any hints on what it was doing... this issue was a
> nice one to debug; trust me!).
> 
> Cedric
> 
> -----Original Message-----
> From: Jan Kiszka [mailto:jan.kiszka@siemens.com] 
> Sent: Wednesday, March 7, 2018 12:24 PM
> To: Hombourger, Cedric <Cedric_Hombourger@mentor.com>; isar-users@googlegroups.com
> Subject: Re: [PATCH 1/1] isar-events: try unmounting all mounts before returning an error
> 
> On 2018-03-07 12:00, Cedric Hombourger wrote:
>> Isar would loop forever if a mount cannot be unmounted because another 
>> bind mount was done underneath it (e.g. <basedir>/sys/devices on top 
>> of <basedir>/sys). Increase of chances of success by trying to unmount 
>> everything before returning an error. In the <basedir>/sys/dev case, 
>> we may try <basedir>/sys first and fail but would continue to unmount 
>> <basedir>/sys/dev. On the following call, only <basedir>/sys will 
>> remain and the unmount_all should now succeed. We may want to 
>> implement a
>> timeout: e.g. giveup after 30 tries (i.e. approximately 30 seconds)?
>>
>> Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
>> ---
>>  meta/classes/isar-events.bbclass | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/classes/isar-events.bbclass 
>> b/meta/classes/isar-events.bbclass
>> index 29c7437..b3ce434 100644
>> --- a/meta/classes/isar-events.bbclass
>> +++ b/meta/classes/isar-events.bbclass
>> @@ -14,14 +14,15 @@ python isar_handler () {
>>          # '/proc/mounts' contains all the active mounts, so knowing basepath
>>          # we can get the list of mounts for the specific multiconfig and
>>          # clean them.
>> +        result = True
>>          with open('/proc/mounts', 'rU') as f:
>>              for line in f:
>>                  if basepath in line:
>>                      if subprocess.call('sudo umount ' + line.split()[1],
>>                                         stdout=devnull, stderr=devnull,
>>                                         shell=True) != 0:
>> -                        return False
>> -        return True
>> +                        result = False
>> +        return result
>>  
>>      devnull = open(os.devnull, 'w')
>>  
>>
> 
> This is by design because it gives us more time when waiting for pending build steps. The problem is that things like multistrap can continue to run in the background, no longer under the control of cooker daemon, and we should wait for them to terminate before
> 
> Jan
> 
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux