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
> > 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.he...@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 =
> > 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
>
> }
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.
