On Mon, May 07, 2007 at 04:26:13PM +0100, Daniel P. Berrange wrote: > So currently, as we all know, the QEMU driver is just a shim through to the > QEMU daemon. Looking at the code for Rich's remote driver I quite like the > way the remote driver / server works like, and don't like the ugliness of > having the special case QEMU code in there. We always had a long term plan > to try & make the QEMU driver fit into the regular driver API more cleanly > but not really tackled it yet. Similarly we've periodically brought up the > idea of having asynchronous notifications of 'interesting' state changes > in the drivers, to eliminate the need for polling. > > After a little thinking I have an idea that may let us address both issues > in one go. Or to be more precise, put infrastructure in place to let us > move closer to allowing async notiifcations. sounds quite interesting, though I'm still a bit afraid of async notification because except registering a file descriptor, I don't see a good way to link in the client event loop and that's still someting I find a bit ugly and would rather avoid. [... discussion removed ...] > Oh, and we need to make sure the QEMU driver is never activated locally, > only when libvirt.so is being used as part of 'libvirtd'. > > So at a high level what I think we'd need to add to the internal driver > API to make this possible is > > In driver.h: > > typedef int (*virDrvStartup)(void); > typedef int (*virDrvShutdown)(void); > > Adding these to the 'virDriver' struct. Regular apps using libvirt.so > would never trigger these APIs - they're only intended for the daemon, > so I figure we'd have to private APIs in the libivrt.so (but not in > the libvirt.h) > > __virStartup(void); > __virShutdown(void); > > Since these are only called by the daemon, we now conveniently also have > a way to stop the QEMU driver being locally activated. That sounds reasonnable to me. > > Next up, event loops. urgh, that's the ugly part ... > First, we don't want to mandate a particular impl > of an event loop. If you get into the argument about QT vs GLib vs libevent > and libvirt picks one, then you make it hard for libvirt to integrate with > apps using the other. Second, we don't want to assume that 1 connection > == 1 file handle. We also don't want to assume that the number of file > handles used by a driver stays the same over time. Basically we need to > expect that any single driver connection will have zero or more file > handles that it is interested in. agreed > The DBus library deals with this exact same issue & the solution is very > simple. The app using the libvirt registers a callback which the internal > driver than then use to (un)register file handles. that's a reasonnabe way. I also like the idea of a synchronous function allowing to synchronously check for an event like in the FAM API, but I disgress. > So in src/internal.h we'd add methods for internal drivers to call to > add / remove file handles to/from the event loop: > > /* > * @fd: the file handle on which the event occurred > * @event: bitset of one or more of POLLxxx constants from poll.h > * @opaque: data associated with 'fd' > * > * Return 0 to continue monitoring, -1 if an error occurred > * and the handle should no longer be monitored > */ > typedef int (*virEventCallback)(int fd, int events, void *opaque); > > /* > * @fd: the file handle to start monitoring > * @events: bitset of POLLxxx contants to monitor > * @cb: callback to invoke when an event ocurrs > * @opaque: data to pass into callback's opaque parameter > */ > int virEventAddHandle(int fd, int events, virEventCallback cb, void *opaque); > > /* > * @fd: the file handle to stop monitoring > */ > int virEventRemoveHandle(int fd); I'm not too fond of directly referencing the poll.h values though it is POSIX apparently. > > And we'd also add methods for a user of libvirt to register an event loop > implementation: > > typedef int (*virEventAddHandleImpl)(int fd, int events, opaque); > typedef int (*virEventRemoveHandleImpl)(int fd); > > void virEventInitialize(virEventAddHandleImpl add, virEventRemoveHandleImpl remove); > > > If we want to add support for async events into the public API, then the > virEventInitialize method could instead be in libvirt.h, and exported in > the libvirt.so. Otherwise we could mark it private __virEventInitialize > in libvirt.so. > > That I think should be enough to let us move QEMU into a regular style libvirt > driver. In general that sounds really good. I would probably not push the event API as public right now, especially until I have a clear understanding how it would work when using a remote model and ideally remote will be in in the next verion ;-) > > NB, what follows now is a quick brain-dump - I'm not suggesting implementing > this yet - its really a rough idea of general architecture. We'd definitely > want to prototype it in a test app before committing to something like this. > There's also many many scenarios to be thought about beyond just watching > create/destroy events testing the APIs internally first would make mee feel better definitely :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/