Patchwork kernel.bbclass: Tolerate empty INITRAMFS_IMAGE when INITRAMFS_TASK is set

login
register
mail settings
Submitter Phil Blundell
Date Nov. 15, 2013, 1:05 p.m.
Message ID <1384520708.23724.12.camel@phil-desktop.brightsign>
Download mbox | patch
Permalink /patch/61783/
State New
Headers show

Comments

Phil Blundell - Nov. 15, 2013, 1:05 p.m.
The idiomatic way to reinstate the old-style initramfs handling appears to be to
set:

INITRAMFS_TASK = "${INITRAMFS_IMAGE}:do_rootfs"

in some appropriate place.  However, this causes INITRAMFS_TASK to end up non-empty
even if INITRAMFS_IMAGE is blank, which has two undesirable consequences:

1. The anonymous python in kernel.bbclass attempts to make the empty
string a dependency for do_configure; bitbake doesn't diagnose this
directly but it leads to subsequent build failures with slightly obscure
messages like:

Missing or unbuildable dependency chain was: [..., 'kernel-modules', '']

2. The shell code in kernel_do_compile() will set CONFIG_INITRAMFS_SOURCE to
${B}/usr/${INITRAMFS_IMAGE}-${MACHINE}.cpio which clearly will have no useful
result if ${INITRAMFS_IMAGE} is empty.

Fix both of these by making the relevant code check for non-empty values in both
${INITRAMFS_TASK} and ${INITRAMFS_IMAGE} before attempting to use these variables.

Signed-off-by: Phil Blundell <philb@gnu.org>
---
 meta/classes/kernel.bbclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Otavio Salvador - Nov. 15, 2013, 1:26 p.m.
Hello Phil,

On Fri, Nov 15, 2013 at 11:05 AM, Phil Blundell <pb@pbcl.net> wrote:
> The idiomatic way to reinstate the old-style initramfs handling appears to be to
> set:
>
> INITRAMFS_TASK = "${INITRAMFS_IMAGE}:do_rootfs"

What problems are you having with the 'new-style'?
Phil Blundell - Nov. 16, 2013, 6:09 p.m.
On Fri, 2013-11-15 at 11:26 -0200, Otavio Salvador wrote:
> Hello Phil,
> 
> On Fri, Nov 15, 2013 at 11:05 AM, Phil Blundell <pb@pbcl.net> wrote:
> > The idiomatic way to reinstate the old-style initramfs handling appears to be to
> > set:
> >
> > INITRAMFS_TASK = "${INITRAMFS_IMAGE}:do_rootfs"
> 
> What problems are you having with the 'new-style'?

Good question.  When I first merged the version of kernel.bbclass that
implemented the "new-style" system I was having a bunch of problems
which seemed to be due to the new code.  However, after a bit of further
debugging it now appears that most of them are issues with the
implementation rather than with the concept and selecting INITRAMFS_TASK
doesn't actually help very much because the code that was causing my
problems is shared between the two paths.

The main difficulty I was having was that kernel.bbclass was looking for
the initramfs image with slightly the wrong filename.  After a certain
amount of sleuthing and some clues from Andrea it eventually became
apparent that kernel.bbclass relies on the initramfs image recipe
leaving ${IMAGE_LINK_NAME} at its default value and will fail if it's
set to anything else.  This seems slightly fragile and it seems as
though it would be better for kernel.bbclass to look for the initramfs
image at its primary location rather than relying on the symlink.

I also discovered that the new code in kernel.bbclass uncompresses the
initramfs image before embedding it in the kernel.  I can guess why this
seemed like a good plan, but it's slightly surprising behaviour and it
seems like anybody who wanted to embed the initramfs in uncompressed
form would just select IMAGE_TYPE = "cpio" for it in the first place.
Also, Andrea reports that he does actually get worse compression overall
with this approach in at least some circumstances.

Anyway, with those things fixed I can now at least build images again
and it's entirely possible that the "new style" system would also work
for me.  However, it doesn't seem as though the new technology would
bring me any particular benefits, and as far as I can tell it would
introduce an extra kernel compile step that I don't particularly want,
so I am inclined to stick with INITRAMFS_TASK for the moment.

Right now I'm trying to debug a slightly obscure problem where, under
circumstances that aren't entirely clear to me, bitbake ends up running
linux:do_compile, linux:do_bundle_initramfs and then
linux:do_populate_sysroot but somehow overlooking do_install.  This
causes sysroot_stage_all() to blow up because the directory it's trying
to copy from doesn't exist.  I'm not entirely sure whether this is
related to the initramfs code or not, though it does only seem to affect
kernel recipes.

