[RFC,1/1] kernel: add kernel-image-fitimage-initramfs

When creating an initramfs bundled into a kernel fitimage, the resulting
fitimage will only be placed into the deploy directory and not packaged
by the kernel recipe/class.

Changing the kernel recipe/class to produce a package with the fitimage
containing the initramfs is not possible, because build dependencies
require the kernel packages to be already build before the initramfs can
be created.

The kernel-image-fitimage-initramfs recipe solves this dependency cycle
by packaging the fitimage with the embedded initramfs from the deploy
directory and replaces the `kernel-image` and `kernel-image-fitimage`
packages from the kernel recipe/class.

Signed-off-by: Claudius Heine <ch@denx.de>
---
 .../kernel-image-fitimage-initramfs_0.1.0.bb  | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 meta/recipes-kernel/kernel-image-fitimage-initramfs/kernel-image-fitimage-initramfs_0.1.0.bb

Hi,

On 2022-03-30 11:15, Claudius Heine wrote:
> When creating an initramfs bundled into a kernel fitimage, the resulting
> fitimage will only be placed into the deploy directory and not packaged
> by the kernel recipe/class.
> 
> Changing the kernel recipe/class to produce a package with the fitimage
> containing the initramfs is not possible, because build dependencies
> require the kernel packages to be already build before the initramfs can
> be created.
> 
> The kernel-image-fitimage-initramfs recipe solves this dependency cycle
> by packaging the fitimage with the embedded initramfs from the deploy
> directory and replaces the `kernel-image` and `kernel-image-fitimage`
> packages from the kernel recipe/class.

This patch was on master-next for a short while, any feedback on it?

In the cover letter I stated the only downside of this solution is, that 
in case of package based updating, it would require manually changing 
the version of this recipe.

Is this an acceptable downside, or is there a good way to fix this?

regards,
Claudius

> 
> Signed-off-by: Claudius Heine <ch@denx.de>
> ---
>   .../kernel-image-fitimage-initramfs_0.1.0.bb  | 50 +++++++++++++++++++
>   1 file changed, 50 insertions(+)
>   create mode 100644 meta/recipes-kernel/kernel-image-fitimage-initramfs/kernel-image-fitimage-initramfs_0.1.0.bb
> 
> diff --git a/meta/recipes-kernel/kernel-image-fitimage-initramfs/kernel-image-fitimage-initramfs_0.1.0.bb b/meta/recipes-kernel/kernel-image-fitimage-initramfs/kernel-image-fitimage-initramfs_0.1.0.bb
> new file mode 100644
> index 0000000000..c25168301b
> --- /dev/null
> +++ b/meta/recipes-kernel/kernel-image-fitimage-initramfs/kernel-image-fitimage-initramfs_0.1.0.bb
> @@ -0,0 +1,50 @@
> +# Copyright (C) 2022 Claudius Heine <ch@denx.de>
> +# Released under the MIT license (see COPYING.MIT for the terms)
> +
> +SUMMARY = "Packages the fitimage with embedded initramfs"
> +LICENSE = "GPL-2.0-only"
> +DEPENDS = "virtual/kernel"
> +
> +SRC_URI = ""
> +
> +inherit kernel-artifact-names
> +INITRAMFS_IMAGE_NAME ?= "${@['${INITRAMFS_IMAGE}-${MACHINE}', ''][d.getVar('INITRAMFS_IMAGE') == '']}"
> +KERNEL_PACKAGE_NAME ?= "kernel"
> +
> +PN = "${KERNEL_PACKAGE_NAME}-image-fitimage-initramfs"
> +
> +do_configure[noexec] = "1"
> +do_compile[noexec] = "1"
> +
> +do_install[deptask] = "do_deploy"
> +do_install() {
> +    fitimage="$(readlink "${DEPLOY_DIR_IMAGE}/fitImage-${INITRAMFS_IMAGE_NAME}-${KERNEL_FIT_LINK_NAME}")"
> +    install -d ${D}/boot
> +    install -m 0644 \
> +        ${DEPLOY_DIR_IMAGE}/${fitimage} \
> +        ${D}/boot/
> +    ln -snf \
> +        ${fitimage} \
> +        ${D}/boot/fitImage
> +}
> +
> +PACKAGES += "${KERNEL_PACKAGE_NAME}-image-initramfs"
> +
> +FILES:${PN} = "/boot/fitImage /boot/fitImage-*"
> +RPROVIDES:${PN} = "${KERNEL_PACKAGE_NAME}-image-fitimage"
> +RCONFLICTS:${PN} = "${KERNEL_PACKAGE_NAME}-image-fitimage"
> +RREPLACES:${PN} = "${KERNEL_PACKAGE_NAME}-image-fitimage"
> +
> +RDEPENDS:${KERNEL_PACKAGE_NAME}-image-initramfs = "${KERNEL_PACKAGE_NAME}-image-fitimage-initramfs (= ${EXTENDPGKV})"
> +RPROVIDES:${KERNEL_PACKAGE_NAME}-image-initramfs = "${KERNEL_PACKAGE_NAME}-image"
> +RCONFLICTS:${KERNEL_PACKAGE_NAME}-image-initramfs = "${KERNEL_PACKAGE_NAME}-image"
> +RREPLACES:${KERNEL_PACKAGE_NAME}-image-initramfs = "${KERNEL_PACKAGE_NAME}-image"
> +ALLOW_EMPTY:${KERNEL_PACKAGE_NAME}-image-initramfs = "1"
> +
> +python() {
> +    if (not 'fitImage' in d.getVar('KERNEL_IMAGETYPES') or
> +            bb.utils.to_boolean(d.getVar('INITRAMFS_IMAGE_BUNDLE'))):
> +        raise bb.parse.SkipRecipe('Requires generation of fitImage and bundled initramfs.')
> +    if not d.getVar('KERNEL_FIT_LINK_NAME'):
> +        raise bb.parse.SkipRecipe('Requires generation of fitImage symlink in deploy dir.')
> +}

