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

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

Am Wed, 02 Nov 2022 16:16:04 +0300
schrieb 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.

Yes. It might even be better to add some empty space in the wks files
we use. That would also make the example images more useful since
people could install additional packages when running them in qemu.
While we could add extra space to that last partition, an empty padding
would allow for testing the expand package and would allow the example
images to be flashed to much bigger disks where they would make all
space available.
So i think i will try and add some few hundred MBs of empty space in
the wks and not create an image copy with "qemu-img resize".

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

Ok. If done via wks i would probably switch the expectation to a
bitbake variable WKS_FILE. But would still have to detect ubuntu.

Maybe i can get ubuntu to be more verbose and always expect to find
traces of a resize when it should have happened.

> > > +                # 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?

It is a copy that did add extra empty space at the end using "qemu-img
resize". But with the wks approach we can likely get that more elegant
and add value for users of the example images.

Henning

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