Re: [PATCH 2/2] CI: install expand-on-first-boot in all tests and also test

[Uladzimir Bely <ubely@ilbers.de>]

In the email from Friday, 28 October 2022 17:32:30 +03 user Henning Schild 
wrote:
> I am not sure i like this one. Keep reading inline.
> 
> Am Fri, 28 Oct 2022 16:21:45 +0200
> 
> schrieb Henning Schild <henning.schild@siemens.com>:
> > Install the package in any rootfs we build. And also resize a few
> > images when we boot them and check the output.
> > 
> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> > 
> >  meta-isar/conf/local.conf.sample      |  2 +-
> >  meta-isar/conf/machine/qemuamd64.conf |  1 +
> >  testsuite/cibuilder.py                | 18 +++++++++++++++---
> >  testsuite/start_vm.py                 | 11 +++++++++++
> >  4 files changed, 28 insertions(+), 4 deletions(-)
> > 
> > diff --git a/meta-isar/conf/local.conf.sample
> > b/meta-isar/conf/local.conf.sample index 57d062025f21..27becf19aefd
> > 100644 --- a/meta-isar/conf/local.conf.sample
> > +++ b/meta-isar/conf/local.conf.sample
> > @@ -198,7 +198,7 @@ CONF_VERSION = "1"
> > 
> >  #
> >  # The default list of extra packages to be installed.
> > 
> > -IMAGE_INSTALL = "hello-isar example-raw
> > example-module-${KERNEL_NAME} enable-fsck isar-exclude-docs samefile
> > hello isar-disable-apt-cache cowsay example-prebuilt" +IMAGE_INSTALL
> > = "hello-isar example-raw example-module-${KERNEL_NAME} enable-fsck
> > isar-exclude-docs samefile hello isar-disable-apt-cache cowsay
> > example-prebuilt expand-on-first-boot" # # Enable cross-compilation
> > support diff --git a/meta-isar/conf/machine/qemuamd64.conf
> > b/meta-isar/conf/machine/qemuamd64.conf index
> > eca2628c4ae5..6e3561901447 100644 ---
> > a/meta-isar/conf/machine/qemuamd64.conf +++
> > b/meta-isar/conf/machine/qemuamd64.conf @@ -19,6 +19,7 @@ QEMU_ARCH
> > ?= "x86_64" QEMU_MACHINE ?= "q35" QEMU_CPU ?= ""
> > 
> >  QEMU_DISK_ARGS ?= "-hda ##ROOTFS_IMAGE## -bios
> > 
> > /usr/share/ovmf/OVMF.fd" +QEMU_DISK_RESIZE ?= "1"
> 
> I hate that i need this. But it seems to be the way to talk to the
> testsuite. Has nothing to do with the machine otherwise.
> 

Probably, it is better to place this in local.conf using machine/distro 
overrides. So, just selected (overrided) targets would be affected and ubuntu 
won't require any workarounds.

> >  MACHINE_SERIAL ?= "ttyS0"
> >  BAUDRATE_TTY ?= "115200"
> > 
> > diff --git a/testsuite/cibuilder.py b/testsuite/cibuilder.py
> > index 07a9edc56d9c..fb0daedc7d49 100755
> > --- a/testsuite/cibuilder.py
> > +++ b/testsuite/cibuilder.py
> > 
> > @@ -231,6 +231,15 @@ class CIBuilder(Test):
> >          login_prompt = b'isar login:'
> >          # the printk of recipes-kernel/example-module
> >          module_output = b'Just an example'
> > 
> > +        # output we see when expand-on-first-boot runs on ext4
> > +        resize_output = b'resized filesystem to'
> > +        expecting_resize = False
> > +        for arg in cmdline:
> 
> > +            if arg.endswith(".wic.resized"):
> Here i do not have that magic variable. So i guess from the filename.
> 

cibuilder.py imports start_vm.py, which has some helper functions to get 
variables from "bitbake -e" output parsing. So, technically the variable 
should be available through get_bitbake_env/get_bitbake_var functions.

