On Thu, Feb 25, 2016 at 12:30:21PM +0300, Roman Kagan wrote: > On Thu, Feb 25, 2016 at 10:54:17AM +0200, Michael S. Tsirkin wrote: > > On Thu, Feb 25, 2016 at 09:44:06AM +0100, Markus Armbruster wrote: > > > "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. > > > > > > Well I already heard: > > > > 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. > > > > in this thread, which seems to mean exactly that people start planning to > > abuse it > > even before it's merged. > > That quote (from yours truly) states the opposite. > > The whole point is that there are several participants in the process, > with independent release cycles and policies, but with a common > "registry" of supported stats (with the master copy being in the kernel, > right?).
For most devices it's the virtio spec. > Once a counter is accepted there, you can start shipping the > guest driver providing it, and you don't have to wait until qemu catches > up: your management level can read "x-stat-NEW_NUMBER" *or* "new_name", > as both NEW_NUMBER and new_name are now allocated for that new counter. > > So yes, people are planning to use it, in particluar, before it's merged > into qemu proper, but I don't see how that creates any extra maintenance > burden on qemu side. Anybody using x- is on their own; the scheme I > sketched seems reasonably safe but is the headache of that management > software anyway. > > Roman. Basically if you do this hack QEMU must not reuse the x-stat-NEW_NUMBER ever, otherwise management handling it will intepret it as legacy QEMU and will break. And the fact we have to argue about it tells me this is a dangerous place to put the debugging hooks since it seems to be begging to be misused. -- MST
