Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Ian Campbell]

On Sun, 2017-01-01 at 12:40 -0800, Martin Michlmayr wrote:
> I investigated a bit more and
> found
> out that it relates to the use of "local".  See e.g.
> http://superuser.com/questions/363444/how-do-i-get-the-output-and-exi
> t-value-of-a-subshell-when-using-bash-e/1103711#1103711
> for an explanation. 

How exciting!

>  Fortunately this is easy enough to work around.

Great!

Ian.

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Martin Michlmayr]

* Ian Campbell  [2016-12-23 00:23]:
> IIRC "set -e" doesn't propagate to subshells with Bash, my
> understanding of this is from http://xenbits.xen.org/gitweb/?p=osstest.
> git;a=commit;h=6ffbf6eee57d0e4a7f1a669a66dc1a0ae1f7d8d6
> 
> But flash-kernel uses /bin/sh which these days ought to be dash not
> bash and the original bug report does say:
> 
> Shell: /bin/sh linked to /bin/dash
> 
> Maybe there is some similar oddity with function calls in dash?

Thanks for pointing out this issue.  We use /bin/sh so we're not
running into this issue.  However, I investigated a bit more and found
out that it relates to the use of "local".  See e.g.
http://superuser.com/questions/363444/how-do-i-get-the-output-and-exit-value-of-a-subshell-when-using-bash-e/1103711#1103711
for an explanation.  Fortunately this is easy enough to work around.

-- 
Martin Michlmayr
http://www.cyrius.com/

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Ian Campbell]

On Sat, 2016-12-17 at 14:09 -0800, Martin Michlmayr wrote:

> It seems the exit called from find_dtb_file() doesn't exit the whole
> program.  I know this is normal because it's called in a subshell,
> but
> flash-kernel itself does a "set -e" so I thought any exit should
> trigger the whole script to stop.
> 
> Ian, any idea?

IIRC "set -e" doesn't propagate to subshells with Bash, my
understanding of this is from http://xenbits.xen.org/gitweb/?p=osstest.
git;a=commit;h=6ffbf6eee57d0e4a7f1a669a66dc1a0ae1f7d8d6

But flash-kernel uses /bin/sh which these days ought to be dash not
bash and the original bug report does say:

Shell: /bin/sh linked to /bin/dash

Maybe there is some similar oddity with function calls in dash?

Ian.

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Martin Michlmayr]

* Ian Campbell  [2016-10-01 08:29]:
> > Looking at the code, all uses of find_dtb_file() check for the result
> > and produce an error if the file doesn't exist, so maybe we should
> > just move the error messages into find_dtb_file().  Then we could
> > tell
> > the user where we were looking.
> > 
> > Ian, any comments?
> 
> Nope, sounds sensible to me!
> 
> Looks like handle_dtb() doesn't produce an error if the file doesn't
> exist -- but it probably should. Need to take care in the postrm.d case
> (where the file might legitimately not be there) though -- fortunately
> it looks like the result of find_dtb_file isn't actually used in that
> half of the if, so it could probably be moved.

Sorry for the delay (again!).  I finally looked into this again.
I came up with the patch below and I get:

> Using DTB: XXXarmada-370-seagate-personal-cloud.dtb
> Couldn't find DTB XXXarmada-370-seagate-personal-cloud.dtb in 
> /usr/lib/linux-image-4.9.0-rc8-armmp or /etc/flash-kernel/dtbs

So that part looks good.

But then I get:

> Installing  into 
> /boot/dtbs/4.9.0-rc8-armmp/XXXarmada-370-seagate-personal-cloud.dtb
> cp: cannot stat '': No such file or directory

It seems the exit called from find_dtb_file() doesn't exit the whole
program.  I know this is normal because it's called in a subshell, but
flash-kernel itself does a "set -e" so I thought any exit should
trigger the whole script to stop.

Ian, any idea?

Here's the patch so far:

diff --git a/functions b/functions
index e050269..acc1a33 100644
--- a/functions
+++ b/functions
@@ -249,7 +249,7 @@ get_dtb_name() {
;;
esac
if [ -n "$dtb_name" ] ; then
-   echo "DTB: $dtb_name" >&2
+   echo "Using DTB: $dtb_name" >&2
fi
 }
 
