On Fri, Aug 19, 2016 at 3:39 PM, Roman Kagan <rka...@virtuozzo.com> 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.
>
> Signed-off-by: Roman Kagan <rka...@virtuozzo.com>
> Cc: "Michael S. Tsirkin" <m...@redhat.com>
> Cc: Ladi Prosek <lpro...@redhat.com>
> Cc: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> 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 6d4c57c..f00ad8e 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,
> @@ -414,6 +420,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);
> @@ -436,6 +451,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);
> }
>
> @@ -444,6 +460,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
>
Hi Roman,
I talked to Michael Tsirkin and he agrees with merging this patch for
2.7. Could you please resubmit and use the set_status callback instead
of adding another VM state change handler?
static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
{
VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
if (!vdev->vm_running && s->stats_vq_elem) {
balloon_stats_push_elem(s);
}
}
and
vdc->set_status = virtio_balloon_set_status;
in virtio_balloon_class_init.
This is somewhat urgent because 2.7 will be out soon. If you're busy
or if I don't hear from you I'll post it on your behalf.
Thanks!
Ladi
