On Dec 14 12:08, Emilio G. Cota wrote: > On Fri, Dec 14, 2018 at 15:57:32 +0000, Aaron Lindsay wrote: > > Emilio, > > > > First, thanks for putting this together - I think everyone doing this > > sort of thing will benefit if we're able to agree on one upstream plugin > > interface. > > > > One thing I'd like to see is support for unregistering callbacks once > > they are registered. For instance, you can imagine that a plugin may > > have 'modality', where it may care about tracing very detailed > > information in one mode, but trace only limited information otherwise - > > perhaps only enough to figure out when it needs to switch back to the > > other mode. > > > > I added a function to the user-facing plugin API in my own version of > > Pavel's plugin patchset to clear all existing plugin instrumentation, > > allowing the plugin to drop instrumentation if it was no longer > > interested. Of course, you could always add logic to a plugin to ignore > > unwanted callbacks, but it could be significantly more efficient to > > remove them entirely. In Pavel's patchset, this was essentially just a > > call to tb_flush(), though I think it would be a little more involved > > for yours. > > This is a good point. > > "Regular" callbacks can be unregistered by just passing NULL as the > callback (see plugin_unregister_cb__locked, which is called from > do_plugin_register_cb).
Ah, okay, thanks - I missed this detail when looking through your patches earlier. > "Direct" callbacks, however (i.e. the ones > that embed a callback directly in the translated code), > would have to be unregistered by flushing the particular TB > (or all TBs, which is probably better from an API viewpoint). > > I think the following API call would do what you need: > > typedef int (*qemu_plugin_reset_cb_t)(qemu_plugin_id_t id); > void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_reset_cb_t cb); > > (or maybe qemu_plugin_reinstall?) I think I prefer your initial suggestion of qemu_plugin_reset - to me it does a better job communicating that existing callbacks will be removed. But, then again, _reinstall makes it more clear that you are responsible for re-adding any callbacks you still want... > The idea is that a plugin can "reset" itself, so that (1) all > its CBs are cleared and (2) the plugin can register new callbacks. > This would all happen in an atomic context (no vCPU running), so > that the plugin would miss no CPU events. The implication being that there would not be the same possibility of other callbacks being called between when qemu_plugin_reset and the qemu_plugin_reset_cb_t callback are called as there is at plugin un-installation time? > How does this sound? I think what you describe is exactly what I'm interested in. -Aaron