* [Qemu-devel] [PATCH] vl.c: remove duplicate call to qemu_get_machine_opts
@ 2016-02-08 11:45 Marcel Apfelbaum
2016-02-08 12:22 ` Laszlo Ersek
0 siblings, 1 reply; 7+ messages in thread
From: Marcel Apfelbaum @ 2016-02-08 11:45 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, pbonzini, lersek
Also we can use current machine properties instead of querying machine's opts.
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) {
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: remove duplicate call to qemu_get_machine_opts
2016-02-08 11:45 [Qemu-devel] [PATCH] vl.c: remove duplicate call to qemu_get_machine_opts Marcel Apfelbaum
@ 2016-02-08 12:22 ` Laszlo Ersek
2016-02-08 12:27 ` Marcel Apfelbaum
0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2016-02-08 12:22 UTC (permalink / raw)
To: Marcel Apfelbaum, qemu-devel; +Cc: pbonzini
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.
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: remove duplicate call to qemu_get_machine_opts
2016-02-08 12:22 ` Laszlo Ersek
@ 2016-02-08 12:27 ` Marcel Apfelbaum
2016-02-08 16:47 ` Laszlo Ersek
0 siblings, 1 reply; 7+ messages in thread
From: Marcel Apfelbaum @ 2016-02-08 12:27 UTC (permalink / raw)
To: Laszlo Ersek, qemu-devel; +Cc: pbonzini
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.
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: remove duplicate call to qemu_get_machine_opts
2016-02-08 12:27 ` Marcel Apfelbaum
@ 2016-02-08 16:47 ` Laszlo Ersek
2016-02-08 18:59 ` Marcel Apfelbaum
0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2016-02-08 16:47 UTC (permalink / raw)
To: Marcel Apfelbaum, qemu-devel; +Cc: pbonzini
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?
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
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: remove duplicate call to qemu_get_machine_opts
2016-02-08 16:47 ` Laszlo Ersek
@ 2016-02-08 18:59 ` Marcel Apfelbaum
2016-02-08 19:23 ` Laszlo Ersek
0 siblings, 1 reply; 7+ messages in thread
From: Marcel Apfelbaum @ 2016-02-08 18:59 UTC (permalink / raw)
To: Laszlo Ersek, qemu-devel; +Cc: pbonzini
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);
}
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
>>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: remove duplicate call to qemu_get_machine_opts
2016-02-08 18:59 ` Marcel Apfelbaum
@ 2016-02-08 19:23 ` Laszlo Ersek
2016-02-08 19:28 ` Marcel Apfelbaum
0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2016-02-08 19:23 UTC (permalink / raw)
To: Marcel Apfelbaum, qemu-devel; +Cc: pbonzini
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>
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
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: remove duplicate call to qemu_get_machine_opts
2016-02-08 19:23 ` Laszlo Ersek
@ 2016-02-08 19:28 ` Marcel Apfelbaum
0 siblings, 0 replies; 7+ messages in thread
From: Marcel Apfelbaum @ 2016-02-08 19:28 UTC (permalink / raw)
To: Laszlo Ersek, qemu-devel; +Cc: pbonzini
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
>>>>>
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-02-08 19:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-08 11:45 [Qemu-devel] [PATCH] vl.c: remove duplicate call to qemu_get_machine_opts Marcel Apfelbaum
2016-02-08 12:22 ` Laszlo Ersek
2016-02-08 12:27 ` Marcel Apfelbaum
2016-02-08 16:47 ` Laszlo Ersek
2016-02-08 18:59 ` Marcel Apfelbaum
2016-02-08 19:23 ` Laszlo Ersek
2016-02-08 19:28 ` Marcel Apfelbaum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).