qemu-system-FOO's main() acts on command line arguments in its own
idiosyncratic order. There's not much method to its madness.
Whenever we find a case where one kind of command line argument needs
to refer to something created for another kind later, we rejigger the
order.
Block devices get created long after machine properties get processed.
Therefore, block device machine properties can be created, but not
set. No such properties exist. But the next commit will create some.
Time to rejigger again: create block devices earlier.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
vl.c | 66 ++++++++++++++++++++++++++++++------------------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/vl.c b/vl.c
index 6ce3d2d448..5cb0810ffa 100644
--- a/vl.c
+++ b/vl.c
@@ -4232,6 +4232,39 @@ int main(int argc, char **argv, char **envp)
exit(0);
}
+ /* If the currently selected machine wishes to override the units-per-bus
+ * property of its default HBA interface type, do so now. */
+ if (machine_class->units_per_default_bus) {
+ override_max_devs(machine_class->block_default_type,
+ machine_class->units_per_default_bus);
+ }
+
+ /* open the virtual block devices */
+ while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
+ BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(&bdo_queue);
+
+ QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
+ loc_push_restore(&bdo->loc);
+ qmp_blockdev_add(bdo->bdo, &error_fatal);
+ loc_pop(&bdo->loc);
+ qapi_free_BlockdevOptions(bdo->bdo);
+ g_free(bdo);
+ }
+ if (snapshot || replay_mode != REPLAY_MODE_NONE) {
+ qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
+ NULL, NULL);
+ }
+ if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
+ &machine_class->block_default_type, &error_fatal)) {
+ /* We printed help */
+ exit(0);
+ }
+
+ default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
+ CDROM_OPTS);
+ default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
+ default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
+
machine_opts = qemu_get_machine_opts();
qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
&error_fatal);
@@ -4355,39 +4388,6 @@ int main(int argc, char **argv, char **envp)
ram_mig_init();
dirty_bitmap_mig_init();
- /* If the currently selected machine wishes to override the units-per-bus
- * property of its default HBA interface type, do so now. */
- if (machine_class->units_per_default_bus) {
- override_max_devs(machine_class->block_default_type,
- machine_class->units_per_default_bus);
- }
-
- /* open the virtual block devices */
- while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
- BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(&bdo_queue);
-
- QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
- loc_push_restore(&bdo->loc);
- qmp_blockdev_add(bdo->bdo, &error_fatal);
- loc_pop(&bdo->loc);
- qapi_free_BlockdevOptions(bdo->bdo);
- g_free(bdo);
- }
- if (snapshot || replay_mode != REPLAY_MODE_NONE) {
- qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
- NULL, NULL);
- }
- if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
- &machine_class->block_default_type, &error_fatal)) {
- /* We printed help */
- exit(0);
- }
-
- default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
- CDROM_OPTS);
- default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
- default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
-
qemu_opts_foreach(qemu_find_opts("mon"),
mon_init_func, NULL, &error_fatal);
--
2.17.2
On 2/25/19 7:37 PM, Markus Armbruster wrote:
> qemu-system-FOO's main() acts on command line arguments in its own
> idiosyncratic order. There's not much method to its madness.
> Whenever we find a case where one kind of command line argument needs
> to refer to something created for another kind later, we rejigger the
> order.
>
> Block devices get created long after machine properties get processed.
> Therefore, block device machine properties can be created, but not
> set. No such properties exist. But the next commit will create some.
> Time to rejigger again: create block devices earlier.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> vl.c | 66 ++++++++++++++++++++++++++++++------------------------------
> 1 file changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 6ce3d2d448..5cb0810ffa 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4232,6 +4232,39 @@ int main(int argc, char **argv, char **envp)
> exit(0);
> }
>
Can we extract this from the main, maybe as "parse_blockdev()"? This
will ease further rejigger :)
> + /* If the currently selected machine wishes to override the units-per-bus
> + * property of its default HBA interface type, do so now. */
> + if (machine_class->units_per_default_bus) {
> + override_max_devs(machine_class->block_default_type,
> + machine_class->units_per_default_bus);
> + }
> +
> + /* open the virtual block devices */
> + while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
> + BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(&bdo_queue);
> +
> + QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
> + loc_push_restore(&bdo->loc);
> + qmp_blockdev_add(bdo->bdo, &error_fatal);
> + loc_pop(&bdo->loc);
> + qapi_free_BlockdevOptions(bdo->bdo);
> + g_free(bdo);
> + }
> + if (snapshot || replay_mode != REPLAY_MODE_NONE) {
> + qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
> + NULL, NULL);
> + }
> + if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
> + &machine_class->block_default_type, &error_fatal)) {
> + /* We printed help */
> + exit(0);
> + }
> +
> + default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
> + CDROM_OPTS);
> + default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
> + default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
> +
Can you add a comment here such:
/*
* Arguments setting properties on devices has to be processed before
* the following qemu_opt_foreach(...machine_set_property...) call.
*/
> machine_opts = qemu_get_machine_opts();
> qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
> &error_fatal);
> @@ -4355,39 +4388,6 @@ int main(int argc, char **argv, char **envp)
> ram_mig_init();
> dirty_bitmap_mig_init();
>
> - /* If the currently selected machine wishes to override the units-per-bus
> - * property of its default HBA interface type, do so now. */
> - if (machine_class->units_per_default_bus) {
> - override_max_devs(machine_class->block_default_type,
> - machine_class->units_per_default_bus);
> - }
> -
> - /* open the virtual block devices */
> - while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
> - BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(&bdo_queue);
> -
> - QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
> - loc_push_restore(&bdo->loc);
> - qmp_blockdev_add(bdo->bdo, &error_fatal);
> - loc_pop(&bdo->loc);
> - qapi_free_BlockdevOptions(bdo->bdo);
> - g_free(bdo);
> - }
> - if (snapshot || replay_mode != REPLAY_MODE_NONE) {
> - qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
> - NULL, NULL);
> - }
> - if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
> - &machine_class->block_default_type, &error_fatal)) {
> - /* We printed help */
> - exit(0);
> - }
> -
> - default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
> - CDROM_OPTS);
> - default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
> - default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
> -
> qemu_opts_foreach(qemu_find_opts("mon"),
> mon_init_func, NULL, &error_fatal);
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 2/25/19 7:37 PM, Markus Armbruster wrote:
>> qemu-system-FOO's main() acts on command line arguments in its own
>> idiosyncratic order. There's not much method to its madness.
>> Whenever we find a case where one kind of command line argument needs
>> to refer to something created for another kind later, we rejigger the
>> order.
>>
>> Block devices get created long after machine properties get processed.
>> Therefore, block device machine properties can be created, but not
>> set. No such properties exist. But the next commit will create some.
>> Time to rejigger again: create block devices earlier.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> vl.c | 66 ++++++++++++++++++++++++++++++------------------------------
>> 1 file changed, 33 insertions(+), 33 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 6ce3d2d448..5cb0810ffa 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -4232,6 +4232,39 @@ int main(int argc, char **argv, char **envp)
>> exit(0);
>> }
>>
>
> Can we extract this from the main, maybe as "parse_blockdev()"? This
> will ease further rejigger :)
I can try and see whether I like it.
>> + /* If the currently selected machine wishes to override the units-per-bus
>> + * property of its default HBA interface type, do so now. */
>> + if (machine_class->units_per_default_bus) {
>> + override_max_devs(machine_class->block_default_type,
>> + machine_class->units_per_default_bus);
>> + }
>> +
>> + /* open the virtual block devices */
>> + while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
>> + BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(&bdo_queue);
>> +
>> + QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
>> + loc_push_restore(&bdo->loc);
>> + qmp_blockdev_add(bdo->bdo, &error_fatal);
>> + loc_pop(&bdo->loc);
>> + qapi_free_BlockdevOptions(bdo->bdo);
>> + g_free(bdo);
>> + }
>> + if (snapshot || replay_mode != REPLAY_MODE_NONE) {
>> + qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
>> + NULL, NULL);
>> + }
>> + if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
>> + &machine_class->block_default_type, &error_fatal)) {
>> + /* We printed help */
>> + exit(0);
>> + }
>> +
>> + default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
>> + CDROM_OPTS);
>> + default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
>> + default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
>> +
>
> Can you add a comment here such:
>
> /*
> * Arguments setting properties on devices has to be processed before
> * the following qemu_opt_foreach(...machine_set_property...) call.
> */
Hmm, is this accurate? The issue this patch addresses is
"arguments creating block backends have to be processed before
machine_set_property()",
which is an instance of
"arguments creating backends ...",
which is an instance of
"arguments creating stuff machine property values can reference ..."
The patch only addresses the instance "block backends". I have no
appetite for attacking more general problems right now.
See also
Subject: Re: [Qemu-devel] [PATCH RFC 0/3] qdev: order devices by priority before creating them
Date: Wed, 11 May 2016 09:51:39 +0200
Message-ID: <87y47hhw1g.fsf@dusky.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg01589.html
Subject: Re: [Qemu-devel] [PATCH] vl: Delay initialization of memory backends
Date: Fri, 02 Sep 2016 08:13:17 +0200
Message-ID: <87poomg77m.fsf@dusky.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg00163.html
That said, I'm fine with adding a comment explaining the more general
mess. Can you give me some hints on what future readers will find
useful?
>> machine_opts = qemu_get_machine_opts();
>> qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
>> &error_fatal);
>> @@ -4355,39 +4388,6 @@ int main(int argc, char **argv, char **envp)
>> ram_mig_init();
>> dirty_bitmap_mig_init();
>>
>> - /* If the currently selected machine wishes to override the units-per-bus
>> - * property of its default HBA interface type, do so now. */
>> - if (machine_class->units_per_default_bus) {
>> - override_max_devs(machine_class->block_default_type,
>> - machine_class->units_per_default_bus);
>> - }
>> -
>> - /* open the virtual block devices */
>> - while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
>> - BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(&bdo_queue);
>> -
>> - QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
>> - loc_push_restore(&bdo->loc);
>> - qmp_blockdev_add(bdo->bdo, &error_fatal);
>> - loc_pop(&bdo->loc);
>> - qapi_free_BlockdevOptions(bdo->bdo);
>> - g_free(bdo);
>> - }
>> - if (snapshot || replay_mode != REPLAY_MODE_NONE) {
>> - qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
>> - NULL, NULL);
>> - }
>> - if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
>> - &machine_class->block_default_type, &error_fatal)) {
>> - /* We printed help */
>> - exit(0);
>> - }
>> -
>> - default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
>> - CDROM_OPTS);
>> - default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
>> - default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
>> -
>> qemu_opts_foreach(qemu_find_opts("mon"),
>> mon_init_func, NULL, &error_fatal);
>>
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Thanks!
On 3/5/19 11:38 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> On 2/25/19 7:37 PM, Markus Armbruster wrote:
>>> qemu-system-FOO's main() acts on command line arguments in its own
>>> idiosyncratic order. There's not much method to its madness.
>>> Whenever we find a case where one kind of command line argument needs
>>> to refer to something created for another kind later, we rejigger the
>>> order.
>>>
>>> Block devices get created long after machine properties get processed.
>>> Therefore, block device machine properties can be created, but not
>>> set. No such properties exist. But the next commit will create some.
>>> Time to rejigger again: create block devices earlier.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>> vl.c | 66 ++++++++++++++++++++++++++++++------------------------------
>>> 1 file changed, 33 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index 6ce3d2d448..5cb0810ffa 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -4232,6 +4232,39 @@ int main(int argc, char **argv, char **envp)
>>> exit(0);
>>> }
>>>
>>
>> Can we extract this from the main, maybe as "parse_blockdev()"? This
>> will ease further :)
>
> I can try and see whether I like it.
>
>>> + /* If the currently selected machine wishes to override the units-per-bus
>>> + * property of its default HBA interface type, do so now. */
>>> + if (machine_class->units_per_default_bus) {
>>> + override_max_devs(machine_class->block_default_type,
>>> + machine_class->units_per_default_bus);
>>> + }
>>> +
>>> + /* open the virtual block devices */
>>> + while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
>>> + BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(&bdo_queue);
>>> +
>>> + QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
>>> + loc_push_restore(&bdo->loc);
>>> + qmp_blockdev_add(bdo->bdo, &error_fatal);
>>> + loc_pop(&bdo->loc);
>>> + qapi_free_BlockdevOptions(bdo->bdo);
>>> + g_free(bdo);
>>> + }
>>> + if (snapshot || replay_mode != REPLAY_MODE_NONE) {
>>> + qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
>>> + NULL, NULL);
>>> + }
>>> + if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
>>> + &machine_class->block_default_type, &error_fatal)) {
>>> + /* We printed help */
>>> + exit(0);
>>> + }
>>> +
>>> + default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
>>> + CDROM_OPTS);
>>> + default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
>>> + default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
>>> +
>>
>> Can you add a comment here such:
>>
>> /*
>> * Arguments setting properties on devices has to be processed before
>> * the following qemu_opt_foreach(...machine_set_property...) call.
>> */
>
> Hmm, is this accurate? The issue this patch addresses is
>
> "arguments creating block backends have to be processed before
> machine_set_property()",
>
> which is an instance of
>
> "arguments creating backends ...",
>
> which is an instance of
>
> "arguments creating stuff machine property values can reference ..."
>
> The patch only addresses the instance "block backends". I have no
> appetite for attacking more general problems right now.
>
> See also
>
> Subject: Re: [Qemu-devel] [PATCH RFC 0/3] qdev: order devices by priority before creating them
> Date: Wed, 11 May 2016 09:51:39 +0200
> Message-ID: <87y47hhw1g.fsf@dusky.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg01589.html
>
> Subject: Re: [Qemu-devel] [PATCH] vl: Delay initialization of memory backends
> Date: Fri, 02 Sep 2016 08:13:17 +0200
> Message-ID: <87poomg77m.fsf@dusky.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg00163.html
>
> That said, I'm fine with adding a comment explaining the more general
> mess. Can you give me some hints on what future readers will find
> useful?
This can be as simple as:
/*
* Arguments creating block backends have to be processed before
* machine_set_property().
*/
parse_blockdev(...);
So if I have to hack around in the future I'd look at the commit
introducing this comment:
Block devices get created long after machine properties get processed.
Therefore, block device machine properties can be created, but not
set. No such properties exist. But the next commit will create some.
So I won't rejig this at his previous place :)
>
>>> machine_opts = qemu_get_machine_opts();
>>> qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
>>> &error_fatal);
>>> @@ -4355,39 +4388,6 @@ int main(int argc, char **argv, char **envp)
>>> ram_mig_init();
>>> dirty_bitmap_mig_init();
>>>
>>> - /* If the currently selected machine wishes to override the units-per-bus
>>> - * property of its default HBA interface type, do so now. */
>>> - if (machine_class->units_per_default_bus) {
>>> - override_max_devs(machine_class->block_default_type,
>>> - machine_class->units_per_default_bus);
>>> - }
>>> -
>>> - /* open the virtual block devices */
>>> - while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
>>> - BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(&bdo_queue);
>>> -
>>> - QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
>>> - loc_push_restore(&bdo->loc);
>>> - qmp_blockdev_add(bdo->bdo, &error_fatal);
>>> - loc_pop(&bdo->loc);
>>> - qapi_free_BlockdevOptions(bdo->bdo);
>>> - g_free(bdo);
>>> - }
>>> - if (snapshot || replay_mode != REPLAY_MODE_NONE) {
>>> - qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
>>> - NULL, NULL);
>>> - }
>>> - if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
>>> - &machine_class->block_default_type, &error_fatal)) {
>>> - /* We printed help */
>>> - exit(0);
>>> - }
>>> -
>>> - default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
>>> - CDROM_OPTS);
>>> - default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
>>> - default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
>>> -
>>> qemu_opts_foreach(qemu_find_opts("mon"),
>>> mon_init_func, NULL, &error_fatal);
>>>
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Thanks!
>
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 3/5/19 11:38 AM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>> On 2/25/19 7:37 PM, Markus Armbruster wrote:
>>>> qemu-system-FOO's main() acts on command line arguments in its own
>>>> idiosyncratic order. There's not much method to its madness.
>>>> Whenever we find a case where one kind of command line argument needs
>>>> to refer to something created for another kind later, we rejigger the
>>>> order.
>>>>
>>>> Block devices get created long after machine properties get processed.
>>>> Therefore, block device machine properties can be created, but not
>>>> set. No such properties exist. But the next commit will create some.
>>>> Time to rejigger again: create block devices earlier.
>>>>
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>> ---
>>>> vl.c | 66 ++++++++++++++++++++++++++++++------------------------------
>>>> 1 file changed, 33 insertions(+), 33 deletions(-)
>>>>
>>>> diff --git a/vl.c b/vl.c
>>>> index 6ce3d2d448..5cb0810ffa 100644
>>>> --- a/vl.c
>>>> +++ b/vl.c
>>>> @@ -4232,6 +4232,39 @@ int main(int argc, char **argv, char **envp)
>>>> exit(0);
>>>> }
>>>>
>>>
>>> Can we extract this from the main, maybe as "parse_blockdev()"? This
>>> will ease further :)
>>
>> I can try and see whether I like it.
>>
>>>> + /* If the currently selected machine wishes to override the units-per-bus
>>>> + * property of its default HBA interface type, do so now. */
>>>> + if (machine_class->units_per_default_bus) {
>>>> + override_max_devs(machine_class->block_default_type,
>>>> + machine_class->units_per_default_bus);
>>>> + }
>>>> +
>>>> + /* open the virtual block devices */
>>>> + while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
>>>> + BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(&bdo_queue);
>>>> +
>>>> + QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
>>>> + loc_push_restore(&bdo->loc);
>>>> + qmp_blockdev_add(bdo->bdo, &error_fatal);
>>>> + loc_pop(&bdo->loc);
>>>> + qapi_free_BlockdevOptions(bdo->bdo);
>>>> + g_free(bdo);
>>>> + }
>>>> + if (snapshot || replay_mode != REPLAY_MODE_NONE) {
>>>> + qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
>>>> + NULL, NULL);
>>>> + }
>>>> + if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
>>>> + &machine_class->block_default_type, &error_fatal)) {
>>>> + /* We printed help */
>>>> + exit(0);
>>>> + }
>>>> +
>>>> + default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
>>>> + CDROM_OPTS);
>>>> + default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
>>>> + default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
>>>> +
>>>
>>> Can you add a comment here such:
>>>
>>> /*
>>> * Arguments setting properties on devices has to be processed before
>>> * the following qemu_opt_foreach(...machine_set_property...) call.
>>> */
>>
>> Hmm, is this accurate? The issue this patch addresses is
>>
>> "arguments creating block backends have to be processed before
>> machine_set_property()",
>>
>> which is an instance of
>>
>> "arguments creating backends ...",
>>
>> which is an instance of
>>
>> "arguments creating stuff machine property values can reference ..."
>>
>> The patch only addresses the instance "block backends". I have no
>> appetite for attacking more general problems right now.
>>
>> See also
>>
>> Subject: Re: [Qemu-devel] [PATCH RFC 0/3] qdev: order devices by priority before creating them
>> Date: Wed, 11 May 2016 09:51:39 +0200
>> Message-ID: <87y47hhw1g.fsf@dusky.pond.sub.org>
>> https://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg01589.html
>>
>> Subject: Re: [Qemu-devel] [PATCH] vl: Delay initialization of memory backends
>> Date: Fri, 02 Sep 2016 08:13:17 +0200
>> Message-ID: <87poomg77m.fsf@dusky.pond.sub.org>
>> https://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg00163.html
>>
>> That said, I'm fine with adding a comment explaining the more general
>> mess. Can you give me some hints on what future readers will find
>> useful?
>
> This can be as simple as:
>
> /*
> * Arguments creating block backends have to be processed before
> * machine_set_property().
> */
> parse_blockdev(...);
>
> So if I have to hack around in the future I'd look at the commit
> introducing this comment:
>
> Block devices get created long after machine properties get processed.
> Therefore, block device machine properties can be created, but not
> set. No such properties exist. But the next commit will create some.
>
> So I won't rejig this at his previous place :)
Makes sense, thanks.
[...]
© 2016 - 2025 Red Hat, Inc.