qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 4/4] virtio-balloon: keep collecting stats on sa


From: Ladi Prosek
Subject: Re: [Qemu-devel] [PATCH 4/4] virtio-balloon: keep collecting stats on save/restore
Date: Fri, 19 Aug 2016 11:57:44 +0200

On Thu, Aug 18, 2016 at 8:27 PM, Roman Kagan <address@hidden> wrote:
> Upon save/restore virtio-balloon stats acquisition stops.  The reason is
> that the fact that the (only) virtqueue element is being used by QEMU is
> not recorded anywhere on save, so upon restore it's not released to the
> guest, making further progress impossible.
>
> Saving the information about the used element would introduce unjustified
> vmstate incompatibility.
>
> So instead just make sure the element is pushed before save, leaving the
> ball on the guest side.  For that, add vm state change handler to
> virtio-ballon which would take care of pushing the element if there is
> one.

Please take a look at:
https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg01038.html

virtqueue_discard looks like a better choice than virtqueue_push.
There's no need to cause churn on the guest side and receive an extra
set of stats in between the regular timer callbacks. Also in that
thread see the note about stats_vq_offset which is misused and can be
killed.

Thanks,
Ladi

> Signed-off-by: Roman Kagan <address@hidden>
> Cc: "Michael S. Tsirkin" <address@hidden>
> ---
>  hw/virtio/virtio-balloon.c         | 27 ++++++++++++++++++++++-----
>  include/hw/virtio/virtio-balloon.h |  1 +
>  2 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index b56fecd..66a926a 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -88,10 +88,19 @@ static void balloon_stats_change_timer(VirtIOBalloon *s, 
> int64_t secs)
>      timer_mod(s->stats_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + secs * 
> 1000);
>  }
>
> +static void balloon_stats_push_elem(VirtIOBalloon *s)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +
> +    virtqueue_push(s->svq, s->stats_vq_elem, s->stats_vq_offset);
> +    virtio_notify(vdev, s->svq);
> +    g_free(s->stats_vq_elem);
> +    s->stats_vq_elem = NULL;
> +}
> +
>  static void balloon_stats_poll_cb(void *opaque)
>  {
>      VirtIOBalloon *s = opaque;
> -    VirtIODevice *vdev = VIRTIO_DEVICE(s);
>
>      if (!s->stats_vq_elem) {
>          /* The guest hasn't sent the stats yet (either not enabled or we came
> @@ -100,10 +109,7 @@ static void balloon_stats_poll_cb(void *opaque)
>          return;
>      }
>
> -    virtqueue_push(s->svq, s->stats_vq_elem, s->stats_vq_offset);
> -    virtio_notify(vdev, s->svq);
> -    g_free(s->stats_vq_elem);
> -    s->stats_vq_elem = NULL;
> +    balloon_stats_push_elem(s);
>  }
>
>  static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
> @@ -411,6 +417,15 @@ static int virtio_balloon_load_device(VirtIODevice 
> *vdev, QEMUFile *f,
>      return 0;
>  }
>
> +static void balloon_vm_state_change(void *opaque, int running, RunState 
> state)
> +{
> +    VirtIOBalloon *s = opaque;
> +
> +    if (!running && s->stats_vq_elem) {
> +        balloon_stats_push_elem(s);
> +    }
> +}
> +
>  static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> @@ -433,6 +448,7 @@ static void virtio_balloon_device_realize(DeviceState 
> *dev, Error **errp)
>      s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
>      s->svq = virtio_add_queue(vdev, 1, virtio_balloon_receive_stats);
>
> +    s->change = qemu_add_vm_change_state_handler(balloon_vm_state_change, s);
>      reset_stats(s);
>  }
>
> @@ -441,6 +457,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.7.4
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]