Re: [Qemu-devel] Bug in virtio_net_load

[Cornelia Huck]

On Thu, 30 Jun 2016 20:23:08 +0300
"Michael S. Tsirkin" <address@hidden> wrote:

> I'm not sure what was I thinking when I applied this:
> it changes load without changing save - how can this work?

The ordering implications are easy to miss :(

> I am inclined to revert 1f8828ef573c83365b4a87a776daf8bcef1caa21 and
> apply this instead:
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 7ed06ea..18153d5 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1499,6 +1499,16 @@ int virtio_load(VirtIODevice *vdev, QEMUFile
> *f, int version_id) }
>      qemu_get_be32s(f, &features);
> 
> +    /*
> +     * Temporarily set guest_features low bits - needed by
> +     * virtio net load code testing for
> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> +     * VIRTIO_NET_F_GUEST_ANNOUNCE and VIRTIO_NET_F_CTRL_VQ.
> +     *
> +     * Note: devices should always test host features in future -
> don't create
> +     * new dependencies like this.

docs/virtio-migration.txt should probably talk about that as well. And
any conditional stuff needs to go into a subsection in the future.

> +     */
> +    vdev->guest_features = features;
> +
>      config_len = qemu_get_be32(f);
> 
>      /*
> 
> Could you please confirm whether this help?
> Jason, Cornelia - any comments?

After staring at the code, I'm inclined to think that this will work.

virtio migration: Frying unsuspecting brains since 2008.
<To be fair, the original code wasn't that convoluted.>