[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API |
Date: |
Mon, 17 Dec 2012 10:33:57 -0200 |
On Mon, 17 Dec 2012 12:23:59 +0000
Dietmar Maurer <address@hidden> wrote:
> > > I don't really see any conflicts here?
> >
> > query-balloon resets all stats to -1. So, if it's issued after the stats
> > have been
> > polled all values are lost.
>
> I can't see that in the code. I also tested with my patch, and it does not
> reset
> any stats. So I can't see that conflict?
>
> There is no code to reset stats inside virtio_balloon_stat()
static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
{
VirtIOBalloon *dev = opaque;
#if 0
/* Disable guest-provided stats for now. For more details please check:
* https://bugzilla.redhat.com/show_bug.cgi?id=623903
*
* If you do enable it (which is probably not going to happen as we
* need a new command for it), remember that you also need to fill the
* appropriate members of the BalloonInfo structure so that the stats
* are returned to the client.
*/
if (dev->vdev.guest_features & (1 << VIRTIO_BALLOON_F_STATS_VQ)) {
virtqueue_push(dev->svq, &dev->stats_vq_elem, dev->stats_vq_offset);
virtio_notify(&dev->vdev, dev->svq);
return;
}
#endif
/* Stats are not supported. Clear out any stale values that might
* have been set by a more featureful guest kernel.
*/
reset_stats(dev);
<<<<========================================================
^^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^
info->actual = ram_size - ((uint64_t) dev->actual <<
VIRTIO_BALLOON_PFN_SHIFT);
}
>
> > > > It's important to note that the QMP and HMP interfaces are also
> > > > dropped by this commit. That shouldn't be a problem though, because:
> > > >
> > > > 1. All QMP fields are optional
> > > > 2. This has never been really used
> > >
> > > Why don't we simply implement the missing parts - it's just a few lines of
> > code.
> >
> > For the reasons explained in my last email. We'd need at least two
> > commands, one for to enable polling/set time interval and also query-
> > balloon-stats, as adding this to query-balloon would mix semantics (ie. one
> > field is always there and the others have polling semantics).
>
> Yes, we need 2 commands - and what is the problem?
The problem is adding new commands when that's not required. With QOM
everything can be done with existing commands.
- [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats, Luiz Capitulino, 2012/12/14
- Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API, Eric Blake, 2012/12/18
- Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API, Dietmar Maurer, 2012/12/19
- Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API, Luiz Capitulino, 2012/12/19
- Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API, Eric Blake, 2012/12/20
[Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats, Luiz Capitulino, 2012/12/14
Re: [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats, Dietmar Maurer, 2012/12/15