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

[Anthony Liguori <anthony@codemonkey.ws>]

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