Philippe Mathieu-Daudé <phi...@redhat.com> writes:
> On 3/5/19 11:38 AM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <phi...@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 <arm...@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....@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....@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.
[...]
