Re: [Qemu-devel] [PATCH 1/1] s390x: vmstatify config migration for virtio-ccw

[Halil Pasic]

On 05/31/2017 06:00 PM, Dr. David Alan Gilbert wrote:
> * Halil Pasic (address@hidden) wrote:
>> Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for
>> flexibility (extending using subsections) and for fun.
>>
>> To achieve this we need to hack the config_vector, which is VirtIODevice
>> (that is common virtio) state, in the middle of the VirtioCcwDevice state
>> representation.  This somewhat ugly, but we have no choice because the

s/This somewhat/This is somewhat
                    
>> stream format needs to be preserved.
> 
> I don't think the hack you have here is any different to the existing
> code really; the flow is pretty much the same.
> 

I agree, it's the best we can do, but still a qdev device migrating the
state of some other qdev device is ugly to me. I prefer keeping the
explanation, as neither what has to be done, nor how it is done is
really straight forward. If you recommend dropping the sentence I
can do that to.

>> Almost no changes in behavior. Exception is everything that comes with
>> vmstate like extra bookkeeping about what's in the stream, and maybe some
>> extra checks and better error reporting.
>>
>> Signed-off-by: Halil Pasic <address@hidden>
> 
> Without actually understanding any of the s390 magic, it does look
> like a faithful conversion, so:
> 
> 
> Reviewed-by: Dr. David Alan Gilbert <address@hidden>

Thanks a lot! I guess Connie is going to have to take the
responsibility and review the s390 magic. This has to go
trough her tree anyway. The eyes of an migration expert are
highly appreciated (even if its s390 only code)! 

Will wait for Connie and probably re-spin with your r-b and
the above typo fixed.

Cheers,
Halil