Re: [Qemu-devel] [PATCH v2] vl.c: fix regression when reading machine type from config file

[Marcel Apfelbaum <marcel@redhat.com>]

On 01/07/2015 01:35 PM, Paolo Bonzini wrote:
>
>
> On 07/01/2015 12:34, Marcel Apfelbaum wrote:
>> After 'Machine as QOM' series the machine type input triggers
>> the creation of the machine class.
>> If the machine type is set in the configuration file, the machine
>> class is not updated accordingly and remains the default.
>>
>> Fixed that by querying the machine options after the configuration
>> file is loaded.
>>
>> Cc: qemu-stable@nongnu.org
>> Reported-by: William Dauchy <william@gandi.net>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>> v1 -> v2:
>>   - Addressed Paolo Bonzini's comments:
>>     - moved the call to machine_parse after the switch
>>       and brought QEMU_OPTION_M near QEMU_OPTION_machine
>>
>>   vl.c | 17 +++++++++--------
>>   1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 7786b2f..da69dba 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2796,9 +2796,6 @@ int main(int argc, char **argv, char **envp)
>>                   exit(1);
>>               }
>>               switch(popt->index) {
>> -            case QEMU_OPTION_M:
>> -                machine_class = machine_parse(optarg);
>> -                break;
>>               case QEMU_OPTION_no_kvm_irqchip: {
>>                   olist = qemu_find_opts("machine");
>>                   qemu_opts_parse(olist, "kernel_irqchip=off", 0);
>> @@ -3420,17 +3417,14 @@ int main(int argc, char **argv, char **envp)
>>                   olist = qemu_find_opts("machine");
>>                   qemu_opts_parse(olist, "accel=kvm", 0);
>>                   break;
>> +            case QEMU_OPTION_M:
>>               case QEMU_OPTION_machine:
>>                   olist = qemu_find_opts("machine");
>>                   opts = qemu_opts_parse(olist, optarg, 1);
>>                   if (!opts) {
>>                       exit(1);
>>                   }
>> -                optarg = qemu_opt_get(opts, "type");
>> -                if (optarg) {
>> -                    machine_class = machine_parse(optarg);
>> -                }
>> -                break;
>> +               break;
>
> Unnecessary whitespace change? :(
Ooops! Sent v3, thanks for catching it!
Marcel

>
>>                case QEMU_OPTION_no_kvm:
>>                   olist = qemu_find_opts("machine");
>>                   qemu_opts_parse(olist, "accel=tcg", 0);
>> @@ -3752,6 +3746,13 @@ int main(int argc, char **argv, char **envp)
>>               }
>>           }
>>       }
>> +
>> +    opts = qemu_get_machine_opts();
>> +    optarg = qemu_opt_get(opts, "type");
>> +    if (optarg) {
>> +        machine_class = machine_parse(optarg);
>> +    }
>> +
>>       loc_set_none();
>>
>>       os_daemonize();
>>
>
> Looks good apart from that.
>
> Paolo
>