On Fri, Jun 22, 2007 at 03:07:08AM +0100, Daniel P. Berrange wrote:
Some drivers are stateless (Xen, test), while others are stateful (QEMU). The later can only be accessed via the daemon. This patch adds a new internal driver API to allow drivers to register a set of functions for performing work on daemon startup, shutdown and reload. It makes the QEMU driver in qemud/driver.c provide implementations of these funtions. It adapts the qemud/qemud.c to call these new global driver functions. Finally it fixes the idle timeout shutdown of the daemon disabled in an earlier patch. NB this patch adds 3 new exported symbols for private use by the daemon.
[...]
+typedef int (*virDrvStateInitialize) (void); +typedef int (*virDrvStateCleanup) (void); +typedef int (*virDrvStateReload) (void); +typedef int (*virDrvStateActive) (void); + +typedef struct _virStateDriver virStateDriver; +typedef virStateDriver *virStateDriverPtr; + +struct _virStateDriver { + virDrvStateInitialize initialize; + virDrvStateCleanup cleanup; + virDrvStateReload reload; + virDrvStateActive active; +};
/* * Registration @@ -324,6 +338,7 @@ struct _virNetworkDriver { */ int virRegisterDriver(virDriverPtr); int virRegisterNetworkDriver(virNetworkDriverPtr); +int virRegisterStateDriver(virStateDriverPtr);
Hum, shouldn't that be more closely associated to the virDriver itself it looks completely orthogonal, so I'm a bit surprized.
#ifdef __cplusplus } diff -r 968ca2c71e5f src/internal.h --- a/src/internal.h Thu Jun 21 21:20:57 2007 -0400 +++ b/src/internal.h Thu Jun 21 21:20:58 2007 -0400 @@ -214,6 +214,15 @@ int virFreeNetwork (virConnectPtr conn, #define virGetDomain(c,n,u) __virGetDomain((c),(n),(u)) #define virGetNetwork(c,n,u) __virGetNetwork((c),(n),(u))
+int __virStateInitialize(void); +int __virStateCleanup(void); +int __virStateReload(void); +int __virStateActive(void); +#define virStateInitialize() __virStateInitialize() +#define virStateCleanup() __virStateCleanup() +#define virStateReload() __virStateReload() +#define virStateActive() __virStateActive()
Funky, a small comment why we do this might help newbies going over the code. /* * those function are exported by the library but not supposed to be * used for normal use of libvirt. */ or something similar.
--- a/src/libvirt.c Thu Jun 21 21:20:57 2007 -0400 +++ b/src/libvirt.c Thu Jun 21 21:20:58 2007 -0400 @@ -40,6 +40,8 @@ static int virDriverTabCount = 0; static int virDriverTabCount = 0; static virNetworkDriverPtr virNetworkDriverTab[MAX_DRIVERS]; static int virNetworkDriverTabCount = 0; +static virStateDriverPtr virStateDriverTab[MAX_DRIVERS]; +static int virStateDriverTabCount = 0; static int initialized = 0;
I'm still a bit surprized this is separate from the drivers, somehow I would have expected the virStateDriverPtr to be a subfield of virDriver
diff -r 968ca2c71e5f src/libvirt_sym.version --- a/src/libvirt_sym.version Thu Jun 21 21:20:57 2007 -0400 +++ b/src/libvirt_sym.version Thu Jun 21 21:20:58 2007 -0400 @@ -101,5 +101,10 @@
__virEventRegisterImpl;
+ __virStateInitialize; + __virStateCleanup; + __virStateReload; + __virStateActive; +
Are all the __functions used by the daemon ? making a quick check in the end and adding a comment would make sense. But I think the patch is fine even if I'm a bit surprised about the way state is handled. I would have though it was a per driver property and hence associated to the driver structure. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/