p.
Richard Purdie - Nov. 18, 2013, 12:26 p.m.
On Sat, 2013-11-16 at 18:09 +0000, Phil Blundell wrote:
> On Fri, 2013-11-15 at 11:26 -0200, Otavio Salvador wrote:
> > Hello Phil,
> > 
> > On Fri, Nov 15, 2013 at 11:05 AM, Phil Blundell <pb@pbcl.net> wrote:
> > > The idiomatic way to reinstate the old-style initramfs handling appears to be to
> > > set:
> > >
> > > INITRAMFS_TASK = "${INITRAMFS_IMAGE}:do_rootfs"
> > 
> > What problems are you having with the 'new-style'?
> 
> Good question.  When I first merged the version of kernel.bbclass that
> implemented the "new-style" system I was having a bunch of problems
> which seemed to be due to the new code.  However, after a bit of further
> debugging it now appears that most of them are issues with the
> implementation rather than with the concept and selecting INITRAMFS_TASK
> doesn't actually help very much because the code that was causing my
> problems is shared between the two paths.
> 
> The main difficulty I was having was that kernel.bbclass was looking for
> the initramfs image with slightly the wrong filename.  After a certain
> amount of sleuthing and some clues from Andrea it eventually became
> apparent that kernel.bbclass relies on the initramfs image recipe
> leaving ${IMAGE_LINK_NAME} at its default value and will fail if it's
> set to anything else.  This seems slightly fragile and it seems as
> though it would be better for kernel.bbclass to look for the initramfs
> image at its primary location rather than relying on the symlink.
> 
> I also discovered that the new code in kernel.bbclass uncompresses the
> initramfs image before embedding it in the kernel.  I can guess why this
> seemed like a good plan, but it's slightly surprising behaviour and it
> seems like anybody who wanted to embed the initramfs in uncompressed
> form would just select IMAGE_TYPE = "cpio" for it in the first place.
> Also, Andrea reports that he does actually get worse compression overall
> with this approach in at least some circumstances.
> 
> Anyway, with those things fixed I can now at least build images again
> and it's entirely possible that the "new style" system would also work
> for me.  However, it doesn't seem as though the new technology would
> bring me any particular benefits, and as far as I can tell it would
> introduce an extra kernel compile step that I don't particularly want,
> so I am inclined to stick with INITRAMFS_TASK for the moment.

FWIW I would *love* to simplify this code and have one good/efficient
codepath everyone uses. The initramfs decompression code looks nasty to
me for example and I agree with your thoughts on that.

Cheers,

Richard
Andrea Adami - Nov. 18, 2013, 11:26 p.m.
On Mon, Nov 18, 2013 at 1:26 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Sat, 2013-11-16 at 18:09 +0000, Phil Blundell wrote:
>> On Fri, 2013-11-15 at 11:26 -0200, Otavio Salvador wrote:
>> > Hello Phil,
>> >
>> > On Fri, Nov 15, 2013 at 11:05 AM, Phil Blundell <pb@pbcl.net> wrote:
>> > > The idiomatic way to reinstate the old-style initramfs handling appears to be to
>> > > set:
>> > >
>> > > INITRAMFS_TASK = "${INITRAMFS_IMAGE}:do_rootfs"
>> >
>> > What problems are you having with the 'new-style'?
>>
>> Good question.  When I first merged the version of kernel.bbclass that
>> implemented the "new-style" system I was having a bunch of problems
>> which seemed to be due to the new code.  However, after a bit of further
>> debugging it now appears that most of them are issues with the
>> implementation rather than with the concept and selecting INITRAMFS_TASK
>> doesn't actually help very much because the code that was causing my
>> problems is shared between the two paths.
>>
>> The main difficulty I was having was that kernel.bbclass was looking for
>> the initramfs image with slightly the wrong filename.  After a certain
>> amount of sleuthing and some clues from Andrea it eventually became
>> apparent that kernel.bbclass relies on the initramfs image recipe
>> leaving ${IMAGE_LINK_NAME} at its default value and will fail if it's
>> set to anything else.  This seems slightly fragile and it seems as
>> though it would be better for kernel.bbclass to look for the initramfs
>> image at its primary location rather than relying on the symlink.
>>
>> I also discovered that the new code in kernel.bbclass uncompresses the
>> initramfs image before embedding it in the kernel.  I can guess why this
>> seemed like a good plan, but it's slightly surprising behaviour and it
>> seems like anybody who wanted to embed the initramfs in uncompressed
>> form would just select IMAGE_TYPE = "cpio" for it in the first place.
>> Also, Andrea reports that he does actually get worse compression overall
>> with this approach in at least some circumstances.
>>
>> Anyway, with those things fixed I can now at least build images again
>> and it's entirely possible that the "new style" system would also work
>> for me.  However, it doesn't seem as though the new technology would
>> bring me any particular benefits, and as far as I can tell it would
>> introduce an extra kernel compile step that I don't particularly want,
>> so I am inclined to stick with INITRAMFS_TASK for the moment.
>
> FWIW I would *love* to simplify this code and have one good/efficient
> codepath everyone uses. The initramfs decompression code looks nasty to
> me for example and I agree with your thoughts on that.
>


