"Michael S. Tsirkin" <m...@redhat.com> writes: > 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.
No, QEMU should aggressively reuse the number part. Heck, it's free to randomly mess with it without notice. Makes the x-stat-N effectively useless for anything but experimenting. Which is exactly the point of naming them x-. > 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. x-things always look like they'd invite misuse. That look goes away fast when your misuse breaks on every other QEMU upgrade.
