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

[Anthony Liguori]

On 08/23/2010 08:42 AM, Avi Kivity wrote:
> > > GPIO is just one way for a device to talk, same as 
> > > (*bus)_phys_memory_rw() or its netdev or its chardev or its timers.  
> > > It doesn't need to have special status within DeviceState, but it 
> > > doesn't hurt so much that I can tell.
> >
> > Everything extra hurts when you're trying to move code in to a 
> > library with unit tests covering the functionality :-)
>
> Sure, it's a worthy cleanup.  But it's not a reason to go to DEFCON 1.

I think you're reading too much into my late night rantings ;-)

> > > > But being able to associate timers with devices seems like a very 
> > > > good idea to me because it means that you can see which devices are 
> > > > registering timers.
> > >
> > > You might also have the timers auto-cancelled and auto-destroyed on 
> > > device removal.  But the whole thing seems like a minor coding issue 
> > > rather than something fundamental.
> >
> > 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.

> 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.

Regards,

Anthony Liguori