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

[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.
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.

-- 
MST