From: Paolo Bonzini <pbonzini@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [PATCH 27/29] remove preconfig state
Date: Fri, 20 Nov 2020 17:22:53 +0100 [thread overview]
Message-ID: <85d6f1f0-844a-8a70-a8fd-e9a1a7bafa18@redhat.com> (raw)
In-Reply-To: <20201120170158.33413fc9@redhat.com>
On 20/11/20 17:01, Igor Mammedov wrote:
> On Tue, 27 Oct 2020 14:21:42 -0400
> 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>
>
> ERROR:
> tests/qtest/qmp-test.c:298:test_qmp_preconfig: assertion failed (qdict_get_try_str(ret, "status") == "preconfig"): ("prelaunch" == "preconfig")
Uff, of course. So this would be an incompatible change.
Do you think it's important to keep the runstate? Especially when
allowing "-incoming defer" with preconfig things become complicated,
because there's code that checks for RUN_STATE_INMIGRATE and it would
break if the state is preconfig.
Paolo
>> ---
>> 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 +++++++
>> 10 files changed, 33 insertions(+), 28 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) {
>> + error_setg(errp, "The command is permitted only before the machine has been created");
>> return;
>> }
>>
>> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
>> index af8d96c570..1486cac3ef 100644
>> --- a/include/qapi/qmp/dispatch.h
>> +++ b/include/qapi/qmp/dispatch.h
>> @@ -48,6 +48,7 @@ void qmp_disable_command(QmpCommandList *cmds, const char *name);
>> void qmp_enable_command(QmpCommandList *cmds, const char *name);
>>
>> bool qmp_command_is_enabled(const QmpCommand *cmd);
>> +bool qmp_command_available(const QmpCommand *cmd, Error **errp);
>> const char *qmp_command_name(const QmpCommand *cmd);
>> bool qmp_has_success_response(const QmpCommand *cmd);
>> QDict *qmp_error_response(Error *err);
>> diff --git a/monitor/hmp.c b/monitor/hmp.c
>> index f13ef455e2..0027f1465d 100644
>> --- a/monitor/hmp.c
>> +++ b/monitor/hmp.c
>> @@ -24,6 +24,7 @@
>>
>> #include "qemu/osdep.h"
>> #include <dirent.h>
>> +#include "hw/qdev-core.h"
>> #include "monitor-internal.h"
>> #include "qapi/error.h"
>> #include "qapi/qmp/qdict.h"
>> @@ -215,7 +216,7 @@ static bool cmd_can_preconfig(const HMPCommand *cmd)
>>
>> static bool cmd_available(const HMPCommand *cmd)
>> {
>> - return !runstate_check(RUN_STATE_PRECONFIG) || cmd_can_preconfig(cmd);
>> + return qdev_hotplug || cmd_can_preconfig(cmd);
>> }
>>
>> static void help_cmd_dump_one(Monitor *mon,
>> @@ -658,8 +659,8 @@ static const HMPCommand *monitor_parse_command(MonitorHMP *hmp_mon,
>> return NULL;
>> }
>> if (!cmd_available(cmd)) {
>> - monitor_printf(mon, "Command '%.*s' not available with -preconfig "
>> - "until after exit_preconfig.\n",
>> + monitor_printf(mon, "Command '%.*s' not available "
>> + "until machine initialization has completed.\n",
>> (int)(p - cmdp_start), cmdp_start);
>> return NULL;
>> }
>> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
>> index a08143b323..7c10b182e4 100644
>> --- a/monitor/qmp-cmds.c
>> +++ b/monitor/qmp-cmds.c
>> @@ -104,9 +104,8 @@ void qmp_system_powerdown(Error **errp)
>>
>> void qmp_x_exit_preconfig(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) {
>> + error_setg(errp, "The command is permitted only before machine initialization");
>> return;
>> }
>> qemu_exit_preconfig_request();
>> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
>> index 9a2d7dd29a..0a2b20a4e4 100644
>> --- a/qapi/qmp-dispatch.c
>> +++ b/qapi/qmp-dispatch.c
>> @@ -167,10 +167,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request,
>> goto out;
>> }
>>
>> - if (runstate_check(RUN_STATE_PRECONFIG) &&
>> - !(cmd->options & QCO_ALLOW_PRECONFIG)) {
>> - error_setg(&err, "The command '%s' isn't permitted in '%s' state",
>> - cmd->name, RunState_str(RUN_STATE_PRECONFIG));
>> + if (!qmp_command_available(cmd, &err)) {
>> goto out;
>> }
>>
>> diff --git a/qapi/run-state.json b/qapi/run-state.json
>> index 964c8ef391..38194b0e44 100644
>> --- a/qapi/run-state.json
>> +++ b/qapi/run-state.json
>> @@ -50,15 +50,12 @@
>> # @colo: guest is paused to save/restore VM state under colo checkpoint,
>> # VM can not get into this state unless colo capability is enabled
>> # for migration. (since 2.8)
>> -# @preconfig: QEMU is paused before board specific init callback is executed.
>> -# The state is reachable only if the --preconfig CLI option is used.
>> -# (Since 3.0)
>> ##
>> { 'enum': 'RunState',
>> 'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
>> 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
>> 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
>> - 'guest-panicked', 'colo', 'preconfig' ] }
>> + 'guest-panicked', 'colo' ] }
>>
>> ##
>> # @ShutdownCause:
>> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
>> index bcfcbac181..00720eb827 100644
>> --- a/softmmu/qdev-monitor.c
>> +++ b/softmmu/qdev-monitor.c
>> @@ -25,6 +25,7 @@
>> #include "sysemu/arch_init.h"
>> #include "qapi/error.h"
>> #include "qapi/qapi-commands-qdev.h"
>> +#include "qapi/qmp/dispatch.h"
>> #include "qapi/qmp/qdict.h"
>> #include "qapi/qmp/qerror.h"
>> #include "qemu/config-file.h"
>> @@ -997,3 +998,14 @@ int qemu_global_option(const char *str)
>>
>> return 0;
>> }
>> +
>> +bool qmp_command_available(const QmpCommand *cmd, Error **errp)
>> +{
>> + if (!qdev_hotplug &&
>> + !(cmd->options & QCO_ALLOW_PRECONFIG)) {
>> + error_setg(errp, "The command '%s' is permitted only after machine initialization has completed",
>> + cmd->name);
>> + return false;
>> + }
>> + return true;
>> +}
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index edabd17dac..68acd24d01 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -574,7 +574,7 @@ static int default_driver_check(void *opaque, QemuOpts *opts, Error **errp)
>> /***********************************************************/
>> /* QEMU state */
>>
>> -static RunState current_run_state = RUN_STATE_PRECONFIG;
>> +static RunState current_run_state = RUN_STATE_PRELAUNCH;
>>
>> /* We use RUN_STATE__MAX but any invalid value will do */
>> static RunState vmstop_requested = RUN_STATE__MAX;
>> @@ -586,13 +586,7 @@ typedef struct {
>> } RunStateTransition;
>>
>> static const RunStateTransition runstate_transitions_def[] = {
>> - /* from -> to */
>> - { RUN_STATE_PRECONFIG, RUN_STATE_PRELAUNCH },
>> - /* Early switch to inmigrate state to allow -incoming CLI option work
>> - * as it used to. TODO: delay actual switching to inmigrate state to
>> - * the point after machine is built and remove this hack.
>> - */
>> - { RUN_STATE_PRECONFIG, RUN_STATE_INMIGRATE },
>> + { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
>>
>> { RUN_STATE_DEBUG, RUN_STATE_RUNNING },
>> { RUN_STATE_DEBUG, RUN_STATE_FINISH_MIGRATE },
>> @@ -1633,9 +1627,6 @@ static bool main_loop_should_exit(void)
>> ShutdownCause request;
>>
>> if (preconfig_exit_requested) {
>> - if (runstate_check(RUN_STATE_PRECONFIG)) {
>> - runstate_set(RUN_STATE_PRELAUNCH);
>> - }
>> preconfig_exit_requested = false;
>> return true;
>> }
>> diff --git a/stubs/meson.build b/stubs/meson.build
>> index 82b7ba60ab..cc56c83063 100644
>> --- a/stubs/meson.build
>> +++ b/stubs/meson.build
>> @@ -29,6 +29,7 @@ stub_ss.add(files('pci-bus.c'))
>> stub_ss.add(files('pci-host-piix.c'))
>> stub_ss.add(files('qemu-timer-notify-cb.c'))
>> stub_ss.add(files('qmp_memory_device.c'))
>> +stub_ss.add(files('qmp-command-available.c'))
>> stub_ss.add(files('qtest.c'))
>> stub_ss.add(files('ram-block.c'))
>> stub_ss.add(files('ramfb.c'))
>> diff --git a/stubs/qmp-command-available.c b/stubs/qmp-command-available.c
>> new file mode 100644
>> index 0000000000..46540af7bf
>> --- /dev/null
>> +++ b/stubs/qmp-command-available.c
>> @@ -0,0 +1,7 @@
>> +#include "qemu/osdep.h"
>> +#include "qapi/qmp/dispatch.h"
>> +
>> +bool qmp_command_available(const QmpCommand *cmd, Error **errp)
>> +{
>> + return true;
>> +}
>
next prev parent reply other threads:[~2020-11-20 16:33 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-27 18:21 [RFC PATCH v2 00/37] cleanup qemu_init and make sense of command line processing Paolo Bonzini
2020-10-27 18:21 ` [PATCH 01/29] trace: remove argument from trace_init_file Paolo Bonzini
2020-10-27 18:21 ` [PATCH 02/29] semihosting: fix order of initialization functions Paolo Bonzini
2020-10-27 18:21 ` [PATCH 03/29] vl: extract validation of -smp to machine.c Paolo Bonzini
2020-10-28 16:32 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 04/29] vl: remove bogus check Paolo Bonzini
2020-10-28 16:48 ` Igor Mammedov
2020-10-28 16:55 ` Daniel P. Berrangé
2020-10-28 19:32 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 05/29] vl: split various early command line options to a separate function Paolo Bonzini
2020-11-02 15:30 ` Igor Mammedov
2020-11-02 16:33 ` Paolo Bonzini
2020-10-27 18:21 ` [PATCH 06/29] vl: move various initialization routines out of qemu_init Paolo Bonzini
2020-11-02 15:40 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 07/29] vl: extract qemu_init_subsystems Paolo Bonzini
2020-11-02 15:55 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 08/29] vl: move prelaunch part of qemu_init to new functions Paolo Bonzini
2020-11-11 19:29 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 09/29] vl: extract various command line validation snippets to a new function Paolo Bonzini
2020-11-11 19:39 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 10/29] vl: preconfig and loadvm are mutually exclusive Paolo Bonzini
2020-11-11 19:58 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 11/29] vl: extract various command line desugaring snippets to a new function Paolo Bonzini
2020-11-11 19:57 ` Igor Mammedov
2020-11-11 20:04 ` Paolo Bonzini
2020-11-18 16:55 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 12/29] vl: create "-net nic -net user" default earlier Paolo Bonzini
2020-11-18 14:43 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 13/29] vl: load plugins as late as possible Paolo Bonzini
2020-10-27 18:21 ` [PATCH 14/29] vl: move semihosting command line fallback to qemu_finish_machine_init Paolo Bonzini
2020-10-27 18:21 ` [PATCH 15/29] vl: extract default devices to separate functions Paolo Bonzini
2020-10-27 18:21 ` [PATCH 16/29] vl: move CHECKPOINT_INIT after preconfig Paolo Bonzini
2020-10-27 18:21 ` [PATCH 17/29] vl: separate qemu_create_early_backends Paolo Bonzini
2020-11-18 16:29 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 18/29] vl: separate qemu_create_late_backends Paolo Bonzini
2020-11-18 16:33 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 19/29] vl: separate qemu_create_machine Paolo Bonzini
2020-11-18 16:45 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 20/29] vl: separate qemu_apply_machine_options Paolo Bonzini
2020-11-18 16:57 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 21/29] vl: separate qemu_resolve_machine_memdev Paolo Bonzini
2020-11-20 13:15 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 22/29] vl: initialize displays before preconfig loop Paolo Bonzini
2020-11-20 15:11 ` Igor Mammedov
2020-11-20 15:53 ` Paolo Bonzini
2020-11-20 16:32 ` Igor Mammedov
2020-11-20 16:46 ` Paolo Bonzini
2020-11-20 17:11 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 23/29] vl: move -global check earlier Paolo Bonzini
2020-11-20 15:10 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 24/29] migration, vl: start migration via qmp_migrate_incoming Paolo Bonzini
2020-11-20 15:34 ` Igor Mammedov
2020-11-20 16:02 ` Paolo Bonzini
2020-12-02 13:10 ` Dr. David Alan Gilbert
2020-12-02 13:15 ` Daniel P. Berrangé
2020-12-02 13:29 ` Paolo Bonzini
2020-12-02 13:36 ` Paolo Bonzini
2020-12-02 15:08 ` Dr. David Alan Gilbert
2020-10-27 18:21 ` [PATCH 25/29] vl: start VM via qmp_cont Paolo Bonzini
2020-11-20 16:08 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 26/29] hmp: introduce cmd_available Paolo Bonzini
2020-11-20 15:46 ` Igor Mammedov
2020-10-27 18:21 ` [PATCH 27/29] remove preconfig state Paolo Bonzini
2020-11-20 16:01 ` Igor Mammedov
2020-11-20 16:22 ` Paolo Bonzini [this message]
2020-10-27 18:21 ` [PATCH 28/29] vl: remove separate preconfig main_loop Paolo Bonzini
2020-11-20 16:26 ` Igor Mammedov
2020-11-20 16:39 ` Paolo Bonzini
2020-11-23 8:34 ` Paolo Bonzini
2020-10-27 18:21 ` [PATCH 29/29] vl: allow -incoming defer with -preconfig Paolo Bonzini
2020-11-20 16:28 ` Igor Mammedov
2020-11-20 16:45 ` Paolo Bonzini
2020-11-02 15:57 ` [RFC PATCH v2 00/37] cleanup qemu_init and make sense of command line processing Igor Mammedov
2020-11-02 16:34 ` Paolo Bonzini
2020-11-03 12:57 ` Igor Mammedov
2020-11-03 14:37 ` Paolo Bonzini
2020-11-20 16:19 ` Igor Mammedov
2020-11-20 16:27 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=85d6f1f0-844a-8a70-a8fd-e9a1a7bafa18@redhat.com \
--to=pbonzini@redhat.com \
--cc=imammedo@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.