[Libvir] Making QEMU use the real internal driver API

[Daniel Veillard]

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 at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/