> > +                # in ubuntu the resize works but no trace in boot log
> 
> > +                if not 'ubuntu' in arg:
> same here, i guess from the filename which distro my suite is, while
> there is a variable distro ... which is a suite
> 
> There is a similar guess somewhere in the code
> 
> >> base = 'ubuntu' if distro in ['focal', 'bionic'] else 'debian'
> > 
> > +                    expecting_resize = True
> > +                    break
> > 
> >          timeout = time.time() + int(time_to_wait)
> > 
> > @@ -265,8 +274,11 @@ class CIBuilder(Test):
> >              with open(output_file, "rb") as f1:
> >                  data = f1.read()
> > 
> >                  if module_output in data and login_prompt in data:
> > -                    return
> > -                else:
> > -                    app_log.error(data.decode(errors='replace'))
> > +                    if expecting_resize:
> > +                        if resize_output in data:
> > +                            return
> > +                    else:
> > +                        return
> > +                app_log.error(data.decode(errors='replace'))
> > 
> >          self.fail('Log ' + output_file)
> > 
> > diff --git a/testsuite/start_vm.py b/testsuite/start_vm.py
> > index f761a8bda789..2262af557a32 100755
> > --- a/testsuite/start_vm.py
> > +++ b/testsuite/start_vm.py
> > @@ -5,6 +5,7 @@
> > 
> >  import argparse
> >  import os
> > 
> > +import shutil
> > 
> >  import subprocess
> >  import sys
> >  import time
> > 
> > @@ -57,6 +58,7 @@ def format_qemu_cmdline(arch, build, distro, out,
> > pid, enforce_pcbios=False): qemu_machine = get_bitbake_var(bb_output,
> > 'QEMU_MACHINE') qemu_cpu = get_bitbake_var(bb_output, 'QEMU_CPU')
> > 
> >      qemu_disk_args = get_bitbake_var(bb_output, 'QEMU_DISK_ARGS')
> > 
> > +    qemu_disk_resize = get_bitbake_var(bb_output,
> > 'QEMU_DISK_RESIZE') == "1"
> > 
> >      if out:
> >          extra_args.extend(['-chardev','stdio,id=ch0,logfile=' + out])
> > 
> > @@ -65,6 +67,15 @@ def format_qemu_cmdline(arch, build, distro, out,
> > 
> > pid, enforce_pcbios=False): if pid:
> >          extra_args.extend(['-pidfile', pid])
> > 
> > +    if qemu_disk_resize:
> Here i smuggle in a file copy in what should just give me a cmdline for
> qemu. But is is the only place where all information seems available.
> 
> And i leave that magic copy forever, wasting space and use a magic
> suffix for the caller to know to look for log traces of resizing.
> 
> As i said. I am not happy. But wanted to show that for discussion and
> maybe get feedback on how to do things better.
> 
> The patches ran successfully on ilbers-ci. It might not be pretty, but
> it works. And might help us get that expand thingy more stable and keep
> it stable.
> 
> Henning
> 
> > +        suffix = ".resized"
> > +        abs_src = os.path.join(deploy_dir_image, rootfs_image)
> > +        abs_dst = abs_src + suffix
> > +        if not os.path.exists(abs_dst):
> > +            shutil.copy(abs_src, abs_dst)
> > +            subprocess.run(["qemu-img", "resize", abs_dst, "+2G"])
> > +        rootfs_image += suffix
> > +
> > 

Is this ".resized" image just a something allowed to be "spoiled" by qemu in 
order to have original file untouched?

If yes, it might be OK, but if it's just a hint for the code above, I would 
prefer to remove this code and parse "bitbake -e" in "cbuilder.py".

> >      qemu_disk_args = qemu_disk_args.replace('##ROOTFS_IMAGE##',
> > 
> > deploy_dir_image + '/' + rootfs_image).split() if enforce_pcbios and
> > '-bios' in qemu_disk_args: bios_idx = qemu_disk_args.index('-bios')


-- 
Uladzimir Bely