Re: [Qemu-devel] [PATCH v3] balloon: Fix failure of updating guest memory status

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Aug 05, 2016 at 01:36:32PM +0200, Ladi Prosek wrote:
> The stats_vq_elem field keeps track of the state of the balloon stats
> virtqueue protocol but it wasn't preserved across migrations, resulting
> in losing guest memory status updates on the receiving VM.
> 
> This commit adds a new VM state change handler which resets stats_vq_elem
> to NULL when the VM is stopped, eliminating the need for the field to be
> migrated. When the VM starts running again, the field is reinitialized by
> re-popping the element from the virtqueue.
> 
> Signed-off-by: Ladi Prosek <lprosek@redhat.com>
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>

Almost there. Except we should not touch rings
unless guest set DRIVER_OK in status register, and we should
not touch status vq at all unless VIRTIO_BALLOON_F_STATS_VQ
has been negotiated.

I'll post a patch to do this.


> ---
>  hw/virtio/virtio-balloon.c         | 23 +++++++++++++++++++++++
>  include/hw/virtio/virtio-balloon.h |  1 +
>  2 files changed, 24 insertions(+)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 5af429a..65457e9 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -423,6 +423,26 @@ static int virtio_balloon_load_device(VirtIODevice *vdev, QEMUFile *f,
>      return 0;
>  }
>  
> +static void virtio_balloon_vmstate_cb(void *opaque, int running,
> +                                      RunState state)
> +{
> +    VirtIOBalloon *s = opaque;
> +
> +    if (!running) {
> +        /* put the stats element back if the VM is not running */
> +        if (s->stats_vq_elem != NULL) {
> +            virtqueue_discard(s->svq, s->stats_vq_elem, s->stats_vq_offset);
> +            g_free(s->stats_vq_elem);
> +            s->stats_vq_elem = NULL;
> +        }
> +
> +    } else {
> +        /* poll stats queue for the element we may have discarded
> +         * when the VM was stopped */
> +        virtio_balloon_receive_stats(VIRTIO_DEVICE(s), s->svq);
> +    }
> +}
> +
>  static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> @@ -446,6 +466,8 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
>      s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
>  
>      reset_stats(s);
> +
> +    s->change = qemu_add_vm_change_state_handler(virtio_balloon_vmstate_cb, s);
>  }
>  
>  static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
> @@ -453,6 +475,7 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>      VirtIOBalloon *s = VIRTIO_BALLOON(dev);
>  
> +    qemu_del_vm_change_state_handler(s->change);
>      balloon_stats_destroy_timer(s);
>      qemu_remove_balloon_handler(s);
>      virtio_cleanup(vdev);
> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> index 1ea13bd..d72ff7f 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -43,6 +43,7 @@ typedef struct VirtIOBalloon {
>      int64_t stats_last_update;
>      int64_t stats_poll_interval;
>      uint32_t host_features;
> +    VMChangeStateEntry *change;
>  } VirtIOBalloon;
>  
>  #endif
> -- 
> 2.5.5