Re: [Qemu-devel] [PATCH 06/15] piix: create i8254 through composition

[Anthony Liguori]

On 01/31/2012 10:19 AM, Jan Kiszka wrote:
> On 2012-01-31 17:12, Anthony Liguori wrote:
> > On 01/31/2012 08:58 AM, Paolo Bonzini wrote:
> > > On 01/31/2012 03:51 PM, Jan Kiszka wrote:
> > > > > > BTW, this is yet another benefit of making structures public. You can take the
> > > > > > address of a child and set link fields directly without accessors.
> > > > Well, that has two sides. We introduced properties to avoid this direct
> > > > messing.
> > > >
> > > > Does linking also work without exposing internals?
> > >
> > > Perhaps it doesn't need to expose internals. Just like we create interfaces
> > > automatically when creating a parent object, perhaps we can create children as
> > > well, like
> > >
> > > TypeInfo type_piix3 = {
> > > ...
> > > .children = {
> > > { "pic[0]", TYPE_I8259, offsetof(PIIX3, pic[0]) },
> > > { "pic[1]", TYPE_I8259, offsetof(PIIX3, pic[1]) },
> > > { "pit, TYPE_I8254, offsetof(PIIX3, pit) },
> > > { "rtc", TYPE_RTC, offsetof(PIIX3, rtc) },
> >
> >
> > Eeek.  I absolutely want to avoid any offset based interfaces.
>
> Why?

static void object_initialize(Object *obj)
{
    PIIX3State *s = PIIX3(obj);
    ChildProperty props[] = {
      { "pic[0]", TYPE_I8259, &s->pic[0] },
      { "pic[1]", TYPE_I8259, &s->pic[1] },
    };

    object_property_add_children(obj, props, ARRAY_SIZE(props));
}

Is much nicer IMHO.

> > You can just as well do:
> >
> > void object_property_add_child(Object *obj, const char *name,
> >                                  const char *type, Object **child);
> >
> > It could then do:
> >
> > *child = object_new(type);
>
> How does this resolve where the link is stored in the opaque state
> structure?

See above.  Don't use an offset based interface and it's not a problem.

Regards,

Anthony Liguori

> Jan