Re: [Qemu-devel] [PATCH 5/4] vl: Clean up machine selection in main().

[Markus Armbruster]

Laszlo Ersek <address@hidden> writes:

> On 02/16/16 15:57, Markus Armbruster wrote:
>> We set machine_class to the default first, and update it to the real
>> one later.  Any use of machine_class in between is almost certainly
>> wrong.  Set it once and for all instead.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  vl.c | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>> 
>> diff --git a/vl.c b/vl.c
>> index 7918e9f..098728c 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2748,8 +2748,9 @@ static const QEMUOption *lookup_opt(int argc, char 
>> **argv,
>>      return popt;
>>  }
>>  
>> -static void set_machine_options(MachineClass **machine_class)
>> +static MachineClass *select_machine(MachineClass *dflt)
>>  {
>> +    MachineClass *machine_class = dflt;
>>      const char *optarg;
>>      QemuOpts *opts;
>>      Location loc;
>> @@ -2761,16 +2762,17 @@ static void set_machine_options(MachineClass 
>> **machine_class)
>>  
>>      optarg = qemu_opt_get(opts, "type");
>>      if (optarg) {
>> -        *machine_class = machine_parse(optarg);
>> +        machine_class = machine_parse(optarg);
>>      }
>>  
>> -    if (*machine_class == NULL) {
>> +    if (!machine_class) {
>>          error_report("No machine specified, and there is no default");
>>          error_printf("Use -machine help to list supported machines\n");
>>          exit(1);
>>      }
>>  
>>      loc_pop(&loc);
>> +    return machine_class;
>>  }
>>  
>>  static int machine_set_property(void *opaque,
>> @@ -3075,7 +3077,6 @@ int main(int argc, char **argv, char **envp)
>>      os_setup_early_signal_handling();
>>  
>>      module_call_init(MODULE_INIT_MACHINE);
>> -    machine_class = find_default_machine();
>>      cpu_model = NULL;
>>      snapshot = 0;
>>      cyls = heads = secs = 0;
>> @@ -4066,7 +4067,7 @@ int main(int argc, char **argv, char **envp)
>>  
>>      replay_configure(icount_opts);
>>  
>> -    set_machine_options(&machine_class);
>> +    machine_class = select_machine(find_default_machine());
>>  
>>      set_memory_options(&ram_slots, &maxram_size, machine_class);
>>  
>> 
>
> Sorry for not being more responsive in this thread. I read through the
> patches now (including this one), and they look good to me.
>
> I have one suggestion for the commit message of this patch, after
> checking "vl.c" (and keeping the earlier patches of the series in mind):
> after the statement
>
>   Any use of machine_class in between is almost certainly wrong
>
> can you please observe
>
>   (there are no such uses right now)
>
> ?

Done.

> series
> Reviewed-by: Laszlo Ersek <address@hidden>

Thanks!