Re: [RFC PATCH v2 10/16] qdev-monitor: allow adding any sysbus device before machine is ready

[Ani Sinha <ani@xxxxxxxxxxx>]

On Wed, 22 Sep 2021, Damien Hedde wrote:

> Skip the sysbus device type per-machine allow-list check before the
> MACHINE_INIT_PHASE_READY phase.
>
> 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@xxxxxxxxxxxxx>
> ---
>
> 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 = 
> MACHINE_CLASS(object_get_class(qdev_get_machine()));
>          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
early
    }

    // do the other check on dynamic sysbus

}