[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 before machine is ready




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.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
>
> }

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.





 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.