[PATCH] kernel: Use KERNEL_IMAGETYPE_FOR_MAKE in do_bundle_initramfs

[PATCH] kernel: Use KERNEL_IMAGETYPE_FOR_MAKE in do_bundle_initramfs

[Thomas Perrot]

In the case of a fitImage the this step should be done on the
image use to assemble the fitImage.

Signed-off-by: Thomas Perrot <thomas.perrot@tupi.fr>
---
 meta/classes/kernel.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index c42f85c665..ff09d4c4c7 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -231,7 +231,7 @@ do_bundle_initramfs () {
 		copy_initramfs
 		# Backing up kernel image relies on its type(regular file or symbolic link)
 		tmp_path=""
-		for type in ${KERNEL_IMAGETYPES} ; do
+		for type in ${KERNEL_IMAGETYPE_FOR_MAKE} ; do
 			if [ -h ${KERNEL_OUTPUT_DIR}/$type ] ; then
 				linkpath=`readlink -n ${KERNEL_OUTPUT_DIR}/$type`
 				realpath=`readlink -fn ${KERNEL_OUTPUT_DIR}/$type`
-- 
2.13.6

Re: [PATCH] kernel: Use KERNEL_IMAGETYPE_FOR_MAKE in do_bundle_initramfs

[Andre McCurdy]

On Wed, Jan 17, 2018 at 1:21 AM, Thomas Perrot <thomas.perrot@tupi.fr> wrote:
> In the case of a fitImage the this step should be done on the
> image use to assemble the fitImage.
>
> Signed-off-by: Thomas Perrot <thomas.perrot@tupi.fr>
> ---
>  meta/classes/kernel.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index c42f85c665..ff09d4c4c7 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -231,7 +231,7 @@ do_bundle_initramfs () {
>                 copy_initramfs
>                 # Backing up kernel image relies on its type(regular file or symbolic link)
>                 tmp_path=""
> -               for type in ${KERNEL_IMAGETYPES} ; do
> +               for type in ${KERNEL_IMAGETYPE_FOR_MAKE} ; do

The net result of this change is to no longer backup the non-initramfs
vmlinux.gz kernel image before creating the kernel image(s) containing
bundled initramfs (plus some other corner cases, depending on whether
the legacy KERNEL_IMAGETYPE and KERNEL_ALT_IMAGETYPE variables are
used and whether or not KERNEL_IMAGETYPES contains both vmlinux and
vmlinux.gz or vmlinux.gz only).

Can you explain a little how that's related to building fitImage kernels?

This code in do_bundle_initramfs() is extremely fragile and changes
may only break less well testing targets such as MIPS, so any changes
need to be very carefully reviewed and tested.

>                         if [ -h ${KERNEL_OUTPUT_DIR}/$type ] ; then
>                                 linkpath=`readlink -n ${KERNEL_OUTPUT_DIR}/$type`
>                                 realpath=`readlink -fn ${KERNEL_OUTPUT_DIR}/$type`
> --
> 2.13.6
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] kernel: Use KERNEL_IMAGETYPE_FOR_MAKE in do_bundle_initramfs

[Richard Purdie]

On Wed, 2018-01-17 at 13:31 -0800, Andre McCurdy wrote:
> On Wed, Jan 17, 2018 at 1:21 AM, Thomas Perrot <thomas.perrot@tupi.fr
> > wrote:
> > 
> > In the case of a fitImage the this step should be done on the
> > image use to assemble the fitImage.
> > 
> > Signed-off-by: Thomas Perrot <thomas.perrot@tupi.fr>
> > ---
> >  meta/classes/kernel.bbclass | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes/kernel.bbclass
> > b/meta/classes/kernel.bbclass
> > index c42f85c665..ff09d4c4c7 100644
> > --- a/meta/classes/kernel.bbclass
> > +++ b/meta/classes/kernel.bbclass
> > @@ -231,7 +231,7 @@ do_bundle_initramfs () {
> >                 copy_initramfs
> >                 # Backing up kernel image relies on its
> > type(regular file or symbolic link)
> >                 tmp_path=""
> > -               for type in ${KERNEL_IMAGETYPES} ; do
> > +               for type in ${KERNEL_IMAGETYPE_FOR_MAKE} ; do
> The net result of this change is to no longer backup the non-
> initramfs
> vmlinux.gz kernel image before creating the kernel image(s)
> containing
> bundled initramfs (plus some other corner cases, depending on whether
> the legacy KERNEL_IMAGETYPE and KERNEL_ALT_IMAGETYPE variables are
> used and whether or not KERNEL_IMAGETYPES contains both vmlinux and
> vmlinux.gz or vmlinux.gz only).
> 
> Can you explain a little how that's related to building fitImage
> kernels?
> 
> This code in do_bundle_initramfs() is extremely fragile and changes
> may only break less well testing targets such as MIPS, so any changes
> need to be very carefully reviewed and tested.

Actually, I think it means we need better tests.

I'm seriously considering removing a lot of the stuff in kernel.bbclass
and then rebuilding things in hopefully a cleaner way. The test for
anything being needed would be whether our regression tests pass.

I appreciate the idea will fill some with horror but there are a few
places the code code is turning into an unmaintainable (and
unreviewable) mess, the kernel classes are getting there :(

Cheers,

Richard

Re: [PATCH] kernel: Use KERNEL_IMAGETYPE_FOR_MAKE in do_bundle_initramfs

[Andre McCurdy]

On Sun, Jan 21, 2018 at 2:02 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Wed, 2018-01-17 at 13:31 -0800, Andre McCurdy wrote:
>> On Wed, Jan 17, 2018 at 1:21 AM, Thomas Perrot <thomas.perrot@tupi.fr
>> > wrote:
>> >
>> > In the case of a fitImage the this step should be done on the
>> > image use to assemble the fitImage.
>> >
>> > Signed-off-by: Thomas Perrot <thomas.perrot@tupi.fr>
>> > ---
>> >  meta/classes/kernel.bbclass | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/meta/classes/kernel.bbclass
>> > b/meta/classes/kernel.bbclass
>> > index c42f85c665..ff09d4c4c7 100644
>> > --- a/meta/classes/kernel.bbclass
>> > +++ b/meta/classes/kernel.bbclass
>> > @@ -231,7 +231,7 @@ do_bundle_initramfs () {
>> >                 copy_initramfs
>> >                 # Backing up kernel image relies on its
>> > type(regular file or symbolic link)
>> >                 tmp_path=""
>> > -               for type in ${KERNEL_IMAGETYPES} ; do
>> > +               for type in ${KERNEL_IMAGETYPE_FOR_MAKE} ; do
>> The net result of this change is to no longer backup the non-
>> initramfs
>> vmlinux.gz kernel image before creating the kernel image(s)
>> containing
>> bundled initramfs (plus some other corner cases, depending on whether
>> the legacy KERNEL_IMAGETYPE and KERNEL_ALT_IMAGETYPE variables are
>> used and whether or not KERNEL_IMAGETYPES contains both vmlinux and
>> vmlinux.gz or vmlinux.gz only).
>>
>> Can you explain a little how that's related to building fitImage
>> kernels?
>>
>> This code in do_bundle_initramfs() is extremely fragile and changes
>> may only break less well testing targets such as MIPS, so any changes
>> need to be very carefully reviewed and tested.
>
> Actually, I think it means we need better tests.

Yes. This bug was spotted via code review though (unfortunately too
late to prevent the change being merged to master...).

> I'm seriously considering removing a lot of the stuff in kernel.bbclass
> and then rebuilding things in hopefully a cleaner way. The test for
> anything being needed would be whether our regression tests pass.
>
> I appreciate the idea will fill some with horror but there are a few
> places the code code is turning into an unmaintainable (and
> unreviewable) mess, the kernel classes are getting there :(
>
> Cheers,
>
> Richard