On Mon, 17 Dec 2012 12:23:59 +0000 Dietmar Maurer <diet...@proxmox.com> 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.