[RFC PATCH v2 06/16] qapi: Allow device_add to execute in machine initialized phase

Damien Hedde posted 16 patches 3 years, 2 months ago
There is a newer version of this series
[RFC PATCH v2 06/16] qapi: Allow device_add to execute in machine initialized phase
Posted by Damien Hedde 3 years, 2 months ago
From: Mirela Grujic <mirela.grujic@greensocs.com>

To configure a machine using QMP we need the device_add command to
execute at machine initialized phase.

Note: for device_add command in qdev.json adding the 'allow-init-config'
option has no effect because the command appears to bypass QAPI (see
TODO at qapi/qdev.json:61). The option is added there solely to document
the intent.
For the same reason, the flags have to be explicitly set in
monitor_init_qmp_commands() when the device_add command is registered.

Signed-off-by: Mirela Grujic <mirela.grujic@greensocs.com>
---

The commit is fine, but we may add intermediate commits before this one
in order to add or change the condition for a device type to be accepted
in the 'initialized' state (see the cover-letter of the series).
---
 qapi/qdev.json         | 3 ++-
 monitor/misc.c         | 2 +-
 softmmu/qdev-monitor.c | 6 ++++++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/qapi/qdev.json b/qapi/qdev.json
index b83178220b..ad669ae175 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -67,7 +67,8 @@
 ##
 { 'command': 'device_add',
   'data': {'driver': 'str', '*bus': 'str', '*id': 'str'},
-  'gen': false } # so we can get the additional arguments
+  'gen': false, # so we can get the additional arguments
+  'allow-preconfig': true }
 
 ##
 # @device_del:
diff --git a/monitor/misc.c b/monitor/misc.c
index ffe7966870..2c476de316 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -231,7 +231,7 @@ static void monitor_init_qmp_commands(void)
     qmp_init_marshal(&qmp_commands);
 
     qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
-                         QCO_NO_OPTIONS);
+                         QCO_ALLOW_PRECONFIG);
 
     QTAILQ_INIT(&qmp_cap_negotiation_commands);
     qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 834f2b56b5..47ccd90be8 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -824,6 +824,12 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
     QemuOpts *opts;
     DeviceState *dev;
 
+    if (!phase_check(MACHINE_INIT_PHASE_INITIALIZED)) {
+        error_setg(errp, "The command is permitted only after "
+                         "the machine is initialized");
+        return;
+    }
+
     opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, errp);
     if (!opts) {
         return;
-- 
2.33.0


Re: [RFC PATCH v2 06/16] qapi: Allow device_add to execute in machine initialized phase
Posted by Alistair Francis 3 years, 1 month ago
On Thu, Sep 23, 2021 at 2:26 AM Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> From: Mirela Grujic <mirela.grujic@greensocs.com>
>
> To configure a machine using QMP we need the device_add command to
> execute at machine initialized phase.
>
> Note: for device_add command in qdev.json adding the 'allow-init-config'
> option has no effect because the command appears to bypass QAPI (see
> TODO at qapi/qdev.json:61). The option is added there solely to document
> the intent.
> For the same reason, the flags have to be explicitly set in
> monitor_init_qmp_commands() when the device_add command is registered.
>
> Signed-off-by: Mirela Grujic <mirela.grujic@greensocs.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
> The commit is fine, but we may add intermediate commits before this one
> in order to add or change the condition for a device type to be accepted
> in the 'initialized' state (see the cover-letter of the series).
> ---
>  qapi/qdev.json         | 3 ++-
>  monitor/misc.c         | 2 +-
>  softmmu/qdev-monitor.c | 6 ++++++
>  3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/qdev.json b/qapi/qdev.json
> index b83178220b..ad669ae175 100644
> --- a/qapi/qdev.json
> +++ b/qapi/qdev.json
> @@ -67,7 +67,8 @@
>  ##
>  { 'command': 'device_add',
>    'data': {'driver': 'str', '*bus': 'str', '*id': 'str'},
> -  'gen': false } # so we can get the additional arguments
> +  'gen': false, # so we can get the additional arguments
> +  'allow-preconfig': true }
>
>  ##
>  # @device_del:
> diff --git a/monitor/misc.c b/monitor/misc.c
> index ffe7966870..2c476de316 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -231,7 +231,7 @@ static void monitor_init_qmp_commands(void)
>      qmp_init_marshal(&qmp_commands);
>
>      qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
> -                         QCO_NO_OPTIONS);
> +                         QCO_ALLOW_PRECONFIG);
>
>      QTAILQ_INIT(&qmp_cap_negotiation_commands);
>      qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 834f2b56b5..47ccd90be8 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -824,6 +824,12 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
>      QemuOpts *opts;
>      DeviceState *dev;
>
> +    if (!phase_check(MACHINE_INIT_PHASE_INITIALIZED)) {
> +        error_setg(errp, "The command is permitted only after "
> +                         "the machine is initialized");
> +        return;
> +    }
> +
>      opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, errp);
>      if (!opts) {
>          return;
> --
> 2.33.0
>
>