"Denis V. Lunev" <d...@openvz.org> writes: > On 02/24/2016 06:43 PM, Eric Blake wrote: >> On 02/24/2016 07:31 AM, Michael S. Tsirkin wrote: >>> Roman Kagan <rka...@virtuozzo.com> writes: >>>> 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 <red...@virtuozzo.com> >>>>> > >> >>>>> > >>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 <red...@virtuozzo.com> >>>>> > >>Signed-off-by: Denis V. Lunev <d...@openvz.org> >>>>> > >>CC: Michael S. Tsirkin <m...@redhat.com> >>>>> > >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.
Yes, conditional compilation should be used sparingly. I don't have an opinion on whether using it here is appropriate. >>>>> > Why do you think that it opens "abuse" way? >>>>> >>>>> Because people will use this to hack drivers and management tools >>>>> bypassing qemu. Easy to avoid: shuffle the N in x-stat-N around from time to time, to reinforce the lesson that you must not rely on their presence or semantics. I doubt it'll be necessary beyond the renumbering that happens naturally when we add supported counters, or the reshuffling that happens when somebody messes with the unsupported counters. >>>> I'm curious why you think it's a problem? 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. >> Any management tool that relies on an x- prefix name is broken. Or at least assumes the full risk of breaking without notice whenever QEMU changes. Abbreviating that to just "broken" seems fair enough :) >> We've >> explicitly documented that the x- prefix is unstable and liable to go >> away with a future release. Any management app that wants to use a >> feature beginning with x- should FIRST push hard to get the x- removed >> and stabilize the interface (and libvirt, at least, does just that). >> > this was exactly an original idea. Names started with 'x-' are > _officially_ unstable and for debug purpose. That is why I'd > prefer if v2 of the patchset will be taken. Looks like fair use of x- to me.