qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
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: Tue, 2 Aug 2016 03:11:47 +0300	[thread overview]
Message-ID: <20160802030708-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E0421629E@shsmsx102.ccr.corp.intel.com>

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.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
> 
> Hi Michel,
> 
> Do you have time for this issue recently?  
> 
> Thanks!
> Liang


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

  reply	other threads:[~2016-08-02  0:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-06  2:36 [Qemu-devel] [PATCH v2] balloon: Fix failure of updating guest memory status Liang Li
2016-07-06  8:55 ` Michael S. Tsirkin
2016-07-06  9:23   ` Li, Liang Z
2016-07-06 10:32     ` Michael S. Tsirkin
2016-07-06 12:49       ` Li, Liang Z
2016-07-06 13:40         ` Michael S. Tsirkin
2016-08-01 23:59           ` Li, Liang Z
2016-08-02  0:11             ` Michael S. Tsirkin [this message]
2016-08-03  7:25               ` Ladi Prosek
2016-08-04 15:14                 ` Ladi Prosek
2016-08-04 16:01                   ` Michael S. Tsirkin
2016-08-04 19:45                     ` Ladi Prosek
2016-08-08  7:32                   ` Li, Liang Z

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160802030708-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=liang.z.li@intel.com \
    --cc=lprosek@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).