Re: [Qemu-devel] [PATCH v2 0/7] APIC/IOAPIC cleanup

[Avi Kivity]

 On 08/23/2010 05:26 PM, Anthony Liguori wrote:
> On 08/23/2010 09:00 AM, Avi Kivity wrote:
> >  On 08/23/2010 04:48 PM, Anthony Liguori wrote:
> > > > > The fundamental issue is: every function (minus trivial ones) in 
> > > > > the device models code should have a state reference.  That state 
> > > > > reference should inherit from a DeviceState.  If this statement 
> > > > > isn't true, then the device has been modelled in qdev incorrectly.
> > > > >
> > > > > Using this test, quite a lot of the "converted" devices are being 
> > > > > modelled incorrectly.
> > > >
> > > > Is a "state reference" allowed to have a pointer to the state, or 
> > > > reach it in some other way (for example, static storage for 
> > > > singleton devices)?
> > >
> > >
> > > No.  If this was C++, then the statement would be: device have to be 
> > > implemented in terms of objects that inherit from Device.  Device is 
> > > our common base object.
> >
> > so,
> >
> >     struct MyDevicestate {
> >         struct DeviceState device_state;
> >         bool *some_bit;
> >     };
> >
> > bad, while
> >
> >     struct MyDevicestate {
> >         struct DeviceState device_state;
> >         bool some_bit;
> >     };
> >
> > good?
>
> And the next logical question is whether:
>
> struct MyDeviceState {
>    DeviceState qdev;
>    OtherState *s;
> };
>
> Is bad?  The answer would depend on whether OtherState implemented 
> methods or not.  If OtherState has no methods, it's fine.  If it has 
> methods, it's bad.

I don't see why.  As long as you can manipulate all of MyDevice's state 
via MyDeviceState methods, why do you care about OtherState at all?

> > > > Isn't "save/restore works" an equivalent statement to "device state 
> > > > is reachable from the DeviceState"?
> > >
> > > I'm not sure I can connect the dots here as I'm not sure what 
> > > follows if your assertion is true.
> >
> > If save/restore works then all state is reachable from one point?  
> > Presumable DeviceState?
> >
> > I really don't see why the state has to be in the DeviceState 
> > subclass.  We're probably talking past each other here due to some 
> > confusion in terms.
>
> Probably.  Let's say that 'structs' are containers of primatives that 
> have no methods associated with them.  'objects' are structs that have 
> methods and potentially constructors/destructors.
>
> Devices can contain references to structs and objects.  If a Device 
> contains a reference to an object, the object should be stored in a 
> BusState which is a container of Devices.  Therefore, the object 
> should inherit from Device.

I disagree.  It's up to the author to decide whether to split a Device 
into 1 or 15 objects.

If one of the other objects is also a subclass of DeviceState, then 
you're probably violating that DeviceState's contract.  But that's a 
different (and trivial) matter.

(side point: in C no objects have constructors and methods.  in C++ all 
objects have constructors and methods, even PODs)

--
error compiling committee.c: too many arguments to function