Re: [Qemu-devel] [PATCH] vl.c: remove duplicate call to qemu_get_machine_opts

[Marcel Apfelbaum <marcel@redhat.com>]

On 02/08/2016 09:23 PM, Laszlo Ersek wrote:
> On 02/08/16 19:59, Marcel Apfelbaum wrote:
>> On 02/08/2016 06:47 PM, Laszlo Ersek wrote:
>>> On 02/08/16 13:27, Marcel Apfelbaum wrote:
>>>> On 02/08/2016 02:22 PM, Laszlo Ersek wrote:
>>>>> On 02/08/16 12:45, Marcel Apfelbaum wrote:
>>>>>> Also we can use current machine properties instead of querying
>>>>>> machine's opts.
>>>>>
>>>>> Why? :) For me at least, you'll have to spell it out.
>>>>
>>>> The theory would be that is cleaner to call an object property than use
>>>> a string
>>>> (when possible); compiler helps and so on...
>>>>
>>>>     object->prop vs find_me_prop("prop")
>>>>
>>>> Since all machine properties have already been set with the command line
>>>> options
>>>> we really have no need to use them anymore and we can favor the machine.
>>>
>>> Yes; what I meant is, where exactly have these properties been set?
>>>
>>
>> I didn't get it so I followed a theoretical line, sorry :)
>>
>> A few lines before:
>>
>>      if (qemu_opt_foreach(qemu_get_machine_opts(), machine_set_property,
>>                           current_machine, NULL)) {
>>          object_unref(OBJECT(current_machine));
>>          exit(1);
>>      }
>
> Ah, right, the things I missed in that were:
> - the object_property_parse() call
> - machine_initfn() in "hw/core/machine.c", and the setters it sets up.
>
> FWIW,
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Appreciated!
Note for the maintainer, there is a V2 of this patch on the mailing list:

https://www.mail-archive.com/qemu-devel@nongnu.org/msg351565.html

Thanks,
Marcel


>
> Thanks
> Laszlo
>
>
>>>>>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>>>>>> ---
>>>>>>     vl.c | 9 ++++-----
>>>>>>     1 file changed, 4 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/vl.c b/vl.c
>>>>>> index 5873248..c03a4fe 100644
>>>>>> --- a/vl.c
>>>>>> +++ b/vl.c
>>>>>> @@ -4353,11 +4353,10 @@ int main(int argc, char **argv, char **envp)
>>>>>>             qtest_init(qtest_chrdev, qtest_log, &error_fatal);
>>>>>>         }
>>>>>>
>>>>>> -    machine_opts = qemu_get_machine_opts();
>>>>>> -    kernel_filename = qemu_opt_get(machine_opts, "kernel");
>>>>>> -    initrd_filename = qemu_opt_get(machine_opts, "initrd");
>>>>>> -    kernel_cmdline = qemu_opt_get(machine_opts, "append");
>>>>>> -    bios_name = qemu_opt_get(machine_opts, "firmware");
>>>>>> +    kernel_filename = current_machine->kernel_filename;
>>>>>> +    initrd_filename = current_machine->initrd_filename;
>>>>>> +    kernel_cmdline = current_machine->kernel_cmdline;
>>>>>> +    bios_name = current_machine->firmware;
>>>>>>
>>>>>>         opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
>>>>>>         if (opts) {
>>>>>>
>>>>>
>>>>> Thanks
>>>>> Laszlo
>>>>>
>>>>
>>>
>>
>