Re: [OE-core] [PATCH] runqemu: support uImage kernel

["Bruce Ashfield" <bruce.ashfield@gmail.com>]

On Fri, Apr 16, 2021 at 12:43 PM Adrian Freihofer
<adrian.freihofer@gmail.com> wrote:
>
> u-boot's uImage kernel format is not supported by Qemu. Extract the
> kernel from the uImage to a temporary directory.
>

I'd suggest that we should document in the commit log why someone
wouldn't just build a kernel image format that is already supported by
runqemu (which is what I typically do). I'm guessing it is some sort
of h/w boot being re-purposed to qemu emulation, and this is to avoid
building a second kernel image ?

Additionally, this should have some sort of test, or the code is going
to bitrot due to not being run by anything in core.

> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  scripts/runqemu | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/runqemu b/scripts/runqemu
> index ba0b701aff..fa630fc7a4 100755
> --- a/scripts/runqemu
> +++ b/scripts/runqemu
> @@ -163,6 +163,7 @@ class BaseConfig(object):
>          self.bios = ''
>          self.kernel_cmdline = ''
>          self.kernel_cmdline_script = ''
> +        self.temp_kernel_dir = None
>          self.bootparams = ''
>          self.dtb = ''
>          self.fstype = ''
> @@ -1422,10 +1423,26 @@ class BaseConfig(object):
>          self.setup_serial()
>          self.setup_vga()
>
> +    def dump_uimage(self):
> +        if self.kernel and self.get('KERNEL_IMAGETYPE') == 'uImage':
> +            import tempfile
> +            self.temp_kernel_dir = tempfile.mkdtemp()
> +            kernel = os.path.join(self.temp_kernel_dir, 'image')
> +            dumpimage_bin = os.path.join(self.bindir_native, 'dumpimage')
> +            image_link_name = self.get('IMAGE_LINK_NAME')
> +            logger.info('Running: %s' % str([dumpimage_bin, "-o ", kernel, self.kernel]))
> +            staging_bindir_native = '%s/usr/bin' % self.get('STAGING_DIR_NATIVE')
> +            dumpimage_bin = os.path.join(staging_bindir_native, 'dumpimage')
> +            subprocess.run([dumpimage_bin, "-p", "0", "-o", kernel, self.kernel])
> +            return kernel
> +        return self.kernel
> +
>      def start_qemu(self):
>          import shlex
> -        if self.kernel:
> -            kernel_opts = "-kernel %s -append '%s %s %s %s'" % (self.kernel, self.kernel_cmdline,
> +
> +        kernel = self.dump_uimage()
> +        if kernel:
> +            kernel_opts = "-kernel %s -append '%s %s %s %s'" % (kernel, self.kernel_cmdline,
>                                                                  self.kernel_cmdline_script, self.get('QB_KERNEL_CMDLINE_APPEND'),
>                                                                  self.bootparams)

If the kernel never fails from the dup_uimage() command, why test for
it ? I don't see how the routine could return something that doesn't
pass the test. If so, aren't we going to run into more cryptic errors
(or traceback) later if there was some sort of failure.

The existing code doesn't look like it is completely error proofed
either, that just came to mind when looking at the patch.

Bruce

>              if self.bios:
> @@ -1481,6 +1498,9 @@ class BaseConfig(object):
>              shutil.rmtree(self.rootfs)
>              shutil.rmtree('%s.pseudo_state' % self.rootfs)
>
> +        if self.temp_kernel_dir:
> +            shutil.rmtree(self.temp_kernel_dir)
> +
>          self.cleaned = True
>
>      def run_bitbake_env(self, mach=None):
> --
> 2.26.3
>
>
> 
>


-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II