Re: [PATCH] ide: Get rid of IDEDrive struct

[Eduardo Habkost <ehabkost@redhat.com>]

On Wed, Aug 05, 2020 at 09:41:25PM +0100, Peter Maydell wrote:
> On Wed, 5 Aug 2020 at 20:49, Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > The struct had a single field (IDEDevice dev), and is only used
> > in the QOM type declarations and property lists.  We can simply
> > use the IDEDevice struct directly instead.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > @@ -327,7 +323,6 @@ static void ide_hd_class_init(ObjectClass *klass, void *data)
> >  static const TypeInfo ide_hd_info = {
> >      .name          = "ide-hd",
> >      .parent        = TYPE_IDE_DEVICE,
> > -    .instance_size = sizeof(IDEDrive),
> >      .class_init    = ide_hd_class_init,
> >  };
> 
> This is one of those areas where this change works and reduces
> amount of code, but on the other hand it means the QOM type
> doesn't follow the common pattern for a leaf type of:
>  * it has a struct
>  * it has cast macros that cast to that struct
>  * the typeinfo instance_size is the size of that struct
> (it wasn't exactly following this pattern before, of course).

Is this really a pattern that exists and we want to follow?
I don't see why that pattern would be useful for simple leaf
types.

Also, in this case the code wasn't even following that pattern:
it was using the same IDEDrive struct for all TYPE_IDE_DEVICE
subtypes.

> 
> We define in https://wiki.qemu.org/Documentation/QOMConventions
> (in the 'When to create class types and macros' bit at the bottom)
> what we expect for whether to provide class cast macros/a
> class struct/class_size in the TypeInfo, essentially recommending
> that types follow one of two patterns (simple leaf class with no
> methods or class members, vs everything else) even if in a
> particular case you could take a short-cut and not define
> everything. We haven't really defined similar "this is the
> standard pattern, provide it all even if you don't strictly
> need it" rules for the instance struct/macros. Maybe we should?

I think we should include the instance struct/macros in the
recommendations there, but I would expect those recommendations
to apply only to non-leaf types.

> 
> Just a thought, not a nak; I know we have quite a number
> of types that take this kind of "we don't really need to
> provide all the standard QOM macros/structs/etc" approach
> (some of which I wrote!).
> 
> thanks
> -- PMM
> 

-- 
Eduardo