Re: [Qemu-devel] [PATCHv2] rtl8139: save/load RxMulOk counter (again)

[Jason Wang <jasowang@redhat.com>]

On 2016年06月21日 18:11, Paolo Bonzini wrote:
> On 21/06/2016 11:36, David Vrabel wrote:
>> >On 21/06/16 08:35, Paolo Bonzini wrote:
>>> >>
>>> >>
>>> >>On 21/06/2016 03:44, Jason Wang wrote:
>>>> >>>
>>>> >>>
>>>> >>>On 2016年06月21日 01:53, David Vrabel wrote:
>>>>> >>>>Commit 9d29cdeaaca3a0383af764000b71492c4fc67c6e (rtl8139: port
>>>>> >>>>TallyCounters to vmstate) introduced in incompatibility in the v4
>>>>> >>>>format as it omitted the RxOkMul counter.
>>>>> >>>>
>>>>> >>>>There are presumably no users that were impacted by the v4 to v4'
>>>>> >>>>breakage, so increase the save version to 5 and re-add the field,
>>>>> >>>>keeping backward compatibility with v4'.
>>>>> >>>>
>>>>> >>>>We can't have a field conditional on the section version in
>>>>> >>>>vmstate_tally_counters since this version checked would not be the
>>>>> >>>>section version (but the version defined in this structure).  So, move
>>>>> >>>>all the fields into the main state structure.
>>>>> >>>>
>>>>> >>>>Signed-off-by: David Vrabel<david.vrabel@citrix.com>
>>>> >>>
>>>> >>>Migration to old version is important for the user and this patch seems
>>>> >>>to break this. How about something like:
>>>> >>>
>>>> >>>- introduce a subsection for RXOKMul
>>>> >>>- only migrate it for new version (e.g >= 2.7)
>> >
>> >I don't see how this can work with snapshots where the QEMU version that
>> >is going to restore the snapshot is not known in advance.
> By "new version" he meant the versioned machine types, e.g.
> pc-i440fx-2.6 and older wouldn't migrate it.
>
>>> >>Introducing a subsection is not really necessary if the value is going
>>> >>to be migrated always, and upstream generally does not have "migrate it
>>> >>only in some version" checks.  This is left for downstreams to implement
>>> >>if they care.  We just don't have the manpower to ensure that migration
>>> >>to older versions works between all releases of QEMU.
>> >
>> >So is this patch acceptable as is?
> I think it is.
>
> Paolo

Applied to -net.

Thanks