This decompression is apparently what makes my kernel unbootable.
We purposedly specify options for the XZ compressor (for machines with
32MB ram) and if the cpio is uncompressed before these preferences are
lost.

Andrea



> Cheers,
>
> Richard
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
Andrea Adami - Nov. 22, 2013, 10:25 p.m.
On Tue, Nov 19, 2013 at 12:26 AM, Andrea Adami <andrea.adami@gmail.com> wrote:
> On Mon, Nov 18, 2013 at 1:26 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> On Sat, 2013-11-16 at 18:09 +0000, Phil Blundell wrote:
>>> On Fri, 2013-11-15 at 11:26 -0200, Otavio Salvador wrote:
>>> > Hello Phil,
>>> >
>>> > On Fri, Nov 15, 2013 at 11:05 AM, Phil Blundell <pb@pbcl.net> wrote:
>>> > > The idiomatic way to reinstate the old-style initramfs handling appears to be to
>>> > > set:
>>> > >
>>> > > INITRAMFS_TASK = "${INITRAMFS_IMAGE}:do_rootfs"
>>> >
>>> > What problems are you having with the 'new-style'?
>>>
>>> Good question.  When I first merged the version of kernel.bbclass that
>>> implemented the "new-style" system I was having a bunch of problems
>>> which seemed to be due to the new code.  However, after a bit of further
>>> debugging it now appears that most of them are issues with the
>>> implementation rather than with the concept and selecting INITRAMFS_TASK
>>> doesn't actually help very much because the code that was causing my
>>> problems is shared between the two paths.
>>>
>>> The main difficulty I was having was that kernel.bbclass was looking for
>>> the initramfs image with slightly the wrong filename.  After a certain
>>> amount of sleuthing and some clues from Andrea it eventually became
>>> apparent that kernel.bbclass relies on the initramfs image recipe
>>> leaving ${IMAGE_LINK_NAME} at its default value and will fail if it's
>>> set to anything else.  This seems slightly fragile and it seems as
>>> though it would be better for kernel.bbclass to look for the initramfs
>>> image at its primary location rather than relying on the symlink.
>>>
>>> I also discovered that the new code in kernel.bbclass uncompresses the
>>> initramfs image before embedding it in the kernel.  I can guess why this
>>> seemed like a good plan, but it's slightly surprising behaviour and it
>>> seems like anybody who wanted to embed the initramfs in uncompressed
>>> form would just select IMAGE_TYPE = "cpio" for it in the first place.
>>> Also, Andrea reports that he does actually get worse compression overall
>>> with this approach in at least some circumstances.
>>>
>>> Anyway, with those things fixed I can now at least build images again
>>> and it's entirely possible that the "new style" system would also work
>>> for me.  However, it doesn't seem as though the new technology would
>>> bring me any particular benefits, and as far as I can tell it would
>>> introduce an extra kernel compile step that I don't particularly want,
>>> so I am inclined to stick with INITRAMFS_TASK for the moment.
>>
>> FWIW I would *love* to simplify this code and have one good/efficient
>> codepath everyone uses. The initramfs decompression code looks nasty to
>> me for example and I agree with your thoughts on that.
>>
>
>
> This decompression is apparently what makes my kernel unbootable.
> We purposedly specify options for the XZ compressor (for machines with
> 32MB ram) and if the cpio is uncompressed before these preferences are
> lost.
>
> Andrea
>
>

So, can we change it? I'd say if one bothers to specify compression
for the cpio then respect that...
If on the other hand one needs to do dirty things within the cpio,
then don't compress it as first: the compression options can be
specified in the kernel recipe/config so the artifact is compressed
afterwards.

Cheers

Andrea

>
>> Cheers,
>>
>> Richard
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Patch

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 383043e..ef86c71 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -27,7 +27,7 @@  python __anonymous () {
     #       this INITRAMFS_TASK has circular dependency problems
     #       if the initramfs requires kernel modules
     image_task = d.getVar('INITRAMFS_TASK', True)
-    if image_task:
+    if image_task and d.getVar('INITRAMFS_IMAGE', True):
         d.appendVarFlag('do_configure', 'depends', ' ${INITRAMFS_TASK}')
 }
 
@@ -163,7 +163,7 @@  kernel_do_compile() {
 	# different initramfs image.  The way to do that in the kernel
 	# is to specify:
 	# make ...args... CONFIG_INITRAMFS_SOURCE=some_other_initramfs.cpio
-	if [ "$use_alternate_initrd" = "" ] && [ "${INITRAMFS_TASK}" != "" ] ; then
+	if [ "$use_alternate_initrd" = "" ] && [ "${INITRAMFS_TASK}" != "" ] && [ "${INITRAMFS_IMAGE}" != "" ]; then
 		# The old style way of copying an prebuilt image and building it
 		# is turned on via INTIRAMFS_TASK != ""
 		copy_initramfs