Re: [Qemu-devel] Re: optional feature

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Sep 16, 2009 at 09:08:59AM -0500, Anthony Liguori wrote:
> Juan Quintela wrote:
>>>> up rtc version +1
>>>> add the two fields that we need (together with rtc-td-hack value)
>>>>       
>>> And why this is better? You can't migrate old VM to new qemu even if you
>>> don't use rtc-td-hack on new one.
>>>     
>>
>> I think the difference between us is:
>> - is rtc-td-hack a hack that should only be used for some users
>> - it is a valid rtc feature that should be available to everybody
>> - it is independent, or it needs an rtc to have any value.
>>   
>
> We need a single table that contains the full state for the device.
>
> Many devices will have knobs.  There are two likely types of knobs:
>
> 1) something that indicates how an array of state is going to be
> 2) a boolean that indicates whether a portion of state is valid
>
> rtc-td falls into the second category.  It makes sense to me that the  
> table state would contain a boolean to indicate whether a given set of  
> state was valid.  You may need a grouping mechanism for this.   It  
> probably makes sense to do this as separate tables.  For instance,
>
> .fields = (VMStateField []) {
>   VMSTATE_BOOL(td_hack, RTCState, (VMStateField[]){
>        VMSTATE_INT32(irq_coalesced, RTCState),
>        VMSTATE_INT32(period, RTCState),
>        VMSTATE_END_OF_LIST()}),
> }
>
> If we can't maintain backwards compatibility using this approach (we  
> definitely can't for rtc-td) then we'll just have to live with that.

We have to? Why do we? And not only won't we have backwards
compatibility now, we will also break it and have to break it each time
we add a knob.
If instead we would only save/load the part of state if
the knob is set, we won't have a problem.

> I also think arrays should be expressed like this FWIW.  Today we have  
> explicit typed arrays.  I would rather see:
>
> .fields = (VMStateField []) {
>  VMSTATE_ARRAY(nirq, PCIBus, (VMStateField[]) {
>      VMSTATE_INT32(irq_count[0], PCIBus),
>      VMSTATE_END_OF_LIST()}),
> }

Same problem here.