Re: Suspicious QOM types without instance/class size

[Cornelia Huck <cohuck@redhat.com>]

On Fri, 21 Aug 2020 17:01:49 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Fri, Aug 21, 2020 at 11:43:35AM +0200, Cornelia Huck wrote:
> > On Thu, 20 Aug 2020 17:55:29 -0400
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >   
> > > While trying to convert TypeInfo declarations to the new
> > > OBJECT_DECLARE* macros, I've stumbled on a few suspicious cases
> > > where instance_size or class_size is not set, despite having type
> > > checker macros that use a specific type.
> > > 
> > > The ones with "WARNING" are abstract types (maybe not serious if
> > > subclasses set the appropriate sizes).  The ones with "ERROR"
> > > don't seem to be abstract types.
> > > 
> > > ERROR: hw/s390x/virtio-ccw.c:1237:1: class_size should be set to sizeof(VirtioCcwBusClass)?
> > > ERROR: hw/virtio/virtio-pci.c:2101:1: class_size should be set to sizeof(VirtioPCIBusClass)?  
> > 
> > VirtioCcwBusClass and VirtioPCIBusClass are both simple typedefs of
> > VirtioBusClass (it's likely that I copied the ccw definition from the
> > pci one). virtio-mmio instead uses VirtioBusClass directly in its
> > checker macros.
> > 
> > I don't see a real reason for the typedefs, maybe ccw and pci should
> > use the mmio approach as well?  
> 
> I think it's OK to keep the typedefs if the code is consistent
> (i.e. we set instance_size and class_size just in case the
> typedefs are replaced by a real struct one day).

AFAIU, VirtioBusClass is providing functionality needed for all virtio
buses, and they should not need to add anything on top. We can however
try to make it safe, as it is only a line of code for both pci and ccw.

> 
> I'm not sure about the TYPE_VIRTIO_MMIO_BUS approach.  If the
> code just needs VirtioBusState or VirtioBusClass pointers, it can
> already use the VIRTIO_BUS* macros.

We could go ahead and ditch the bus-specific macros if there's no real
need for it. At least, I don't see a real need for *Class. OTOH, having
types and macros defined everywhere makes it more symmetric.

> 
> The OBJECT_DECLARE_TYPE macro Daniel sent expects each QOM type
> to have a separate struct being defined, which isn't true in many
> cases.  I'm considering removing the "typedef struct Foo Foo"
> lines from OBJECT_DECLARE_TYPE(), to make initial conversion
> easier.

Would be interesting to figure out the individual reasons why there's
no separate struct, just to make sure we're not operating from wrong
assumptions.