Re: [PATCH] kvm: qemu: virtio-net: migration fixes

Re: [PATCH] kvm: qemu: virtio-net: migration fixes

[Anthony Liguori]

Avi Kivity wrote:
> From: Mark McLoughlin <markmc@redhat.com>
>
> We were failing to save whether the tx/rx buffers we exchange with the tap
> code should include a virtio_net_hdr header; this caused the migrated guest
> to receive garbage packets because the tap code was stripping away the header
> and virtio_net was interpreting packet data as the virtio_net header
>
> With this fix a guest using mergeable rx buffers and
> GSO passes a simple "ping while migrating" test.
>
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> Signed-off-by: Avi Kivity <avi@redhat.com>
>
> diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
> index e2ba322..1f9dc16 100644
> --- a/qemu/hw/virtio-net.c
> +++ b/qemu/hw/virtio-net.c
> @@ -361,6 +361,10 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
>      qemu_put_buffer(f, n->mac, 6);
>      qemu_put_be32(f, n->tx_timer_active);
>      qemu_put_be32(f, n->mergeable_rx_bufs);
> +
> +#ifdef TAP_VNET_HDR
> +    qemu_put_be32(f, tap_has_vnet_hdr(n->vc->vlan->first_client));
> +#endif
>   

This should increment the save/restore format version number to preserve 
backwards compatibility.  It'll also be required for merging into QEMU 
upstream.

As a rule, if new fields are added to a save/load function, the version 
number should be incremented.

Regards,

Anthony Liguori

>  }
>  
>  static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
> @@ -376,6 +380,11 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>      n->tx_timer_active = qemu_get_be32(f);
>      n->mergeable_rx_bufs = qemu_get_be32(f);
>  
> +#ifdef TAP_VNET_HDR
> +    if (qemu_get_be32(f))
> +        tap_using_vnet_hdr(n->vc->vlan->first_client, 1);
> +#endif
> +
>      if (n->tx_timer_active) {
>          qemu_mod_timer(n->tx_timer,
>                         qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL);
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-commits" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

Re: [PATCH] kvm: qemu: virtio-net: migration fixes

[Avi Kivity]

Anthony Liguori wrote:
>> +
>> +#ifdef TAP_VNET_HDR
>> +    qemu_put_be32(f, tap_has_vnet_hdr(n->vc->vlan->first_client));
>> +#endif
>>   
>
> This should increment the save/restore format version number to 
> preserve backwards compatibility.  It'll also be required for merging 
> into QEMU upstream.
>
> As a rule, if new fields are added to a save/load function, the 
> version number should be incremented.

That only works if this is the very next change in qemu upstream.  If 
you accept another fix first, or require changes to this fix, then we're 
out of luck.

I don't see any solution except continuing to merge.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.