Re: [RFC PATCH v2 07/16] hw/core/machine: add machine_class_is_dynamic_sysbus_dev_allowed

[Damien Hedde <damien.hedde@xxxxxxxxxxxxx>]

On 9/23/21 12:51, Ani Sinha wrote:
> On Wed, 22 Sep 2021, Damien Hedde wrote:
>
> > Right now the allowance check for adding a sysbus device using
> > -device cli option (or device_add qmp command) is done well after
> > the device has been created. It is done during the machine init done
> > notifier: machine_init_notify() in hw/core/machine.c
> >
> > This new function will allow us to check if a sysbus device type is
> > allowed to be dynamically created by the machine during the device
> > creation time.
> >
> > Also make device_is_dynamic_sysbus() use the new function.
> >
> > Signed-off-by: Damien Hedde <damien.hedde@xxxxxxxxxxxxx>
> > ---
> >
> > In the context of our series, we need to be able to do the check at
> > device creation time to allow doing it depending on the current
> > MACHINE_INIT phase.
> > ---
> >   include/hw/boards.h | 17 +++++++++++++++++
> >   hw/core/machine.c   | 15 ++++++++++++---
> >   2 files changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 463a5514f9..934443c1cd 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -51,6 +51,23 @@ void machine_set_cpu_numa_node(MachineState *machine,
> >    */
> >   void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char 
> > *type);
> >
> > +/**
> > + * machine_class_is_dynamic_sysbus_dev_allowed: Check if type is an allowed
> > + * sysbus device type for the machine class.
> > + * @mc: Machine class
> > + * @type: type to check (should be a subtype of TYPE_SYS_BUS_DEVICE)
> > + *
> > + * Returns: true if @type is a type in the machine's list of
> > + * dynamically pluggable sysbus devices; otherwise false.
> > + *
> > + * Check if the QOM type @type is in the list of allowed sysbus device
> > + * types (see machine_class_allowed_dynamic_sysbus_dev()).
> > + * Note that if @type is a subtype of a type which is in the list, it is
> > + * allowed too.
> > + */
> > +bool machine_class_is_dynamic_sysbus_dev_allowed(MachineClass *mc,
> > +                                                 const char *type);
> > +
> How about renaming this to device_type_is_allowed_dynamic_sysbus() ?
I followed the "machine_class_allow_dynamic_sysbus_dev" function name style.

But sure I'll rename it. We can even drop the "allowed" if we want
to stay closer to the device_is_dynamic_sysbus legacy function which if 
used to check this on a device instance.
Damien
--
Thanks


> >   /**
> >    * device_is_dynamic_sysbus: test whether device is a dynamic sysbus device
> >    * @mc: Machine class
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 9125c9aad0..1a18912dc8 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -545,18 +545,27 @@ void machine_class_allow_dynamic_sysbus_dev(MachineClass 
> > *mc, const char *type)
> >
> >   bool device_is_dynamic_sysbus(MachineClass *mc, DeviceState *dev)
> >   {
> > -    bool allowed = false;
> > -    strList *wl;
> >       Object *obj = OBJECT(dev);
> >
> >       if (!object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE)) {
> >           return false;
> >       }
> >
> > +    return machine_class_is_dynamic_sysbus_dev_allowed(mc,
> > +            object_get_typename(obj));
> > +}
> > +
> > +bool machine_class_is_dynamic_sysbus_dev_allowed(MachineClass *mc,
> > +                                                 const char *type)
> > +{
> > +    bool allowed = false;
> > +    strList *wl;
> > +    ObjectClass *klass = object_class_by_name(type);
> > +
> >       for (wl = mc->allowed_dynamic_sysbus_devices;
> >            !allowed && wl;
> >            wl = wl->next) {
> > -        allowed |= !!object_dynamic_cast(obj, wl->value);
> > +        allowed |= !!object_class_dynamic_cast(klass, wl->value);
> >       }
> >
> >       return allowed;
> > --
> > 2.33.0