Re: [Qemu-devel] [PATCH 10/10] machine: Skip global registration for non-existing classes

[Eduardo Habkost]

On Sun, Jun 19, 2016 at 07:50:56PM +0300, Marcel Apfelbaum wrote:
> On 06/19/2016 07:39 PM, Marcel Apfelbaum wrote:
> > On 06/15/2016 11:32 PM, Eduardo Habkost wrote:
> > > MachineClass::compat_props may point to class names that are not
> > > compiled into the QEMU binary. Skip registering those as global
> > > properties. This will allow the qdev global property code to
> > > implement stricter checks on the global property values in the
> > > future.
> > > 
> > > Signed-off-by: Eduardo Habkost <address@hidden>
> > > ---
> > >   hw/core/machine.c | 9 +++++++++
> > >   1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > index d6f6be4..51697cb 100644
> > > --- a/hw/core/machine.c
> > > +++ b/hw/core/machine.c
> > > @@ -583,6 +583,7 @@ static void machine_class_finalize(ObjectClass 
> > > *klass, void *data)
> > >   void machine_register_compat_props(MachineState *machine)
> > >   {
> > >       MachineClass *mc = MACHINE_GET_CLASS(machine);
> > > +    ObjectClass *oc;
> > >       int i;
> > >       GlobalProperty *p;
> > > 
> > > @@ -592,6 +593,14 @@ void machine_register_compat_props(MachineState 
> > > *machine)
> > > 
> > >       for (i = 0; i < mc->compat_props->len; i++) {
> > >           p = g_array_index(mc->compat_props, GlobalProperty *, i);
> > 
> > Hi Eduardo,
> > 
> > > +
> > > +        /* Skip registering globals for non-existing device classes */
> > > +        oc = object_class_by_name(p->driver);
> > 
> > This I understand. It ensure a corresponding class is in QEMU binary.
> > But even so, if such property is not available should we silently continue?
> > Maybe the user things the property is set...
> 
> Please forgive the spelling errors and disregard the above question.
> 
> > 
> > 
> > > +        oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
> > 
> > This I don't understand. Do we check here?
> > 
> 
> Regarding my last question, it seems you followed an existing pattern
> here, however I don't quite understand, we found the class,
> why do we need to ensure is a TYPE_DEVICE?
> 
> What error case do we want to prevent?

Globals are a TYPE_DEVICE feature (implemented in
device_post_init()). Registering globals for non-TYPE_DEVICE
classes don't work.

-- 
Eduardo