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

From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Li, Liang Z" <liang.z.li@intel.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Ladi Prosek <lprosek@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"dgilbert@redhat.com" <dgilbert@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2] balloon: Fix failure of updating guest memory status
Date: Wed, 6 Jul 2016 16:40:09 +0300	[thread overview]
Message-ID: <20160706163656-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E042011BA@shsmsx102.ccr.corp.intel.com>

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.li@intel.com>
> > > > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > > Cc: Ladi Prosek <lprosek@redhat.com>
> > > > > Cc: Paolo Bonzini <pbonzini@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