Re: [PATCH v2 11/17] images: New class wic-img for wic intregration

[Henning Schild <henning.schild@siemens.com>]

Am Mon, 16 Apr 2018 12:17:55 +0200
schrieb Claudius Heine <claudius.heine.ext@siemens.com>:

> Hi Henning,
> 
> first of, great work getting wic functional!
> 
> On 2018-04-13 16:44, [ext] Henning Schild wrote:
> > So this is the patch that does the real magic, unfortunately i have
> > to call it that ...
> > 
> > Am Fri, 13 Apr 2018 16:19:00 +0200
> > schrieb Henning Schild <henning.schild@siemens.com>:
> >   
> >> This patch integrates wic into the bitbake workflow, wic will be
> >> used for the imaging step, no need to call it manually.
> >> After all the previous reverts we now use an unmodified version of
> >> wic.
> >>
> >> Issues:
> >>    - wic was never integrated
> >>    - you always had to build an ext4-img to create a wic image
> >> later
> >>    - there was never a way to control the size of wic
> >> disks/partition, only directly in the wks
> >>    - wic used to leak the hosts bootloader into the final image
> >>
> >> Impact:
> >>    The patch solves the Issues, but drops the ability to manually
> >> start wic after bitbake. And it drops support for building wic
> >> images for Distros before stretch.
> >>
> >> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> >> ---
> >>   doc/user_manual.md                                 |   6 -
> >>   .../scripts/lib/wic/canned-wks/sdimage-efi.wks     |   2 +-
> >>   .../lib/wic/plugins/source/bootimg-efi-isar.py     | 297
> >> +++++++++++++++++++++
> >> meta/classes/wic-img.bbclass                       |  78 ++++++
> >> meta/recipes-devtools/buildchroot/buildchroot.bb   |  19 ++
> >> scripts/wic_fakeroot                               |  37 +++ 6
> >> files changed, 432 insertions(+), 7 deletions(-) create mode 100644
> >> meta-isar/scripts/lib/wic/plugins/source/bootimg-efi-isar.py create
> >> mode 100644 meta/classes/wic-img.bbclass create mode 100755
> >> scripts/wic_fakeroot
> >>
> >> diff --git a/doc/user_manual.md b/doc/user_manual.md
> >> index 058f7bd..7bd52f4 100644
> >> --- a/doc/user_manual.md
> >> +++ b/doc/user_manual.md
> >> @@ -52,16 +52,10 @@ The steps below describe how to build the
> >> images provided by default. Install the following packages:
> >>   ```
> >>   dosfstools
> >> -e2fsprogs/jessie-backports  # wic: e2fsprogs -d
> >> -gdisk                       # wic
> >>   git
> >> -grub-efi-amd64-bin          # wic
> >> UEFI: /usr/lib/grub/x86_64-efi/moddep.lst
> >> -grub-efi-ia32-bin           # wic
> >> UEFI: /usr/lib/grub/i386-efi/moddep.lst -mtools
> >> # wic FAT: mcopy debootstrap parted
> >>   python
> >> -python3                     # wic
> >>   qemu
> >>   qemu-user-static
> >>   rxvt-unicode                # build_parallel  
> > 
> > We are now running wic in buildchroot and do not need all that
> > anymore.
> > 
> > wic uses a set of tools for partitioning etc. and it has
> > dependencies on versions of these tools. In OE people know those
> > and build a native sysroot. But we have debian tools that have
> > slighly different versions.
> > 
> > That is why we carry a few workarounds to make wic still like our
> > tools, i will point them out later.
> > 
> > In OE the native sysroot contains tools that are all statically
> > linked so you can execute them without a problem. In Isar we can
> > just do that with the host tools. Things we have in buildchroot or
> > the image rootfs can only be used when chrooting in.
> > 
> > So why not keep all that and use the host tools? Things like mkfs or
> > parted are fine, but bootloader installers leak host information
> > into our images. So we would build a wheezy with a jessie grub, not
> > the best idea.
> > 
> > So i decided to run wic in buildchroot instead of the host. It also
> > shifts the whole sudo on the host discussion.
> > 
> > Running wic in buildchroot solves several problems but introduces
> > another "interesting" aspect. When doing that with wheezy or jessie
> > you are beaming wic so far into the past that the few tool
> > workarounds will not work anymore. The tools are too old and even
> > do not support certain command line switches. One example we
> > already had for jessie: 
> >> -e2fsprogs/jessie-backports  # wic: e2fsprogs -d  
> > 
> > So the pragmatic decision was to simply not support distros that
> > predate wic itself. It is unlikely we will have to deal with such
> > problems looking in the future. Because the new tools in newer
> > distros are not likely to be backward incompatible.
> >   
> >> a/meta-isar/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> >> b/meta-isar/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> >> new  
> > ...  
> >> file mode 100644 index 0000000..fccf96c --- /dev/null
> >> +            grubefi_conf += "menuentry 'boot'{\n"
> >> +
> >> +            kernel = "/vmlinuz"  
> > 
> > I did not find a better way than to fork the plugins. But they need
> > to be slightly different to what we get from OE. One example is the
> > name of the kernel that is hardcoded there.
> > 
> > But the diffs are small and should not be hard to maintain.  
> 
> Would it make sense to commit first the unmodified plugins from oe
> here, and then patch them to work with isar? That way maintaining
> those patches could be easier, couldn't it?

For sure, i thought about that as well and then decided a simple diff
would be good enough. But you are right, i will split it up to do that
and review all changes again.

> >   
> >> diff --git a/meta/classes/wic-img.bbclass
> >> b/meta/classes/wic-img.bbclass new file mode 100644
> >> index 0000000..d1deff3
> >> --- /dev/null
> >> +++ b/meta/classes/wic-img.bbclass
> >> @@ -0,0 +1,78 @@
> >> +# This software is a part of ISAR.
> >> +# Copyright (C) 2018 Siemens AG
> >> +#
> >> +# this class is heavily inspired by
> >> OEs ./meta/classes/image_types_wic.bbclass +#
> >> +
> >> +WKS_FILE ?= "sdimage-efi"
> >> +ROOTFS_TYPE ?= "ext4"
> >> +
> >> +STAGING_DATADIR ?= "/usr/lib/"
> >> +STAGING_LIBDIR ?= "/usr/lib/"
> >> +STAGING_DIR ?= "${TMPDIR}"
> >> +IMAGE_BASENAME ?= "multiconfig:${MACHINE}-${DISTRO}:${PN}"  
> 
> I currently don't understand your value for the IMAGE_BASENAME. Why
> does it have multiconfig in its name? Also if those should be valid
> bitbake targets, DISTRO is set to "debian-stretch" resulting in 
> 'multiconfig:qemuamd64-debian-stretch:isar-image-base' and that 
> multiconfig doesn't exist AFAIK.

To be honest i will have to check that again. I think there is an
expectation in wic when it is looking for the .env file.

> >> +FAKEROOTCMD ?= "${ISARROOT}/scripts/wic_fakeroot"
> >> +RECIPE_SYSROOT_NATIVE ?= "/"
> >> +
> >> +do_wic_image[stamp-extra-info] = "${DISTRO}-${MACHINE}"
> >> +
> >> +WIC_CREATE_EXTRA_ARGS ?= ""
> >> +
> >> +WICVARS ?= "\
> >> +           BBLAYERS IMGDEPLOYDIR DEPLOY_DIR_IMAGE FAKEROOTCMD
> >> IMAGE_BASENAME IMAGE_BOOT_FILES \
> >> +           IMAGE_LINK_NAME IMAGE_ROOTFS INITRAMFS_FSTYPES INITRD
> >> INITRD_LIVE ISODIR RECIPE_SYSROOT_NATIVE \
> >> +           ROOTFS_SIZE STAGING_DATADIR STAGING_DIR STAGING_LIBDIR
> >> TARGET_SYS TRANSLATED_TARGET_ARCH" +
> >> +# Isar specific vars used in our plugins
> >> +WICVARS += "KERNEL_IMAGE INITRD_IMAGE DISTRO_ARCH"
> >> +
> >> +python do_rootfs_wicenv () {
> >> +    wicvars = d.getVar('WICVARS', True)
> >> +    if not wicvars:
> >> +        return
> >> +
> >> +    stdir = d.getVar('STAGING_DIR', True)
> >> +    outdir = os.path.join(stdir, d.getVar('MACHINE', True),
> >> 'imgdata')
> >> +    bb.utils.mkdirhier(outdir)
> >> +    basename = d.getVar('IMAGE_BASENAME', True)
> >> +    with open(os.path.join(outdir, basename) + '.env', 'w') as
> >> envf:
> >> +        for var in wicvars.split():
> >> +            value = d.getVar(var, True)
> >> +            if value:
> >> +                envf.write('%s="%s"\n' % (var, value.strip()))
> >> +
> >> +    # this part is stolen from
> >> OE ./meta/recipes-core/meta/wic-tools.bb
> >> +    with open(os.path.join(outdir, "wic-tools.env"), 'w') as envf:
> >> +        for var in ('RECIPE_SYSROOT_NATIVE', 'STAGING_DATADIR',
> >> 'STAGING_LIBDIR'):
> >> +            envf.write('%s="%s"\n' % (var, d.getVar(var,
> >> True).strip())) +
> >> +}
> >> +
> >> +addtask do_rootfs_wicenv after do_copy_boot_files before
> >> do_wic_image +do_rootfs_wicenv[vardeps] += "${WICVARS}"
> >> +do_rootfs_wicenv[prefuncs] = 'set_image_size'
> >> +
> >> +WIC_IMAGE_FILE
> >> ="${DEPLOY_DIR_IMAGE}/${PN}-${DISTRO}-${MACHINE}.wic.img" +
> >> +do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
> >> +
> >> +do_wic_image() {
> >> +    if ! grep -q ${BUILDCHROOT_DIR}/dev /proc/mounts; then
> >> +        sudo mount -t devtmpfs -o mode=0755,nosuid devtmpfs
> >> ${BUILDCHROOT_DIR}/dev
> >> +        sudo mount -t proc none ${BUILDCHROOT_DIR}/proc
> >> +    fi
> >> +    for dir in ${BBLAYERS} ${STAGING_DIR} ${ISARROOT}/scripts; do
> >> +	sudo mkdir -p ${BUILDCHROOT_DIR}/$dir
> >> +        sudo mount --bind $dir ${BUILDCHROOT_DIR}/$dir
> >> +    done
> >> +    export FAKEROOTCMD=${FAKEROOTCMD}
> >> +    export BUILDDIR=${BUILDDIR}
> >> +    export MTOOLS_SKIP_CHECK=1  
> > 
> > This is one of the tool workarounds, where i reverted a patch
> > against wic.
> >   
> >> +
> >> +    sudo -E chroot ${BUILDCHROOT_DIR} ${ISARROOT}/scripts/wic
> >> create ${WKS_FILE} --vars "${STAGING_DIR}/${MACHINE}/imgdata/"
> >> -o /tmp/ -e ${IMAGE_BASENAME} ${WIC_CREATE_EXTRA_ARGS}
> >> +    cp -f `ls -t -1 ${BUILDCHROOT_DIR}/tmp/${WKS_FILE}*.direct |
> >> head -1` ${WIC_IMAGE_FILE} +}
> >> +
> >> +do_wic_image[depends] = "buildchroot:do_build"
> >> +
> >> +addtask wic_image before do_build after do_copy_boot_files
> >> diff --git a/meta/recipes-devtools/buildchroot/buildchroot.bb
> >> b/meta/recipes-devtools/buildchroot/buildchroot.bb index
> >> b16e63a..bd4d003 100644 ---
> >> a/meta/recipes-devtools/buildchroot/buildchroot.bb +++
> >> b/meta/recipes-devtools/buildchroot/buildchroot.bb @@ -28,6 +28,25
> >> @@ BUILDCHROOT_PREINSTALL ?= "gcc \ devscripts \
> >>                              equivs"
> >>   
> >> +BUILDCHROOT_PREINSTALL_WIC = " \
> >> +                             parted \
> >> +                             gdisk \
> >> +                             util-linux \
> >> +                             syslinux \
> >> +                             syslinux-common \
> >> +                             dosfstools \
> >> +                             mtools \
> >> +                             e2fsprogs \
> >> +                             grub-efi-amd64-bin \
> >> +                             grub-efi-ia32-bin \
> >> +                             python3"
> >> +
> >> +python () {
> >> +    if d.getVar('IMAGE_TYPE', True) == 'wic-img':  
> 
> That limits how many images types one image recipe has. With OE you 
> could create multiple image types from one image recipe. Here you
> only have one. That might be fine. But just as a note. I currently
> don't have a usecase to support multiple image types for one image.

I actually had "'wic-img' in IMAGE_TYPE" there, which broke building
external layers. IMAGE_TYPE currently is a string an not an array. If
we ever switch to an array, that line needs to change. But right now
it is a string.

Henning
 
> OE supports different post-processes to the wic image. For instance 
> 'wic.xz' creates a compressed wic file and 'wic.bmap' creates a 
> bmap-tool file in addition to the wic image.
> 
> I don't know if you currently have plans to implement such
> expansions, but maybe think about how such could be implemented with
> your current design.
> 
> regards,
> Claudius
> 
> >> +        d.appendVar('BUILDCHROOT_PREINSTALL',
> >> +                    d.getVar('BUILDCHROOT_PREINSTALL_WIC', True))
> >> +}  
> > 
> > Now the wic tools become part of buildchroot if you chose that
> > IMAGE_TYPE.
> >   
> >>   WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}"
> >>   
> >>   do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
> >> diff --git a/scripts/wic_fakeroot b/scripts/wic_fakeroot
> >> new file mode 100755
> >> index 0000000..9e01c38
> >> --- /dev/null
> >> +++ b/scripts/wic_fakeroot
> >> @@ -0,0 +1,37 @@
> >> +#!/usr/bin/env python3
> >> +#
> >> +# wic needs a FAKEROOT cmd to run, the default is pseudo. In Isar
> >> we do/can not +# use pseudo. And we call wic as root to begin
> >> with, so this script could be a +# dummy doing nothing. It is
> >> almost a dummy ... +#
> >> +# If the fsck hack ever becomes obsolete, FAKEROOTCMD ?= "true;"
> >> can be used +#
> >> +# This software is a part of Isar.
> >> +# Copyright (C) 2018 Siemens AG
> >> +#
> >> +import os
> >> +import sys
> >> +import shutil
> >> +import subprocess
> >> +
> >> +args = sys.argv
> >> +args.pop(0)
> >> +cmd = args[0]
> >> +
> >> +# expect to be running as root
> >> +# we could loosen that and execv(sudo, args) but even some early
> >> +# "du"s fail, which do not use the fakeroot-wrapper
> >> +#  i.e. in wics partition.py the "du -ks" fails on
> >> +#    var/cache/apt/archives/partial
> >> +#    rootfs/root ...
> >> +assert 'root' == os.environ["USER"]
> >> +
> >> +# e2fsck <= 1.43.5 returns 1 on non-errors (stretch and before
> >> affected) +# treat 1 as safe ... the filesystem was successfully
> >> repaired and is OK +if cmd.startswith('fsck.'):
> >> +    ret = subprocess.call(args)
> >> +    if ret == 0 or ret == 1:
> >> +        sys.exit(0)
> >> +    sys.exit(ret)  
> > 
> > This is another tool workaround, now outside of wic.
> >   
> >> +
> >> +os.execv(shutil.which(cmd), args)  
> >   
>