On Wed, Aug 3, 2016 at 9:25 AM, Ladi Prosek <lpro...@redhat.com> wrote: > On Tue, Aug 2, 2016 at 2:11 AM, Michael S. Tsirkin <m...@redhat.com> wrote: >> On Mon, Aug 01, 2016 at 11:59:31PM +0000, Li, Liang Z wrote: >>> > On Wed, Jul 06, 2016 at 12:49:06PM +0000, Li, Liang Z 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 <liang.z...@intel.com> >>> > > > > > > Suggested-by: Paolo Bonzini <pbonz...@redhat.com> >>> > > > > > > Cc: Michael S. Tsirkin <m...@redhat.com> >>> > > > > > > Cc: Ladi Prosek <lpro...@redhat.com> >>> > > > > > > Cc: Paolo Bonzini <pbonz...@redhat.com> >>> > > > > > >>> > > > > > 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? >>> > > > > > >>> > > > > >>> > > > > Could you give more explanation about how it works? I can't catch >>> > > > > you. >>> > > > > >>> > > > > Thanks! >>> > > > > Liang >>> > > > >>> > > > virtqueue_discard before migration >>> > > > >>> > > > virtio_balloon_receive_stats after migration >>> > > > >>> > > >>> > > Sorry, I still can't catch you. Maybe it's easier for you to submit a >>> > > patch than writing a lot a words to make me understand your idea. >>> > >>> > I'm rather busy now. I might look into it towards end of the month. >>> > >>> > > I just don't understand why not to use the version to make things >>> > > easier, is that not the original intent of version id? >>> > >>> > This was the original idea but we stopped using version ids since they >>> > have >>> > many shortcomings. >>> > >>> > > If we want to extend the device and more states are needed, the idea >>> > > you suggest can be used as a common solution? >>> > > >>> > > Thanks! >>> > > Liang >>> > >>> > The idea is to try to avoid adding more state. that's not always possible >>> > but in >>> > this case element was seen but not consumed yet, so it should be possible >>> > for destination to simply get it from the VQ again. >>> > >>> > > > -- >>> > > > MST >>> >>> Hi Michel, >>> >>> Do you have time for this issue recently? >>> >>> Thanks! >>> Liang > > Hi Liang, > > I should be able to look into it this week if you help me with testing. > > Thanks, > Ladi
Please try the attached patch. I have tested it with a simple 'migrate' to save the state and then '-incoming' to load it back. One question for you: is it expected that stats_poll_interval is not preserved by save/load? I had to explicitly set guest-stats-polling-interval on the receiving VM to start getting stats again. It's also the reason why the new virtio_balloon_receive_stats call is not under if (balloon_stats_enabled(s)) because this condition always evaluates to false for me. Thanks! Ladi >> Sorry, doesn't look like I will. >> Idea is to make sure balloon_stats_poll_cb runs >> on source. This will set stats_vq_elem to NULL. >> >> >> -- >> MST
From f2f779e12f4aa4d3469d1b44e54484e66f82a2d7 Mon Sep 17 00:00:00 2001 From: Ladi Prosek <lpro...@redhat.com> Date: Thu, 4 Aug 2016 15:22:05 +0200 Subject: [PATCH] balloon: preserve stats virtqueue state across migrations Signed-off-by: Ladi Prosek <lpro...@redhat.com> --- hw/virtio/virtio-balloon.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 5af429a..1293be0 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -396,6 +396,19 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target) trace_virtio_balloon_to_target(target, dev->num_pages); } +static void virtio_balloon_save(QEMUFile *f, void *opaque, size_t size) +{ + VirtIOBalloon *s = VIRTIO_BALLOON(opaque); + + 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; + } + + virtio_save(VIRTIO_DEVICE(opaque), f); +} + static void virtio_balloon_save_device(VirtIODevice *vdev, QEMUFile *f) { VirtIOBalloon *s = VIRTIO_BALLOON(vdev); @@ -417,6 +430,9 @@ static int virtio_balloon_load_device(VirtIODevice *vdev, QEMUFile *f, s->num_pages = qemu_get_be32(f); s->actual = qemu_get_be32(f); + /* poll the queue for the element we may have discarded on save */ + virtio_balloon_receive_stats(VIRTIO_DEVICE(s), s->svq); + if (balloon_stats_enabled(s)) { balloon_stats_change_timer(s, s->stats_poll_interval); } @@ -481,7 +497,7 @@ static void virtio_balloon_instance_init(Object *obj) NULL, s, NULL); } -VMSTATE_VIRTIO_DEVICE(balloon, 1, virtio_balloon_load, virtio_vmstate_save); +VMSTATE_VIRTIO_DEVICE(balloon, 1, virtio_balloon_load, virtio_balloon_save); static Property virtio_balloon_properties[] = { DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features, -- 2.5.5