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

[Michael S. Tsirkin]

On Wed, Jul 06, 2016 at 10:36:33AM +0800, Liang Li wrote:
> After live migration, 'guest-stats' can't get the expected memory
> status in the guest. This issue is caused by commit 4eae2a657d.
> The value of 's->stats_vq_elem' will be NULL after live migration,
> and the check in the function 'balloon_stats_poll_cb()' will
> prevent the 'virtio_notify()' from executing. So guest will not
> update the memory status.
> 
> Commit 4eae2a657d is doing the right thing, but 's->stats_vq_elem'
> should be treated as part of balloon device state and migrated to
> destination if it's not NULL to make everything works well.
> 
> Signed-off-by: Liang Li <address@hidden>
> Suggested-by: Paolo Bonzini <address@hidden>
> Cc: Michael S. Tsirkin <address@hidden>
> Cc: Ladi Prosek <address@hidden>
> Cc: Paolo Bonzini <address@hidden>

I agree there's an issue but we don't change versions anymore.
Breaking migrations for everyone is also not nice.

How about queueing virtio_balloon_receive_stats
so it will get invoked when vm starts?

> ---
>  hw/virtio/virtio-balloon.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 557d3f9..64e80c6 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -31,6 +31,7 @@
>  #include "hw/virtio/virtio-access.h"
>  
>  #define BALLOON_PAGE_SIZE  (1 << VIRTIO_BALLOON_PFN_SHIFT)
> +#define BALLOON_VERSION 2
>  
>  static void balloon_page(void *addr, int deflate)
>  {
> @@ -404,15 +405,24 @@ static void virtio_balloon_save(QEMUFile *f, void 
> *opaque)
>  static void virtio_balloon_save_device(VirtIODevice *vdev, QEMUFile *f)
>  {
>      VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
> +    uint16_t elem_num = 0;
>  
>      qemu_put_be32(f, s->num_pages);
>      qemu_put_be32(f, s->actual);
> +    if (s->stats_vq_elem != NULL) {
> +        elem_num = 1;
> +    }
> +    qemu_put_be16(f, elem_num);
> +    if (elem_num) {
> +        qemu_put_virtqueue_element(f, s->stats_vq_elem);
> +    }
>  }
>  
>  static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id)
>  {
> -    if (version_id != 1)
> +    if (version_id < 1 || version_id > BALLOON_VERSION) {
>          return -EINVAL;
> +    }
>  
>      return virtio_load(VIRTIO_DEVICE(opaque), f, version_id);
>  }
> @@ -421,9 +431,17 @@ static int virtio_balloon_load_device(VirtIODevice 
> *vdev, QEMUFile *f,
>                                        int version_id)
>  {
>      VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
> +    uint16_t elem_num = 0;
>  
>      s->num_pages = qemu_get_be32(f);
>      s->actual = qemu_get_be32(f);
> +    if (version_id == BALLOON_VERSION) {
> +        elem_num = qemu_get_be16(f);
> +        if (elem_num == 1) {
> +            s->stats_vq_elem =
> +                    qemu_get_virtqueue_element(f, sizeof(VirtQueueElement));
> +        }
> +    }
>  
>      if (balloon_stats_enabled(s)) {
>          balloon_stats_change_timer(s, s->stats_poll_interval);
> @@ -455,7 +473,7 @@ static void virtio_balloon_device_realize(DeviceState 
> *dev, Error **errp)
>  
>      reset_stats(s);
>  
> -    register_savevm(dev, "virtio-balloon", -1, 1,
> +    register_savevm(dev, "virtio-balloon", -1, BALLOON_VERSION,
>                      virtio_balloon_save, virtio_balloon_load, s);
>  }
>  
> -- 
> 1.8.3.1