Re: [Qemu-devel] [PATCH 09/17] rtc: add qc annotations

[Avi Kivity]

On 06/05/2012 01:40 PM, Jan Kiszka wrote:
> On 2012-06-05 12:25, Avi Kivity wrote:
>> On 06/05/2012 04:00 AM, Michael Roth wrote:
>>> Add our annotations according to QIDL documentation.
>>>
>>> +qc_declaration typedef struct RTCState {
>>> +    ISADevice _immutable dev;
>>> +    MemoryRegion _immutable io;
>>>      uint8_t cmos_data[128];
>>>      uint8_t cmos_index;
>>>      struct tm current_tm;
>>>      int32_t base_year;
>>> -    qemu_irq irq;
>>> -    qemu_irq sqw_irq;
>>> -    int it_shift;
>>> +    qemu_irq _immutable irq;
>>> +    qemu_irq _immutable sqw_irq;
>> 
>> How is qemu_irq immutable? We're raising and lowering it many times a
>> second.  It's _derived, perhaps, but not immutable.
> 
> No, IRQState in its current form is immutable, doesn't contain any
> volatile state.

Good point.  So it's just like any pointer: it depends on the pointed-to
type.  If it saves its state, then great, but if the pointed-to type
doesn't, then it's broken.

> 
>> 
>>> +    int32_t _immutable it_shift;
>>>      /* periodic timer */
>>>      QEMUTimer *periodic_timer;
>>>      int64_t next_periodic_time;
>>>      /* second update */
>>>      int64_t next_second_time;
>>> -    uint16_t irq_reinject_on_ack_count;
>>> +    uint16_t _derived irq_reinject_on_ack_count;
>> 
>> It's not derived from anything.  It's _host, maybe.
> 
> I think it is _broken.

I think it's _complicated.  Migration involves downtime and so lost
ticks.  In the case of RTC we probably need to trigger compensation code
that will try to handle it according to policy.

>>> +    LostTickPolicy _immutable lost_tick_policy;
>> 
>> _host; nothign prevents us from changing it dynamically in theory.
> 
> _host makes no sense to me. Either it is _immutable or _broken - or is
> properly saved/restored. What should be the semantic of _host?

An emulated device manages some state, and also talks to a host device,
often through an interface like BlockDriverState or CharState. _host is
anything related to the host device that we should be able to
reconstruct on resume.

Example of host state is a CharDriverState filename.  Even if we allow
it to change, there is no point in migrating it since it belongs to the
source namespace, not destination.

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