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

[Jason Wang <jasowang@redhat.com>]

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)

Thanks

> ---
> Cc: Jason Wang <jasowang@redhat.com>
> ---
>   hw/net/rtl8139.c | 40 ++++++++++++++--------------------------
>   1 file changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 562c1fd..8ccd1d3 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -1352,29 +1352,6 @@ static void RTL8139TallyCounters_dma_write(RTL8139State *s, dma_addr_t tc_addr)
>       pci_dma_write(d, tc_addr + 62,    (uint8_t *)&val16, 2);
>   }
>   
> -/* Loads values of tally counters from VM state file */
> -
> -static const VMStateDescription vmstate_tally_counters = {
> -    .name = "tally_counters",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> -    .fields = (VMStateField[]) {
> -        VMSTATE_UINT64(TxOk, RTL8139TallyCounters),
> -        VMSTATE_UINT64(RxOk, RTL8139TallyCounters),
> -        VMSTATE_UINT64(TxERR, RTL8139TallyCounters),
> -        VMSTATE_UINT32(RxERR, RTL8139TallyCounters),
> -        VMSTATE_UINT16(MissPkt, RTL8139TallyCounters),
> -        VMSTATE_UINT16(FAE, RTL8139TallyCounters),
> -        VMSTATE_UINT32(Tx1Col, RTL8139TallyCounters),
> -        VMSTATE_UINT32(TxMCol, RTL8139TallyCounters),
> -        VMSTATE_UINT64(RxOkPhy, RTL8139TallyCounters),
> -        VMSTATE_UINT64(RxOkBrd, RTL8139TallyCounters),
> -        VMSTATE_UINT16(TxAbt, RTL8139TallyCounters),
> -        VMSTATE_UINT16(TxUndrn, RTL8139TallyCounters),
> -        VMSTATE_END_OF_LIST()
> -    }
> -};
> -
>   static void rtl8139_ChipCmd_write(RTL8139State *s, uint32_t val)
>   {
>       DeviceState *d = DEVICE(s);
> @@ -3222,7 +3199,7 @@ static void rtl8139_pre_save(void *opaque)
>   
>   static const VMStateDescription vmstate_rtl8139 = {
>       .name = "rtl8139",
> -    .version_id = 4,
> +    .version_id = 5,
>       .minimum_version_id = 3,
>       .post_load = rtl8139_post_load,
>       .pre_save  = rtl8139_pre_save,
> @@ -3293,8 +3270,19 @@ static const VMStateDescription vmstate_rtl8139 = {
>           VMSTATE_UINT32(TimerInt, RTL8139State),
>           VMSTATE_INT64(TCTR_base, RTL8139State),
>   
> -        VMSTATE_STRUCT(tally_counters, RTL8139State, 0,
> -                       vmstate_tally_counters, RTL8139TallyCounters),
> +        VMSTATE_UINT64(tally_counters.TxOk, RTL8139State),
> +        VMSTATE_UINT64(tally_counters.RxOk, RTL8139State),
> +        VMSTATE_UINT64(tally_counters.TxERR, RTL8139State),
> +        VMSTATE_UINT32(tally_counters.RxERR, RTL8139State),
> +        VMSTATE_UINT16(tally_counters.MissPkt, RTL8139State),
> +        VMSTATE_UINT16(tally_counters.FAE, RTL8139State),
> +        VMSTATE_UINT32(tally_counters.Tx1Col, RTL8139State),
> +        VMSTATE_UINT32(tally_counters.TxMCol, RTL8139State),
> +        VMSTATE_UINT64(tally_counters.RxOkPhy, RTL8139State),
> +        VMSTATE_UINT64(tally_counters.RxOkBrd, RTL8139State),
> +        VMSTATE_UINT32_V(tally_counters.RxOkMul, RTL8139State, 5),
> +        VMSTATE_UINT16(tally_counters.TxAbt, RTL8139State),
> +        VMSTATE_UINT16(tally_counters.TxUndrn, RTL8139State),
>   
>           VMSTATE_UINT32_V(cplus_enabled, RTL8139State, 4),
>           VMSTATE_END_OF_LIST()