Re: [Qemu-devel] [PATCH RFC 1/1] virtio: migrate config_vector

[Christian Borntraeger]

Am 14.05.2015 um 11:36 schrieb Michael S. Tsirkin:
> On Thu, May 14, 2015 at 11:22:13AM +0200, Christian Borntraeger wrote:
>> Am 13.05.2015 um 23:47 schrieb Michael S. Tsirkin:
>>> On Wed, May 13, 2015 at 08:57:00PM +0200, Christian Borntraeger wrote:
>>>> Am 13.05.2015 um 18:14 schrieb Michael S. Tsirkin:
>>>>>> - AFAICS, there's no easy way to add transport-specific subsections -
>>>>>>   and simply adding config_vector in ccw would break compatibility
>>>>>
>>>>> subsections break compatibility too.  The only way around that is to set
>>>>> a flag to skip migrating config_vector for old machine types.
>>>>
>>>> My main concern is about undetected compatibility issues. A subsection 
>>>> will 
>>>> tell the user that something went wrong. What happens if we just add a new
>>>> qemu_put_byte in the stream. Will the savevm core always detect that we 
>>>> have
>>>> too many or not enough bytes? If yes, adding new stuff in the stream will
>>>> always be detected in some way as error we can go with just adding
>>>> qemu_put_be16/qemu_get_be16 in 
>>>> virtio_ccw_save_config/virtio_ccw_load_config.
>>>> Old/new QEMUs will then not be compatible - but thats probably ok as long 
>>>> as it
>>>> errors out.
>>>>
>>>> My understanding was that we do not have a guarentee that this will be
>>>> detected all the time and having random junk in some variables is a 
>>>> debugging
>>>> nightmare. Is that correct?
>>>>
>>>>
>>>> Christian
>>>
>>> It's not too bad - normally there's a bunch of strings that
>>> helps you find out what's going on.
>>> But if you really care about debuggability of migration streams, help move
>>> forward dgilbert's RFC that switched to a self-delimiting format.
>>> Just piling up random hacks in virtio seems like a wrong approach.
>>>
>>
>> Thats not my question. PLEASE try to understand my question.
>> I want a hard stop if migration changes in incompatible ways.
>> If adding a qemu_put_byte in virtio_ccw gets detected we can just fix
>> virtio_ccw AS YOU SUGGESTED. I just want to know if I can rely on that 
>> or not.
>>
>> Christian 
> 
> I answered exactly this question but let me try to spell the answer
> out a bit more.
> 
> There are three answers:
> 1.  Yes, it's sure to get detected because everything gets shifted
>     and then you get an unexpected string instead of next device name.

Ok got it. So simply adding a qemu_put/get_byte will always fail the migration 
and we
can just fixup virtio-ccw.c at the cost of being not migrateable between 
versions before/after that change. 

Thanks

Christian


> 2.  If you want a more generic way to detect this, then please work
>     on changing format for devices generally so each device
>     section has a byte length attached to it. Then we know that
>     when we make changes, they are detected as device will end
>     earlier/later than expected.
> 3.  You can have a different workaround: add property "skip config vec
>     on migration" and set it for old spapr machine types.
>     old types continue losing config vec; new ones work better.
>