Re: [Qemu-devel] Re: optional feature

[Anthony Liguori <anthony@codemonkey.ws>]

Michael S. Tsirkin wrote:
>>> If instead we would only save/load the part of state if
>>> the knob is set, we won't have a problem.
>>>   
>>>       
>> The rtc device happens to support an optional feature by splitting the  
>> optional bits into a separate section.  Not every device does this  
>> though so if you want to convert other devices to this style, you'll  
>> break their backwards compatibility.
>>
>> The mechanisms are functionally the same.  One is no more expressive  
>> than the other.
>>     
>
> Yes, separate devices variant is more expressive.
>   

Not when you consider my proposed syntax:

.fields = (VMStateField []) {
  VMSTATE_BOOL(td_hack, RTCState, (VMStateField[]){
       VMSTATE_INT32(irq_coalesced, RTCState),
       VMSTATE_INT32(period, RTCState),
       VMSTATE_END_OF_LIST()}),
}

You could clearly encode this on the wire as a separate section.  You 
could autogenerate the name as "rtc-td_hack".  It won't be backwards 
compatible for save but that's okay.  We can hack things together to 
make it backwards compatible on load.

I don't like this as a wire format though.  The point though is that if 
we get the VMState syntax right, then we can make whatever changes down 
the road we need.

> It is more modular.  With optional features A B C, versioning can not
> support saving only A and C but not B.  This is bad for example for
> backporting features between versions: if C happened to be introduced
> after B, backporting C will force backporting B.
>   

The real argument is against linear versioning.  The whole "optional 
feature" thing is almost orthogonal.

If we want to support downstream versioning, then I think we should 
attack that problem properly instead of shoe horning it via "optional 
features".  This would involve introducing a v4 of the savevm protocol 
that allowed for a minor versions of device state.  QEMU would always 
set the minor version to 0.  If downstream decides to introduce changes, 
they can bump the minor version for a device.  We can also add a minor 
version to the savevm protocol itself along with a vendor id.  This lets 
a vendor identify itself and allows the vendor to change the savevm 
protocol.  We can use reverse fqdn for vendor id to avoid id 
distribution issues.

This solves a number of problems.

If qemu-kvm decides to change a device's state, they can increment a 
minor version of that device and set the vendor id to one allocated for 
qemu-kvm.  Since qemu-kvm has other downstreams, it should also bump the 
savevm protocol minor version to introduce an additional vendor 
id/device minor number.  This way if Fedora/RHEL decides to backport a 
feature into qemu-kvm, they can set their own vendor id and also bump 
the secondary minor version.  This gives downstreams nice, non-linear 
versioning that they can use and extend indefinitely.

It also provides a way to detect whether it's possible to migrate a VM 
from one vendor's qemu to another.  For instance, even though RHEL may 
have backported a feature, if that feature isn't used for a particular 
VM, it's still migratable to an upstream version.  We determine this is 
an entirely programmatic way.  It cannot migrate when that feature is 
enabled (even to a newer qemu that has the feature) but to me, that's a 
not a negative thing.

> if you are not saving irq state, it's better
> to skip the array that create a 0 size array.
> The former works for non-array fields, the later does not,
> and you have to invent a separate valid bit.
>   

I think that's a minor wire detail that has nothing to do with the table 
format.

Regards,

Anthony Liguori