On Fri, Jan 31, 2014 at 07:12:11PM -0700, Eric Blake wrote: > Filtering monitor events by name requires tracking the name for > the duration of the filtering. In order to free the name, I > found it easiest to just piggyback on the user's freecb function, > which gets called when the event is deregistered. > > For events without a name filter, we have the design of multiple > client registrations sharing a common server registration, because > the server side uses the same callback function and we reject > duplicate use of the same function. But with events in the mix, > we want to be able to allow the same function pointer to be used > with more than one event name. The solution is to tweak the > duplicate detection code to only act when there is no additional > filtering; if name filtering is in use, there is exactly one > client registration per server registration. Yes, this means > that there is no longer a bound on the number of server > registrations possible, so a malicious client could repeatedly > register for the same name event to exhaust server memory. On > the other hand, we already restricted monitor events to require > write access (compared to normal events only needing read access), > and separated it into the intentionally unsuported > libvirt-qemu.so, with documentation that using this function is > for debug purposes only; so it is not a security risk worth > worrying about a client trying to abuse multiple registrations. > > * src/conf/domain_event.c (virDomainQemuMonitorEventData): New > struct. > (virDomainQemuMonitorEventFilter) > (virDomainQemuMonitorEventCleanup): New functions. > (virDomainQemuMonitorEventDispatchFunc) > (virDomainQemuMonitorEventStateRegisterID): Use new struct. > * src/conf/object_event.c (virObjectEventCallbackListCount) > (virObjectEventCallbackListAddID) > (virObjectEventCallbackListRemoveID) > (virObjectEventCallbackListMarkDeleteID): Drop duplicate detection > when filtering is in effect. > > Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> > --- > src/conf/domain_event.c | 71 +++++++++++++++++++++++++++++++++++++++++++------ > src/conf/object_event.c | 40 ++++++++++++++++++---------- > 2 files changed, 89 insertions(+), 22 deletions(-) I do wonder if we're adding too much functionality / complexity for an API that we basically don't want people to use in the common case. ACK, though personally I wouldn't bother supporting this feature, nor the following patch. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list