On Fri, Aug 19, 2016 at 11:57:44AM +0200, Ladi Prosek wrote: > On Thu, Aug 18, 2016 at 8:27 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. > > Please take a look at: > https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg01038.html
Thanks for the pointer, I wish I did a better maillist search before submitting. > 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. I forgot about virtqueue_discard; however, I still tend to slightly prefer the push version as it IMO makes things easier to follow: the pumping of stats is always initiated by the guest (both initially on guest driver load and upon restore), and ->stats_vq_elem being non-NULL is enough to know that it needs pushing (both in the timer callback and in vm state change handler). > Also in that thread see the note about stats_vq_offset which is > misused and can be killed. Indeed. Thanks, Roman. > > Signed-off-by: Roman Kagan <rka...@virtuozzo.com> > > Cc: "Michael S. Tsirkin" <m...@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 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 > > > >