Re: [PATCH 13/36] vl: move semihosting command line fallback to qemu_init_board

[Igor Mammedov]

On Fri, 27 Nov 2020 06:03:43 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 26/11/20 18:10, Igor Mammedov wrote:
> > On Mon, 23 Nov 2020 09:14:12 -0500
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> >   
> >> Move more sane parts of the huge qemu_init function out of it.
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >>   softmmu/vl.c | 12 +++++-------
> >>   1 file changed, 5 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/softmmu/vl.c b/softmmu/vl.c
> >> index ab08a0290c..5d68cf828c 100644
> >> --- a/softmmu/vl.c
> >> +++ b/softmmu/vl.c
> >> @@ -3070,6 +3070,11 @@ static void qemu_init_board(void)
> >>   {
> >>       MachineClass *machine_class = MACHINE_GET_CLASS(current_machine);
> >>   
> >> +    if (semihosting_enabled() && !semihosting_get_argc() && 
> >> current_machine->kernel_filename) {
> >> +        /* fall back to the -kernel/-append */
> >> +        semihosting_arg_fallback(current_machine->kernel_filename, 
> >> current_machine->kernel_cmdline);
> >> +    }  
> > 
> > it doesn't seem to depend on anything that warrants calling it this late.  
> 
> Yes, calling it around machine initialization time is also a 
> possibility.  I just wanted to get rid of it in code that I'm actually 
> looking at. :)

I'd prefer it being moved close to CLI parsing,
in a place where other _early call go.

We probably want qemu_init_board() being clear of not really needed clutter.

> 
> Paolo
> 
> >   
> >>       if (machine_class->default_ram_id && current_machine->ram_size &&
> >>           numa_uses_legacy_mem() && !current_machine->ram_memdev_id) {
> >>           create_default_memdev(current_machine, mem_path);
> >> @@ -4385,13 +4390,6 @@ void qemu_init(int argc, char **argv, char **envp)
> >>           boot_order = machine_class->default_boot_order;
> >>       }
> >>   
> >> -    if (semihosting_enabled() && !semihosting_get_argc()) {
> >> -        const char *kernel_filename = qemu_opt_get(machine_opts, 
> >> "kernel");
> >> -        const char *kernel_cmdline = qemu_opt_get(machine_opts, "append");
> >> -        /* fall back to the -kernel/-append */
> >> -        semihosting_arg_fallback(kernel_filename, kernel_cmdline);
> >> -    }  
> > 
> > Can we move this hunk as is to somewhere around qemu_maybe_daemonize() time?
> > 
> >   
> >>       if (net_init_clients(&err) < 0) {
> >>           error_report_err(err);
> >>           exit(1);  
> >   
>