Re: [Qemu-devel] [PATCH] Disable virtio-balloon memory stats interface

[Eduardo Habkost]

On Tue, Sep 14, 2010 at 09:24:11AM -0500, Adam Litke wrote:
> On Tue, 2010-09-14 at 11:09 -0300, Eduardo Habkost wrote:
> > On Wed, Sep 08, 2010 at 09:21:16AM -0500, Adam Litke wrote:
[...]
> > >  static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t 
> > > f)
> > >  {
> > > -    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
> > > +    /*
> > > +     * The addition of memory stats reporting to the virtio balloon 
> > > causes
> > > +     * the 'info balloon' command to become asynchronous.  This is a 
> > > regression
> > > +     * because in some cases it can hang the user monitor.
> > > +     *
> > > +     * Disable this feature until a better interface for asynchronous 
> > > commands
> > > +     * can be worked out.
> > > +     *
> > > +     * -aglitke
> > > +     */
> > > +    /* f |= (1 << VIRTIO_BALLOON_F_STATS_VQ); */
> > 
> > 
> > This field is guest-visible, won't this cause problems on migration?
> 
> I haven't followed migration very closely, but isn't this a common
> problem whenever one migrates a vm to a newer version of qemu that has
> more features.  I thought that virtio feature negotiation would ensure
> that stats have been disabled at the device level and would remain
> disabled post migration.  Please correct me if I am mistaken.

If this is the case, then it's another reason to keep the feature bit
enabled: the feature bit just let the guest know that the host may
request balloon stats at any moment, and it's better to keep that
capability in case the guest is migrated, isn't it?

Also, what happens if we try to migrate from a qemu version that had the
feature bit set to a qemu version without this feature bit?

I don't know the details either, but even if this works gracefully for
migration in both directions, it sound much simpler to not change the
guest-visible machine model and just change the "info balloon" behavior.


> 
> > Isn't it better to disable it on the "info balloon" side, so the guest
> > knows that the host may start requesting stat info eventually? I suggest
> > doing this:
> 
> While I think this method would also work, I would really like to use
> the feature bit if possible, since that is what the mechanism is
> designed for.

My point is that if we can avoid making guest-visible changes easily, we
should do that. It doesn't matter for the guest what the "info balloon"
command does.

-- 
Eduardo