Re: [Qemu-devel] [PATCH 1/2] virtio-balloon: export all balloon statistics

[Michael S. Tsirkin]

On Wed, Feb 24, 2016 at 01:01:34PM +0300, Roman Kagan wrote:
> On Tue, Feb 23, 2016 at 05:49:21PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Feb 23, 2016 at 06:29:33PM +0300, Denis V. Lunev wrote:
> > > On 02/23/2016 06:24 PM, Michael S. Tsirkin wrote:
> > > >On Tue, Feb 23, 2016 at 05:59:44PM +0300, Denis V. Lunev wrote:
> > > >>From: Igor Redko <address@hidden>
> > > >>
> > > >>We are making experiments with different autoballooning strategies
> > > >>based on the guest behavior. Thus we need to experiment with different
> > > >>guest statistics. For now every counter change requires QEMU 
> > > >>recompilation
> > > >>and dances with Libvirt.
> > > >>
> > > >>This patch introduces transport for unrecognized counters in 
> > > >>virtio-balloon.
> > > >>This transport can be used for measuring benefits from using new
> > > >>balloon counters, before submitting any patches. Current alternative
> > > >>is 'guest-exec' transport which isn't made for such delicate matters
> > > >>and can influence test results.
> > > >>
> > > >>Originally all counters with tag >= VIRTIO_BALLOON_S_NR were ignored.
> > > >>Instead of this we keep first (VIRTIO_BALLOON_S_NR + 32) counters from 
> > > >>the
> > > >>queue and pass unrecognized ones with the following names: 
> > > >>'x-stat-XXXX',
> > > >>where XXXX is a tag number in hex. Defined counters are reported with 
> > > >>their
> > > >>regular names.
> > > >>
> > > >>Signed-off-by: Igor Redko <address@hidden>
> > > >>Signed-off-by: Denis V. Lunev <address@hidden>
> > > >>CC: Michael S. Tsirkin <address@hidden>
> > > >This seems to open the ABI to abuse.
> > > >Seems like a reasonable way to experiment though.
> > > >How about adding this within #if 0 statements?
> > > >You can uncomment them for debugging ...
> > > I'd prefer to have this enabled.
> > > 
> > > Why do you think that it opens "abuse" way?
> > 
> > Because people will use this to hack drivers and management tools
> > bypassing qemu.
> 
> I'm curious why you think it's a problem?

Because we'll be stuck maintaining them with these x- names.

> Even the existing stats are
> simply propagated to the management level by qemu with no processing
> other than assigning text labels.
>
>  The proposed naming scheme for
> unrecognized counters includes "x-" prefix which explicitly marks them
> as unstable so people using them take their risk.
>
> One of the benefits is forward compatibility, so that counters that have
> graduated into supported ones and have got their own number and name,
> can be made to work with qemu that doesn't yet recognize them.

Then management does start relying on the x- prefixed things,
and once it's used to that it's a slippery slope.

> [ Yes I've seen Den having posted a version hiding this under #ifdef, but I'd
> still be interested to know why the initial proposal wasn't found
> generally useful ]
> 
> Thanks,
> Roman.