Re: [RFC 1/1] delay creation of initrd until end of rootfs install

[Uladzimir Bely <ubely@ilbers.de>]

On Mon, 2024-03-25 at 17:22 +0100, Gylstorff Quirin wrote:
> 
> 
> On 1/18/24 6:06 PM, 'Jan Kiszka' via isar-users wrote:
> > On 18.01.24 17:44, Moessbauer, Felix (T CED OES-DE) wrote:
> > > On Fri, 2023-12-08 at 10:46 +0000, 'MOESSBAUER, Felix' via isar-
> > > users
> > > wrote:
> > > > On Wed, 2023-12-06 at 09:04 +0900, Jan Kiszka wrote:
> > > > > On 04.12.23 10:36, Felix Moessbauer wrote:
> > > > > > This patch solves major performance issues around the
> > > > > > initramfs
> > > > > > creation by ensuring that the initrd is only created once.
> > > > > > This
> > > > > > is
> > > > > > implemented by stubbing the update-initramfs call during
> > > > > > the
> > > > > > package
> > > > > > installing. After all apt operations are completed, we
> > > > > > manually
> > > > > > trigger the initrd creation. In case a custom initramfs is
> > > > > > used,
> > > > > > the
> > > > > > creation is completely skipped in the image rootfs, as this
> > > > > > would
> > > > > > anyways not be used.
> > > > > > 
> > > > > > Before that, each package install that made a initrd
> > > > > > relevant
> > > > > > change
> > > > > > triggered the update of the initrd. As we have multiple apt
> > > > > > calls
> > > > > > during
> > > > > > the build, this step was sometimes executed multiple times.
> > > > > > In
> > > > > > addition,
> > > > > > the apt install step is emulated, further slowing down the
> > > > > > initrd
> > > > > > generation. On some layers on non native architecutes, this
> > > > > > summed
> > > > > > up to
> > > > > > over 10 minutes of initrd generation time.
> > > > > > 
> > > > > > Signed-off-by: Felix Moessbauer
> > > > > > <felix.moessbauer@siemens.com>
> > > > > > ---
> > > > > >   meta/classes/image.bbclass                    |  4 +++
> > > > > >   meta/classes/rootfs.bbclass                   | 32
> > > > > > +++++++++++++++++++
> > > > > >   .../isar-bootstrap/isar-bootstrap.inc         |  2 ++
> > > > > >   3 files changed, 38 insertions(+)
> > > > > > 
> > > > > > diff --git a/meta/classes/image.bbclass
> > > > > > b/meta/classes/image.bbclass
> > > > > > index 73f1d52c..39addc59 100644
> > > > > > --- a/meta/classes/image.bbclass
> > > > > > +++ b/meta/classes/image.bbclass
> > > > > > @@ -72,6 +72,8 @@ inherit essential
> > > > > >   
> > > > > >   ROOTFSDIR = "${IMAGE_ROOTFS}"
> > > > > >   ROOTFS_FEATURES += "clean-package-cache clean-pycache
> > > > > > generate-
> > > > > > manifest export-dpkg-status clean-log-files clean-debconf-
> > > > > > cache"
> > > > > > +# when using a custom initrd, do not generate one as part
> > > > > > of the
> > > > > > image rootfs
> > > > > > +ROOTFS_FEATURES += "${@ '' if d.getVar('INITRD_IMAGE') ==
> > > > > > ''
> > > > > > else
> > > > > > 'no-generate-initrd'}"
> > > > > >   ROOTFS_PACKAGES += "${IMAGE_PREINSTALL}
> > > > > > ${@isar_multiarch_packages('IMAGE_INSTALL', d)}"
> > > > > >   ROOTFS_MANIFEST_DEPLOY_DIR ?= "${DEPLOY_DIR_IMAGE}"
> > > > > >   ROOTFS_DPKGSTATUS_DEPLOY_DIR ?= "${DEPLOY_DIR_IMAGE}"
> > > > > > @@ -488,6 +490,8 @@ do_rootfs_quality_check() {
> > > > > >       args="${ROOTFS_QA_FIND_ARGS}"
> > > > > >       # rootfs_finalize chroot-setup.sh
> > > > > >       args="${args} ! -path
> > > > > > ${ROOTFSDIR}/var/lib/dpkg/diversions"
> > > > > > +    # initramfs is generated outside of the image rootfs
> > > > > > +    args="${args} ! -path ${ROOTFSDIR}/boot/initrd.img*"
> > > > > >       for cmd in ${ROOTFS_POSTPROCESS_COMMAND}; do
> > > > > >           case "${cmd}" in
> > > > > >              image_postprocess_mark)
> > > > > > diff --git a/meta/classes/rootfs.bbclass
> > > > > > b/meta/classes/rootfs.bbclass
> > > > > > index 1b95115a..69e38dac 100644
> > > > > > --- a/meta/classes/rootfs.bbclass
> > > > > > +++ b/meta/classes/rootfs.bbclass
> > > > > > @@ -14,6 +14,7 @@ ROOTFS_BASE_DISTRO ?= "${BASE_DISTRO}"
> > > > > >   # 'generate-manifest' - generate a package manifest of
> > > > > > the
> > > > > > rootfs
> > > > > > into ${ROOTFS_MANIFEST_DEPLOY_DIR}
> > > > > >   # 'export-dpkg-status' - exports /var/lib/dpkg/status
> > > > > > file to
> > > > > > ${ROOTFS_DPKGSTATUS_DEPLOY_DIR}
> > > > > >   # 'clean-log-files' - delete log files that are not owned
> > > > > > by
> > > > > > packages
> > > > > > +# 'no-generate-initrd' - do not generate debian default
> > > > > > initrd
> > > > > >   ROOTFS_FEATURES ?= ""
> > > > > >   
> > > > > >   ROOTFS_APT_ARGS="install --yes -o
> > > > > > Debug::pkgProblemResolver=yes"
> > > > > > @@ -117,6 +118,16 @@ rootfs_configure_apt() {
> > > > > >   EOSUDO
> > > > > >   }
> > > > > >   
> > > > > > +ROOTFS_CONFIGURE_COMMAND +=
> > > > > > "rootfs_disable_initrd_generation"
> > > > > > +rootfs_disable_initrd_generation[weight] = "1"
> > > > > > +rootfs_disable_initrd_generation() {
> > > > > > +    # fully disable initrd generation
> > > > > > +    echo "replace update-initramfs with stub"
> > > > > > +    sudo mv "${ROOTFSDIR}/usr/sbin/update-initramfs" \
> > > > > > +            "${ROOTFSDIR}/usr/sbin/update-initramfs.isar"
> > > > > > +    sudo chroot "${ROOTFSDIR}" ln -s "/usr/bin/true"
> > > > > > "/usr/sbin/update-initramfs"
> > > > > > +}
> > > > > > +
> > > > > >   
> > > > > >   ROOTFS_INSTALL_COMMAND += "rootfs_install_pkgs_update"
> > > > > >   rootfs_install_pkgs_update[weight] = "5"
> > > > > > @@ -310,6 +321,27 @@ rootfs_cleanup_isar_apt() {
> > > > > >   EOSUDO
> > > > > >   }
> > > > > >   
> > > > > > +ROOTFS_POSTPROCESS_COMMAND +=
> > > > > > "rootfs_restore_initrd_tooling"
> > > > > > +rootfs_generate_initrd[weight] = "1"
> > > > > > +rootfs_restore_initrd_tooling() {
> > > > > > +    if [ -e "${ROOTFSDIR}/usr/sbin/update-initramfs.isar"
> > > > > > ];
> > > > > > then
> > > > > > +        sudo mv -f "${ROOTFSDIR}/usr/sbin/update-
> > > > > > initramfs.isar"
> > > > > > \
> > > > > > +            "${ROOTFSDIR}/usr/sbin/update-initramfs"
> > > > > > +    fi
> > > > > > +}
> > > > > > +
> > > > > > +ROOTFS_POSTPROCESS_COMMAND +=
> > > > > > "${@bb.utils.contains('ROOTFS_FEATURES', 'no-generate-
> > > > > > initrd',
> > > > > > '',
> > > > > > 'rootfs_generate_initrd', d)}"
> > > > > > +rootfs_generate_initrd[weight] = "10"
> > > > > > +rootfs_generate_initrd() {
> > > > > > +    if [ -n "$(sudo find '${ROOTFSDIR}/boot' -type f -name
> > > > > > 'vmlinu[xz]*')" ]; then
> > > > > > +        sudo -E chroot "${ROOTFSDIR}" sh -c '\
> > > > > > +            export kernel_version=$(basename
> > > > > > /boot/vmlinu[xz]* |
> > > > > > cut -d'-' -f2-); \
> > > > > > +            update-initramfs -u -v -k "$kernel_version";'
> > > > > > +    else
> > > > > > +        echo "no kernel in this rootfs, do not generate
> > > > > > initrd"
> > > > > > +    fi
> > > > > > +}
> > > > > > +
> > > > > >   do_rootfs_postprocess[vardeps] =
> > > > > > "${ROOTFS_POSTPROCESS_COMMAND}"
> > > > > >   do_rootfs_postprocess[network] = "${TASK_USE_SUDO}"
> > > > > >   python do_rootfs_postprocess() {
> > > > > > diff --git a/meta/recipes-core/isar-bootstrap/isar-
> > > > > > bootstrap.inc
> > > > > > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > > > > > index 3477c2fb..4c6011bc 100644
> > > > > > --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > > > > > +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > > > > > @@ -24,6 +24,8 @@ DISTRO_BOOTSTRAP_KEYFILES = ""
> > > > > >   THIRD_PARTY_APT_KEYFILES = ""
> > > > > >   DEPLOY_ISAR_BOOTSTRAP ?= ""
> > > > > >   DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
> > > > > > +# install early, so we can stub the update-initramfs
> > > > > > script
> > > > > > before
> > > > > > rootfs install
> > > > > > +DISTRO_BOOTSTRAP_BASE_PACKAGES:append = ",initramfs-tools"
> > > > > >   DISTRO_BOOTSTRAP_BASE_PACKAGES:append:gnupg = ",gnupg"
> > > > > >   DISTRO_BOOTSTRAP_BASE_PACKAGES:append:https-support =
> > > > > > ",ca-
> > > > > > certificates"
> > > > > >   DISTRO_VARS_PREFIX ?= "${@'HOST_' if
> > > > > > d.getVar('BOOTSTRAP_FOR_HOST') == '1' else ''}"
> > > > > 
> > > > > The idea is quite nice for us in Isar, though I'd also
> > > > > eventually
> > > > > like
> > > > > to improve Debian itself in this regard. The building blocks
> > > > > are
> > > > > there
> > > > > in upstream but they do not fully work.
> > > > 
> > > > Yes, definitely. I reported this on the debian-kernel ML as
> > > > well:
> > > > https://lists.debian.org/debian-kernel/2023/12/msg00097.html
> > > > 
> > > > Anyways, are there more objections against this patch?
> > > 
> > > Any news on this? This is on the ML for quite some time without
> > > objections. If required, I can send a rebased version.
> > > 
> > 
> > I assume this is held back now by the current -rc cycle. Likely an
> > improvement after 0.10.
> > 
> > Jan
> > 
> 
> As 0.10 was released,  can we merge this?
> 
> Quirin

Hello all.

I run it in CI and delaying initrd creation seems to cause errors for
some targets:

build/tmp/work/debian-bullseye-armhf/isar-image-base-imx6-
sabrelite/1.0-r0/temp/log.do_image_fit:

```
FATAL ERROR: Couldn't open "/home/builder/isar-image-base-imx6-
sabrelite/deploy/isar-image-base-debian-bullseye-imx6-sabrelite-
initrd.img": No such file or directory
/usr/bin/mkimage: Can't open /home/builder/isar-image-base-imx6-
sabrelite/deploy/isar-image-base-debian-bullseye-imx6-
sabrelite.fit.tmp: No such file or directory
```

There were more patchsets applied for this run, but I think that others
are not related.