@@ -561,18 +561,25 @@ android_flash() {
 }
 
 find_dtb_file() {
+   local dtb
case "$dtb_name" in
/*)
-   echo "$dtb_name"
+   dtb="$dtb_name"
+   if [ ! -f "$dtb" ]; then
+   error "Couldn't find $dtb"
+   fi
;;
*)
-   local dtb=$(find /etc/flash-kernel/dtbs -name $dtb_name 
2>/dev/null | head -n 1)
+   dtb=$(find /etc/flash-kernel/dtbs -name $dtb_name 2>/dev/null | 
head -n 1)
if [ -z "$dtb" ]; then
dtb=$(find /usr/lib/linux-image-$kvers -name $dtb_name 
2>/dev/null | head -n 1)
fi
-   echo $dtb
+   if [ ! -f "$dtb" ]; then
+   error "Couldn't find DTB $dtb_name in 
/usr/lib/linux-image-$kvers or /etc/flash-kernel/dtbs"
+   fi
;;
esac
+   echo $dtb
 }
 
 handle_dtb() {
@@ -580,9 +587,8 @@ handle_dtb() {
return
fi
 
-   local dtb=$(find_dtb_file)
-   local dtb_name=$(basename $dtb_name)
if [ "x$FK_KERNEL_HOOK_SCRIPT" = "xpostrm.d" ] ; then
+   local dtb_name=$(basename $dtb_name)
rm -f "/boot/dtbs/$kvers/$dtb_name"
 
# This was the old name we installed under. We
@@ -606,26 +612,24 @@ handle_dtb() {
rmdir --ignore-fail-on-non-empty /boot/dtbs
fi
else
-   if [ -e "$dtb" ]; then
-   echo "Installing $dtb into /boot/dtbs/$kvers/$dtb_name" 
>&2
-   mkdir -p /boot/dtbs/$kvers/
-   cp "$dtb" "/boot/dtbs/$kvers/$dtb_name.new"
-   backup_and_install \
-   "/boot/dtbs/$kvers/$dtb_name.new" \
-   "/boot/dtbs/$kvers/$dtb_name"
-
-   # Historically we installed the dtb as
-   # dtb-$kvers, keep it around as an alternative
-   # for now. Useful for platforms which do not
-   # set ${fdtfile}
-   ln -nfs "dtbs/$kvers/$dtb_name" "/boot/dtb-$kvers"
-
-   # This can be used along with the unversioned
-   # vmlinuz+initrd.gz e.g. as a fallback option
-   ln -nfs "dtbs/$kvers/$dtb_name" "/boot/dtb"
-   else
-   error "Couldn't find $dtb"
-   fi
+   local dtb=$(find_dtb_file)
+   local dtb_name=$(basename $dtb_name)
+   echo "Installing $dtb into /boot/dtbs/$kvers/$dtb_name" >&2
+   mkdir -p /boot/dtbs/$kvers/
+   cp "$dtb" "/boot/dtbs/$kvers/$dtb_name.new"
+   backup_and_install \
+   "/boot/dtbs/$kvers/$dtb_name.new" \
+   "/boot/dtbs/$kvers/$dtb_name"
+
+   # Historically we installed the dtb as
+   # dtb-$kvers, keep it around as an alternative
+   # for now. Useful for platforms which do not
+   # set ${fdtfile}
+   ln -nfs "dtbs/$kvers/$dtb_name" 

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Ian Campbell]

On Fri, 2016-09-30 at 11:21 -0700, Martin Michlmayr wrote:
> Looking at the code, all uses of find_dtb_file() check for the result
> and produce an error if the file doesn't exist, so maybe we should
> just move the error messages into find_dtb_file().  Then we could
> tell
> the user where we were looking.
> 
> Ian, any comments?

Nope, sounds sensible to me!

Looks like handle_dtb() doesn't produce an error if the file doesn't
exist -- but it probably should. Need to take care in the postrm.d case
(where the file might legitimately not be there) though -- fortunately
it looks like the result of find_dtb_file isn't actually used in that
half of the if, so it could probably be moved.

Cheers,
Ian.

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Martin Michlmayr]

* Dominique Dumont  [2016-09-15 08:06]:
> I guess that the intent is to let user know the the right DTB for his board 
> is 
> sun7i-a20-olinuxino-lime.dtb but this file cannot be found in the expected 
> location (which is I guess /boot). 

No, it's usually found under /usr/lib/linux-image-$kvers (it ships
with the kernel) but flash-kernel also allows users to put DTBs in
/etc/flash-kernel/dtbs

(flash-kernel will take the DTB from there and then store it under
/boot for the boot process.)

> I think this require some context knowledge from user to intepret
> this correctly. I think also that indicating where the DTB file is
> expected would give user a hint to look for an issue with kernel
> installation or kernel content.

> How about something like:
> # flash-kernel
> This board requires DTB file: sun7i-a20-olinuxino-lime.dtb
> Couldn't find sun7i-a20-olinuxino-lime.dtb in /boot

We could definitely do:

> This board requires DTB file: sun7i-a20-olinuxino-lime.dtb
> Couldn't find sun7i-a20-olinuxino-lime.dtb

It's harder to specify the location because there are several options
(find_dtb_file accepts absolute paths, and there's
/usr/lib/linux-image-$kvers and /etc/flash-kernel/dtbs)

Looking at the code, all uses of find_dtb_file() check for the result
and produce an error if the file doesn't exist, so maybe we should
just move the error messages into find_dtb_file().  Then we could tell
the user where we were looking.

Ian, any comments?

-- 
Martin Michlmayr
http://www.cyrius.com/

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Dominique Dumont]

On Wednesday, 14 September 2016 15:39:53 CEST you wrote:
> I can change it to:
> Couldn't find sun7i-a20-olinuxino-lime.dtb
> Does this work?

All in all, user would see:
# flash-kernel
 DTB: sun7i-a20-olinuxino-lime.dtb
Couldn't find sun7i-a20-olinuxino-lime.dtb

I guess that the intent is to let user know the the right DTB for his board is 
sun7i-a20-olinuxino-lime.dtb but this file cannot be found in the expected 
location (which is I guess /boot). 

I think this require some context knowledge from user to intepret this 
correctly. I think also that indicating where the DTB file is expected would 
give user a hint to look for an issue with kernel installation or kernel 
content.

How about something like:
# flash-kernel
This board requires DTB file: sun7i-a20-olinuxino-lime.dtb
Couldn't find sun7i-a20-olinuxino-lime.dtb in /boot


Hope this helps

-- 
 https://github.com/dod38fr/   -o- http://search.cpan.org/~ddumont/
http://ddumont.wordpress.com/  -o-   irc: dod at irc.debian.org

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Martin Michlmayr]

* Dominique Dumont  [2016-09-05 09:42]:
> On the other hand the error message returned by flash-kernel could be 
> improved. 
> 
> When the dtb is missing, flash-kernel shows:
> 
> # flash-kernel
> DTB: sun7i-a20-olinuxino-lime.dtb
> Couldn't find 
> 
> Reading that, I thought that the dtb file was found, but something else was 
> wrong.
> 
> Could you improve this error message ?

Sorry for the delay.  I didn't notice the broken error message when
you first pointed it out.  This happened because I changed the code
that looks for the DTB.

I can change it to:
Couldn't find sun7i-a20-olinuxino-lime.dtb
Does this work?

-- 
Martin Michlmayr
http://www.cyrius.com/

Processed: Re: Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Debian Bug Tracking System]

Processing commands for cont...@bugs.debian.org:

> unmerge 836679
Bug #836679 [src:linux-signed] DTBs are not included in signed kernel packages
Bug #836255 [src:linux-signed] DTBs are no longer bundled
Disconnected #836679 from all other report(s).
> reassign 836679 flash-kernel
Bug #836679 [src:linux-signed] DTBs are not included in signed kernel packages
Bug reassigned from package 'src:linux-signed' to 'flash-kernel'.
No longer marked as found in versions linux-signed/2.
Ignoring request to alter fixed versions of bug #836679 to the same values 
previously set
> severity 836679 normal
Bug #836679 [flash-kernel] DTBs are not included in signed kernel packages
Severity set to 'normal' from 'serious'
> retitle 836679 Warning for missing DTB unclear
Bug #836679 [flash-kernel] DTBs are not included in signed kernel packages
Changed Bug title to 'Warning for missing DTB unclear' from 'DTBs are not 
included in signed kernel packages'.
> thanks
Stopping processing here.

Please contact me if you need assistance.
-- 
836255: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=836255
836679: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=836679
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Ben Hutchings]

Control: reassign -1 src:linux-signed 2
Control: retitle -1 DTBs are not included in signed kernel packages
Control: forcemerge 836255 -1

On Sun, 2016-09-04 at 12:34 -0700, Martin Michlmayr wrote:
> * Dominique Dumont  [2016-09-04 21:16]:
> > 
> > On the other there's no dtb file in linux-image-4.7.0-1-armmp-lpae
> > ...
> > $ dpkg --listfiles linux-image-4.7.0-1-armmp-lpae | grep dtb
> > [ nothing ]
> > 
> > Did I miss something ?
> 
> Maybe the real bug is that the signed package doesn't contain the
> DTBs.

Exactly

Ben.

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

signature.asc
Description: This is a digitally signed message part

Processed: Re: Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Debian Bug Tracking System]

Processing control commands:

> reassign -1 src:linux-signed 2
Bug #836679 [flash-kernel] flash-kernel: cannot configure kernel 4.7 with new 
flash-kernel
Bug reassigned from package 'flash-kernel' to 'src:linux-signed'.
No longer marked as found in versions flash-kernel/3.68.
Ignoring request to alter fixed versions of bug #836679 to the same values 
previously set
Bug #836679 [src:linux-signed] flash-kernel: cannot configure kernel 4.7 with 
new flash-kernel
Marked as found in versions linux-signed/2.
> retitle -1 DTBs are not included in signed kernel packages
Bug #836679 [src:linux-signed] flash-kernel: cannot configure kernel 4.7 with 
new flash-kernel
Changed Bug title to 'DTBs are not included in signed kernel packages' from 
'flash-kernel: cannot configure kernel 4.7 with new flash-kernel'.
> forcemerge 836255 -1
Bug #836255 [src:linux-signed] DTBs are no longer bundled
Bug #836679 [src:linux-signed] DTBs are not included in signed kernel packages
Severity set to 'serious' from 'important'
Merged 836255 836679

-- 
836255: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=836255
836679: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=836679
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Martin Michlmayr]

* Dominique Dumont  [2016-09-04 21:16]:
> On the other there's no dtb file in linux-image-4.7.0-1-armmp-lpae ...
> $ dpkg --listfiles linux-image-4.7.0-1-armmp-lpae | grep dtb
> [ nothing ]
> 
> Did I miss something ?

Maybe the real bug is that the signed package doesn't contain the
DTBs.

-- 
Martin Michlmayr
http://www.cyrius.com/

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Dominique Dumont]

On the other there's no dtb file in linux-image-4.7.0-1-armmp-lpae ...
$ dpkg --listfiles linux-image-4.7.0-1-armmp-lpae | grep dtb
[ nothing ]

Did I miss something ?


-- 
 https://github.com/dod38fr/   -o- http://search.cpan.org/~ddumont/
http://ddumont.wordpress.com/  -o-   irc: dod at irc.debian.org

Bug#836679: flash-kernel: cannot configure kernel 4.7 with new flash-kernel

[Dominique]

Package: flash-kernel
Version: 3.68
Severity: important

Dear Maintainer,

I cannot install latest kernel on my OlinuXino card:
# dpkg --configure linux-image-4.7.0-1-armmp-lpae
dpkg: dependency problems prevent configuration of 
linux-image-4.7.0-1-armmp-lpae:
 linux-image-4.7.0-1-armmp-lpae depends on initramfs-tools (>= 0.110~) | 
linux-initramfs-tool; however:
  Package initramfs-tools is not configured yet.
  Package linux-initramfs-tool is not installed.
  Package initramfs-tools which provides linux-initramfs-tool is not configured 
yet.

dpkg: error processing package linux-image-4.7.0-1-armmp-lpae (--configure):
 dependency problems - leaving unconfigured
Errors were encountered while processing:
 linux-image-4.7.0-1-armmp-lpae

After this failure, flash-kernel fails with the following message:
# flash-kernel
DTB: sun7i-a20-olinuxino-lime.dtb
Couldn't find 

Indeed, there's no DTB file for kernel 4.7 in /boot:
# ls /boot/dtbs/
4.4.0-1-armmp-lpae  4.5.0-2-armmp-lpae  4.6.0-1-armmp-lpae

This may be because kernel 4.7 is not yet fully installed:
# dpkg -l linux-image*
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name   Version  
Architecture Description
+++-==---==
rc  linux-image-3.16.0-4-armmp-lpae3.16.7-ckt25-1   armhf   
 Linux 3.16 for ARMv7 multiplatform compatible SoCs supporting LPAE
ii  linux-image-4.4.0-1-armmp-lpae 4.4.6-1  armhf   
 Linux 4.4 for ARMv7 multiplatform compatible SoCs supporting LPAE
hi  linux-image-4.5.0-2-armmp-lpae 4.5.2-1  armhf   
 Linux 4.5 for ARMv7 multiplatform compatible SoCs supporting LPAE
ii  linux-image-4.6.0-1-armmp-lpae 4.6.4-1  armhf   
 Linux 4.6 for ARMv7 multiplatform compatible SoCs supporting LPAE
iF  linux-image-4.7.0-1-armmp-lpae 4.7.2-1  armhf   
 Linux 4.7 for ARMv7 multiplatform compatible SoCs supporting LPAE 
(signed)
un  linux-image-4.7.0-1-armmp-lpae-unsigne  
 (no description available)
iU  linux-image-armmp-lpae 4.7+75   armhf   
 Linux for ARMv7 multiplatform compatible SoCs supporting LPAE 
(meta-package)

Once I remove the fix done with #833097 [1] , I get:
# flash-kernel 
DTB: sun7i-a20-olinuxino-lime.dtb
flash-kernel: installing version 4.7.0-1-armmp-lpae
Generating boot script u-boot image... done.
Taking backup of boot.scr.
Installing new boot.scr.

Which is much better...

Now I can finish the installation:
# dpkg --configure --pending
Setting up initramfs-tools (0.125) ...
update-initramfs: deferring update (trigger activated)
Setting up linux-image-4.7.0-1-armmp-lpae (4.7.2-1) ...
/etc/kernel/postinst.d/initramfs-tools:
update-initramfs: Generating /boot/initrd.img-4.7.0-1-armmp-lpae
cp: cannot stat '/etc/modprobe.d/*': No such file or directory
DTB: sun7i-a20-olinuxino-lime.dtb
flash-kernel: deferring update (trigger activated)
/etc/kernel/postinst.d/zz-flash-kernel:
DTB: sun7i-a20-olinuxino-lime.dtb
flash-kernel: deferring update (trigger activated)
Setting up linux-image-armmp-lpae (4.7+75) ...
Processing triggers for initramfs-tools (0.125) ...
update-initramfs: Generating /boot/initrd.img-4.7.0-1-armmp-lpae
cp: cannot stat '/etc/modprobe.d/*': No such file or directory
DTB: sun7i-a20-olinuxino-lime.dtb
flash-kernel: installing version 4.7.0-1-armmp-lpae
Generating boot script u-boot image... done.
Taking backup of boot.scr.
Installing new boot.scr.
Processing triggers for flash-kernel (3.68) ...
DTB: sun7i-a20-olinuxino-lime.dtb
flash-kernel: installing version 4.7.0-1-armmp-lpae
Generating boot script u-boot image... done.
Taking backup of boot.scr.
Installing new boot.scr.


Could find another fix for #833097 ?

All the best

[1] 
https://anonscm.debian.org/cgit/d-i/flash-kernel.git/commit/?id=33929c93c03d9d48bc4e1c58610bcb2ab3efdd04



-- System Information:
Debian Release: stretch/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing')
Architecture: armhf (armv7l)

Kernel: Linux 4.6.0-1-armmp-lpae (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages flash-kernel depends on:
ii  debconf [debconf-2.0]  1.5.59
ii  devio  1.2-1.2
ih  initramfs-tools0.125
ii  linux-base 4.4
ii  mtd-utils  1:1.5.2-1
ii  ucf3.0036

Versions of packages flash-kernel recommends:
ii  u-boot-tools