Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

[Igor Mammedov]

On Thu, 14 Feb 2013 09:18:53 -0200
Eduardo Habkost <address@hidden> wrote:

> On Wed, Feb 13, 2013 at 05:17:44PM +0100, Igor Mammedov wrote:
> > qemu boots from disk image 3 times faster than direct kernel load.
> 
> That's surprising. Do you have any idea why that happens?
> 
> (CCing qemu-devel in case other QEMU developers can explain it)
> 
> 
> > Use grub2-mkrescue tool to create boot image with test kernel if available
> > and do image boot then.
> > On FC17 it reduces 1 VM run from ~15 sec to ~5sec.
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >  qemu/tests/cpuid.py                    |   11 +++++++++--
> >  shared/deps/cpuid_test_kernel/Makefile |   18 ++++++++++++++++--
> >  shared/deps/cpuid_test_kernel/grub.cfg |    8 ++++++++
> >  3 files changed, 33 insertions(+), 4 deletions(-)
> >  create mode 100644 shared/deps/cpuid_test_kernel/grub.cfg
> > 
> > diff --git a/qemu/tests/cpuid.py b/qemu/tests/cpuid.py
> > index 5065c6a..731411c 100644
> > --- a/qemu/tests/cpuid.py
> > +++ b/qemu/tests/cpuid.py
> > @@ -118,13 +118,20 @@ def run_cpuid(test, params, env):
> >                                         "cpuid_test_kernel")
> >          os.chdir(test_kernel_dir)
> >          utils.make("cpuid_dump_kernel.bin")
> > +        utils.make("boot.img", ignore_status=True)
> > +        cmdres = utils.run("cd %s && ls boot.img" % test_kernel_dir,
> > ignore_status=True)
> 
> Why fork a shell just to check if a file exists? You can simply use
> "os.path.exists('%s/boot.img' % (test_kernel_dir))" or
> "os.path.exists(os.path.join(test_kernel_dir, 'boot.img'))".
Thanks, I'll fix it.

> 
> The rest of the patch looks good to me.
> 
> I wonder if we could make this a generic "boot kernel image" function,
> that uses grub2-mkrescue if available, and -kernel otherwise. I believe
> other test cases may benefit for a general mechanism to boot test
> kernels.
I'd make something like make_boot_image() a generic, which would
  * take kernel, (optional initrd, kernel options), root tree directory
  * create boot image using host available tools (which is greatly distro
    specific depending on available tools)
and leave decision what boot method to use to test cases.

But due to lack of free time, I went through easy road.
I'd glad to amend patch, if there are suggestions how to generalize it and
improve "create boot image" process for distros that doesn't have
grub2-mkrescue.


> 
> >  
> >          vm_name = params.get('main_vm')
> >          params_b = params.copy()
> > -        params_b["kernel"] = os.path.join(test_kernel_dir,
> > "cpuid_dump_kernel.bin")
> > +        if cmdres.exit_status == 0:
> > +            params_b["image_name_custom"] =
> > os.path.join(test_kernel_dir, "boot.img")
> > +            params_b["image_format_custom"] = ""
> > +            params_b["images"] = "custom"
> > +        else:
> > +            params_b["kernel"] = os.path.join(test_kernel_dir,
> > "cpuid_dump_kernel.bin")
> > +            del params_b["images"]
> >          params_b["cpu_model"] = cpu_model
> >          params_b["cpu_model_flags"] = feature
> > -        del params_b["images"]
> >          del params_b["nics"]
> >          env_process.preprocess_vm(self, params_b, env, vm_name)
> >          vm = env.get_vm(vm_name)
> > diff --git a/shared/deps/cpuid_test_kernel/Makefile
> > b/shared/deps/cpuid_test_kernel/Makefile index 0e34c43..5999804 100644
> > --- a/shared/deps/cpuid_test_kernel/Makefile
> > +++ b/shared/deps/cpuid_test_kernel/Makefile
> > @@ -1,10 +1,24 @@
> >  CFLAGS  := -m32 -fno-stack-protector -fno-builtin -nostdinc -O -g -Wall
> > -I. LDFLAGS := -nostdlib -Wl,-N -Wl,-Ttext -Wl,100000
> >  
> > -all: cpuid_dump_kernel.bin
> > +boot := $(shell sh -c 'which grub2-mkrescue 2>/dev/null && echo
> > grub2boot.img') +
> > +all: cpuid_dump_kernel.bin boot.img
> >  
> >  cpuid_dump_kernel.bin: boot.S main.c test.c
> >     $(CC) -o $@ $(CFLAGS) $^ $(LDFLAGS)
> > +   ln -s $@ kernel.bin
> > +
> > +boot.img: $(boot)
> > +
> > +
> > +grub2boot.img:
> > +   mkdir -p iso/boot/grub
> > +   cp -f grub.cfg iso/boot/grub
> > +   cp -f kernel.bin iso/
> > +   grub2-mkrescue -o boot.img iso
> >  
> >  clean:
> > -   rm -f *.o *.bin
> > +   rm -rf *.o *.bin iso *.img
> > +
> > +.PHONY: grub2boot.img
> > diff --git a/shared/deps/cpuid_test_kernel/grub.cfg
> > b/shared/deps/cpuid_test_kernel/grub.cfg new file mode 100644
> > index 0000000..aeb5b2b
> > --- /dev/null
> > +++ b/shared/deps/cpuid_test_kernel/grub.cfg
> > @@ -0,0 +1,8 @@
> > +set timeout=0
> > +set default=0
> > +
> > +menuentry "test" {
> > +   set root=(hd0)
> > +   multiboot /kernel.bin
> > +   boot
> > +}
> > -- 
> > 1.7.1
> > 
>