Re: [PATCH 26/36] remove preconfig state

[Paolo Bonzini <pbonzini@redhat.com>]

On 26/11/20 19:55, Igor Mammedov wrote:
> On Mon, 23 Nov 2020 09:14:25 -0500
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> The preconfig state is only used if -incoming is not specified, which
>> makes the RunState state machine more tricky than it need be.  However
>> there is already an equivalent condition which works even with -incoming,
>> namely qdev_hotplug.  Use it instead of a separate runstate.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   hw/core/machine-qmp-cmds.c    |  5 ++---
>>   include/qapi/qmp/dispatch.h   |  1 +
>>   monitor/hmp.c                 |  7 ++++---
>>   monitor/qmp-cmds.c            |  5 ++---
>>   qapi/qmp-dispatch.c           |  5 +----
>>   qapi/run-state.json           |  5 +----
>>   softmmu/qdev-monitor.c        | 12 ++++++++++++
>>   softmmu/vl.c                  | 13 ++-----------
>>   stubs/meson.build             |  1 +
>>   stubs/qmp-command-available.c |  7 +++++++
>>   tests/qtest/qmp-test.c        |  2 +-
>>   11 files changed, 34 insertions(+), 29 deletions(-)
>>   create mode 100644 stubs/qmp-command-available.c
>>
>> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
>> index 5362c80a18..cb9387c5f5 100644
>> --- a/hw/core/machine-qmp-cmds.c
>> +++ b/hw/core/machine-qmp-cmds.c
>> @@ -286,9 +286,8 @@ HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
>>   
>>   void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
>>   {
>> -    if (!runstate_check(RUN_STATE_PRECONFIG)) {
>> -        error_setg(errp, "The command is permitted only in '%s' state",
>> -                   RunState_str(RUN_STATE_PRECONFIG));
>> +    if (qdev_hotplug) {
> 
> that would work only as long as qemu_init_board() hasn't been called,
> and fall apart as soon as we permit creating cold-pluged devices
> (qemu_create_cli_devices()) at preconfig stage.
> 
> for qmp_set_numa_node() the better fit would something like
>    if(is_board_created)
>       error_out
> so it won't break silently when we start extending list of
> commands allowed at preconfig time.
> 
>> +         error_setg(errp, "The command is permitted only before the machine has been created");
>>            return;
>>       }

I don't understand...  qdev_hotplug is a bad name for is_board_created, 
it is set by qdev_machine_creation_done which is called after preconfig 
is left.  As of this patch that happens after the early 
qemu_main_loop(); the next patch moves it to qmp_x_exit_preconfig.

Cold-plugged devices would (by definition) be created while qdev_hotplug 
is false.  But before we get there, I will have replaced the two states 
permitted by qdev_hotplug with five separate phases (PHASE_NO_MACHINE, 
PHASE_MACHINE_CREATED, PHASE_ACCEL_CREATED, PHASE_MACHINE_INITIALIZED, 
PHASE_MACHINE_READY) to clarify the QMP command implementation and to 
assert that various functions are called in the right phase.

Thanks,

Paolo