[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 26/36] remove preconfig state
From: |
Igor Mammedov |
Subject: |
Re: [PATCH 26/36] remove preconfig state |
Date: |
Fri, 27 Nov 2020 11:50:33 +0100 |
On Fri, 27 Nov 2020 06:19:51 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:
> 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.
it works in context of this series since
+ qemu_init_board();
+ qemu_create_cli_devices();
+ qemu_machine_creation_done();
are called within the same command qmp_x_exit_preconfig,
if preconfig is enabled we happen to call qmp_set_numa_node()
and if (qdev_hotplug) {error} work as expected, since qemu_init_board()
hasn't been called yet.
but I'm thinking about what happens beyond this series, when we start
splitting qmp_x_exit_preconfig() after this series on separate stages.
By using qdev_hotplug here, we practically loose dependency tracking
on qemu_init_board() not being yet called. And if later we forget that,
then it would allow to call qmp_set_numa_node() after qemu_init_board()
but before qemu_machine_creation_done()
So for this intermediate stage, instead of abusing qdev_hotplug adding
a temporary is_board_created might be used. And when we introduce
new phases you've described below, is_board_created could be replaced
with appropriate phase check.
> 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
>
- [PATCH 18/36] vl: separate qemu_create_machine, (continued)
- [PATCH 18/36] vl: separate qemu_create_machine, Paolo Bonzini, 2020/11/23
- [PATCH 25/36] hmp: introduce cmd_available, Paolo Bonzini, 2020/11/23
- [PATCH 16/36] vl: separate qemu_create_early_backends, Paolo Bonzini, 2020/11/23
- [PATCH 23/36] migration, vl: start migration via qmp_migrate_incoming, Paolo Bonzini, 2020/11/23
- [PATCH 20/36] vl: separate qemu_resolve_machine_memdev, Paolo Bonzini, 2020/11/23
- [PATCH 26/36] remove preconfig state, Paolo Bonzini, 2020/11/23
[PATCH 21/36] vl: initialize displays before preconfig loop, Paolo Bonzini, 2020/11/23
[PATCH 28/36] vl: allow -incoming defer with -preconfig, Paolo Bonzini, 2020/11/23
[PATCH 30/36] vl: extract machine done notifiers, Paolo Bonzini, 2020/11/23
[PATCH 31/36] vl: extract softmmu/rtc.c, Paolo Bonzini, 2020/11/23