On Wed, 2022-04-13 at 09:18 +0200, Claudius Heine wrote:
> Hi,
> 
> On 2022-03-30 11:15, Claudius Heine wrote:
> > When creating an initramfs bundled into a kernel fitimage, the resulting
> > fitimage will only be placed into the deploy directory and not packaged
> > by the kernel recipe/class.
> > 
> > Changing the kernel recipe/class to produce a package with the fitimage
> > containing the initramfs is not possible, because build dependencies
> > require the kernel packages to be already build before the initramfs can
> > be created.
> > 
> > The kernel-image-fitimage-initramfs recipe solves this dependency cycle
> > by packaging the fitimage with the embedded initramfs from the deploy
> > directory and replaces the `kernel-image` and `kernel-image-fitimage`
> > packages from the kernel recipe/class.
> 
> This patch was on master-next for a short while, any feedback on it?
> 
> In the cover letter I stated the only downside of this solution is, that 
> in case of package based updating, it would require manually changing 
> the version of this recipe.
> 
> Is this an acceptable downside, or is there a good way to fix this?

There were a few things which concern me. It is late in the release cycle for
this to make it into kirkstone so I was deferring it until 4.1 and the next
development cycle.

I was worried that it doesn't have a maintainers entry. I was also worried that
it doesn't have any tests and doesn't get used at all on the autobuilder. There
doesn't seem to have been much other feedback/review on the mailing list either.
There also isn't any documentation. This makes it likely to become a piece of
untested and potentially dead code going forward.

People are fixing up bits of the fitimage and initramfs code adhoc with just
enough changes to sovle their use case. We really need docs and tests and
ideally a clear plan on how this all fits together.

I suspect that the downside you mention, the fact that the kernel is removed
from package management will also catch people out. It is different to our
normal behaviour and I don't know how we highlight to users that this issue is
present.

I'm also worried that we don't have enough people looking at rewiew and that
there are probably issues here we need to discuss but we simply don't have the
right people with time to look at and think about it. I'm not the subject matter
expert here :(.

I wish I had something I could say as to how to make it acceptable to merge
(definitely needs tests) but I really don't know this area well enough and I
haven't had the time to think enough about it. It hovered in -next for a while
as I was trying my best to do so but with the release breaking I needed to clear
out -next for my own sanity and this ended in my defer pile. Even writing the
email explaining why takes 10 minutes.

Cheers,

Richard

Hi Richard,

On 2022-04-13 09:35, Richard Purdie wrote:
> On Wed, 2022-04-13 at 09:18 +0200, Claudius Heine wrote:
>> Hi,
>>
>> On 2022-03-30 11:15, Claudius Heine wrote:
>>> When creating an initramfs bundled into a kernel fitimage, the resulting
>>> fitimage will only be placed into the deploy directory and not packaged
>>> by the kernel recipe/class.
>>>
>>> Changing the kernel recipe/class to produce a package with the fitimage
>>> containing the initramfs is not possible, because build dependencies
>>> require the kernel packages to be already build before the initramfs can
>>> be created.
>>>
>>> The kernel-image-fitimage-initramfs recipe solves this dependency cycle
>>> by packaging the fitimage with the embedded initramfs from the deploy
>>> directory and replaces the `kernel-image` and `kernel-image-fitimage`
>>> packages from the kernel recipe/class.
>>
>> This patch was on master-next for a short while, any feedback on it?
>>
>> In the cover letter I stated the only downside of this solution is, that
>> in case of package based updating, it would require manually changing
>> the version of this recipe.
>>
>> Is this an acceptable downside, or is there a good way to fix this?
> 
> There were a few things which concern me. It is late in the release cycle for
> this to make it into kirkstone so I was deferring it until 4.1 and the next
> development cycle.
> 
> I was worried that it doesn't have a maintainers entry. I was also worried that
> it doesn't have any tests and doesn't get used at all on the autobuilder. There
> doesn't seem to have been much other feedback/review on the mailing list either.
> There also isn't any documentation. This makes it likely to become a piece of
> untested and potentially dead code going forward.
> 
> People are fixing up bits of the fitimage and initramfs code adhoc with just
> enough changes to sovle their use case. We really need docs and tests and
> ideally a clear plan on how this all fits together.
> 
> I suspect that the downside you mention, the fact that the kernel is removed
> from package management will also catch people out. It is different to our
> normal behaviour and I don't know how we highlight to users that this issue is
> present.
> 
> I'm also worried that we don't have enough people looking at rewiew and that
> there are probably issues here we need to discuss but we simply don't have the
> right people with time to look at and think about it. I'm not the subject matter
> expert here :(.
> 
> I wish I had something I could say as to how to make it acceptable to merge
> (definitely needs tests) but I really don't know this area well enough and I
> haven't had the time to think enough about it. It hovered in -next for a while
> as I was trying my best to do so but with the release breaking I needed to clear
> out -next for my own sanity and this ended in my defer pile. Even writing the
> email explaining why takes 10 minutes.

Thanks a lot for the explanation! I might come back to this at a more 
relaxed period with the suggested improvements.

regards,
Claudius