On Tue, Jun 05, 2012 at 03:42:30PM +0300, Avi Kivity wrote:
> 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.
Agreed, using _derived was an error on my part
>
> 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.
We'd likely only be able to compensate based on calculated downtime or
something along that line. I think we'd still want to send accumulated ticks
as well, even if it's of little importance, since it's still guest state
of a sort (in the sense that it guest-perceivable) that we should avoid
discarding.
>
> >>> + 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
>
