[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH v2 10/16] qdev-monitor: allow adding any sysbus device be

From: Damien Hedde
Subject: Re: [RFC PATCH v2 10/16] qdev-monitor: allow adding any sysbus device before machine is ready
Date: Thu, 23 Sep 2021 16:04:44 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0

On 9/23/21 13:55, Ani Sinha wrote:

On Thu, 23 Sep 2021, Ani Sinha wrote:

On Wed, 22 Sep 2021, Damien Hedde wrote:

Skip the sysbus device type per-machine allow-list check before the

This patch permits adding any sysbus device (it still needs to be
user_creatable) when using the -preconfig experimental option.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>

This commit is RFC. Depending on the condition to allow a device
to be added, it may change.
  softmmu/qdev-monitor.c | 9 +++++++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index f1c9242855..73b991adda 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -269,8 +269,13 @@ static DeviceClass *qdev_get_device_class(const char 
**driver, Error **errp)
          return NULL;

-    if (object_class_dynamic_cast(oc, TYPE_SYS_BUS_DEVICE)) {
-        /* sysbus devices need to be allowed by the machine */
+    if (object_class_dynamic_cast(oc, TYPE_SYS_BUS_DEVICE) &&
+        phase_check(MACHINE_INIT_PHASE_READY)) {
+        /*
+         * Sysbus devices need to be allowed by the machine.
+         * We only check that after the machine is ready in order to let
+         * us add any user_creatable sysbus device during machine creation.
+         */
          MachineClass *mc = 
          if (!machine_class_is_dynamic_sysbus_dev_allowed(mc, *driver)) {
              error_setg(errp, "'%s' is not an allowed pluggable sysbus device "

Since now you are adding the state of the machine creation in the
valiation condition, the failure error message becomes misleading.
Better to do this I think :

if (object class is TYPE_SYS_BUS_DEVICE)
   if (!phase_check(MACHINE_INIT_PHASE_READY))
       // error out here saying the state of the machine creation is too

     // do the other check on dynamic sysbus


The other thing to consider is whether we should put the machine phaze
check at a higher level, at qdev_device_add() perhaps. One might envison
that different device types may be allowed to be added at different
stages of machine creation. So this check needs be more generalized for
the future.

Hi Ani,

I think moving out the allowance check from qdev_get_device_class is a good idea. The code will be more clear even if the check is not generalized.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]