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

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

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.

>  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.

> +                # 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
> +
>      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')