On Fri, 2008-11-14 at 12:59 -0500, David Lively wrote: > On Fri, 2008-11-14 at 17:09 +0000, Daniel P. Berrange wrote: > > Or have the virConnectDomainEventRegister method take an extra parameter > > which is a callback void (*freefunc)(void*). libvirt would just invoke > > that to free the opaque data chunk. > > Yeah, I like this better. The dbus(?) API allows an optional > destructor (freefunc) to be specified for callback userdata. So let's > allow it to be null (in which case we obviously don't call it at remove > time). > > > I think we need a similar thing with the event loops APIs for timers > > and file handle watches, to make it easier to free the opaque data > > blob they have. > > Sounds good too. > > I can make the DomainEvent changes today / this weekend while working on > the Java bindings (since I need them to plug the Java leak), and submit > them on Monday (or perhaps later today, if I don't get diverted). The attached patch implements this change (adds a "freefunc" arg to virConnectDomainEventRegister and calls it on Deregister (or Close)). It also modifies the event-test.c example to register a freefunc and deregister callbacks when interrupted or terminated (to verify the freefuncs are properly called). Dave
commit 1cacb0944958dbd39f002d99721112ec2b8df7f5 Author: David Lively <dlively@xxxxxxxxxxxxxxx> Date: Mon Nov 17 15:48:50 2008 -0500 vi-patch: events As discussed on libvir-list, added an extra arg: void (*freefunc)(void *opaque) to virConnectDomainEventRegister. If non-NULL, this function is called by virConnectDomainEventDeregister() and passed the void *opaque argument registered with the callback being removed. diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index 0a741ea..11d62c7 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -1,7 +1,9 @@ #include <config.h> #include <stdio.h> +#include <stdlib.h> #include <string.h> +#include <signal.h> #if HAVE_SYS_POLL_H #include <sys/types.h> @@ -168,6 +170,13 @@ int myDomainEventCallback2 (virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } +static void myFreeFunc(void *opaque) +{ + char *str = opaque; + printf("%s: Freeing [%s]\n", __FUNCTION__, str); + free(str); +} + /* EventImpl Functions */ int myEventHandleTypeToPollEvent(virEventHandleType events) @@ -254,15 +263,27 @@ void usage(const char *pname) printf("%s uri\n", pname); } +int run = 1; + +static void stop(int sig) +{ + printf("Exiting on signal %d\n", sig); + run = 0; +} + + int main(int argc, char **argv) { - int run=1; int sts; + struct sigaction action_stop = { + .sa_handler = stop + }; if(argc > 1 && STREQ(argv[1],"--help")) { usage(argv[0]); return -1; } + virEventRegisterImpl( myEventAddHandleFunc, myEventUpdateHandleFunc, myEventRemoveHandleFunc, @@ -277,11 +298,16 @@ int main(int argc, char **argv) return -1; } + sigaction(SIGTERM, &action_stop, NULL); + sigaction(SIGINT, &action_stop, NULL); + DEBUG0("Registering domain event cbs"); /* Add 2 callbacks to prove this works with more than just one */ - virConnectDomainEventRegister(dconn, myDomainEventCallback1, NULL); - virConnectDomainEventRegister(dconn, myDomainEventCallback2, NULL); + virConnectDomainEventRegister(dconn, myDomainEventCallback1, + strdup("callback 1"), myFreeFunc); + virConnectDomainEventRegister(dconn, myDomainEventCallback2, + strdup("callback 2"), myFreeFunc); while(run) { struct pollfd pfd = { .fd = h_fd, @@ -315,9 +341,15 @@ int main(int argc, char **argv) } + DEBUG0("Deregistering event handlers"); + virConnectDomainEventDeregister(dconn, myDomainEventCallback1); + virConnectDomainEventDeregister(dconn, myDomainEventCallback2); + + DEBUG0("Closing connection"); if( dconn && virConnectClose(dconn)<0 ) { printf("error closing\n"); } + printf("done\n"); return 0; } diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index d1bb154..c56d272 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -1095,7 +1095,8 @@ typedef int (*virConnectDomainEventCallback)(virConnectPtr conn, int virConnectDomainEventRegister(virConnectPtr conn, virConnectDomainEventCallback cb, - void *opaque); + void *opaque, + void (*freefunc)(void *)); int virConnectDomainEventDeregister(virConnectPtr conn, virConnectDomainEventCallback cb); diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 0ee657a..6a63ef4 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1095,7 +1095,8 @@ typedef int (*virConnectDomainEventCallback)(virConnectPtr conn, int virConnectDomainEventRegister(virConnectPtr conn, virConnectDomainEventCallback cb, - void *opaque); + void *opaque, + void (*freefunc)(void *)); int virConnectDomainEventDeregister(virConnectPtr conn, virConnectDomainEventCallback cb); diff --git a/python/libvir.c b/python/libvir.c index 96ccb28..3888af5 100644 --- a/python/libvir.c +++ b/python/libvir.c @@ -1651,7 +1651,7 @@ libvirt_virConnectDomainEventRegister(ATTRIBUTE_UNUSED PyObject * self, ret = virConnectDomainEventRegister(conn, libvirt_virConnectDomainEventCallback, - (void *)pyobj_conn_inst); + (void *)pyobj_conn_inst, NULL); LIBVIRT_END_ALLOW_THREADS; diff --git a/qemud/remote.c b/qemud/remote.c index e9b2387..68030ea 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -3754,7 +3754,7 @@ remoteDispatchDomainEventsRegister (struct qemud_server *server ATTRIBUTE_UNUSED /* Register event delivery callback */ REMOTE_DEBUG("%s","Registering to relay remote events"); - virConnectDomainEventRegister(client->conn, remoteRelayDomainEvent, client); + virConnectDomainEventRegister(client->conn, remoteRelayDomainEvent, client, NULL); if(ret) ret->cb_registered = 1; diff --git a/src/domain_event.c b/src/domain_event.c index 2d15936..7adc177 100644 --- a/src/domain_event.c +++ b/src/domain_event.c @@ -39,6 +39,9 @@ virDomainEventCallbackListFree(virDomainEventCallbackListPtr list) { int i; for (i=0; i<list->count; i++) { + void (*freefunc)(void *) = list->callbacks[i]->freefunc; + if (freefunc) + (*freefunc)(list->callbacks[i]->opaque); VIR_FREE(list->callbacks[i]); } VIR_FREE(list); @@ -60,6 +63,9 @@ virDomainEventCallbackListRemove(virConnectPtr conn, for (i = 0 ; i < cbList->count ; i++) { if(cbList->callbacks[i]->cb == callback && cbList->callbacks[i]->conn == conn) { + void (*freefunc)(void *) = cbList->callbacks[i]->freefunc; + if (freefunc) + (*freefunc)(cbList->callbacks[i]->opaque); virUnrefConnect(cbList->callbacks[i]->conn); VIR_FREE(cbList->callbacks[i]); @@ -94,7 +100,8 @@ int virDomainEventCallbackListAdd(virConnectPtr conn, virDomainEventCallbackListPtr cbList, virConnectDomainEventCallback callback, - void *opaque) + void *opaque, + void (*freefunc)(void *)) { virDomainEventCallbackPtr event; int n; @@ -120,6 +127,7 @@ virDomainEventCallbackListAdd(virConnectPtr conn, event->conn = conn; event->cb = callback; event->opaque = opaque; + event->freefunc = freefunc; /* Make space on list */ n = cbList->count; diff --git a/src/domain_event.h b/src/domain_event.h index c26ec3b..e1083b3 100644 --- a/src/domain_event.h +++ b/src/domain_event.h @@ -30,6 +30,8 @@ struct _virDomainEventCallback { virConnectPtr conn; virConnectDomainEventCallback cb; void *opaque; + void (*freefunc)(void *); + }; typedef struct _virDomainEventCallback virDomainEventCallback; typedef virDomainEventCallback *virDomainEventCallbackPtr; @@ -46,7 +48,8 @@ void virDomainEventCallbackListFree(virDomainEventCallbackListPtr list); int virDomainEventCallbackListAdd(virConnectPtr conn, virDomainEventCallbackListPtr cbList, virConnectDomainEventCallback callback, - void *opaque); + void *opaque, + void (*freefunc)(void *)); int virDomainEventCallbackListRemove(virConnectPtr conn, virDomainEventCallbackListPtr cbList, diff --git a/src/driver.h b/src/driver.h index 98a2763..d6a6d1f 100644 --- a/src/driver.h +++ b/src/driver.h @@ -281,7 +281,8 @@ typedef int (*virDrvDomainEventRegister) (virConnectPtr conn, void *callback, - void *opaque); + void *opaque, + void (*freefunc)(void *)); typedef int (*virDrvDomainEventDeregister) diff --git a/src/libvirt.c b/src/libvirt.c index f257c0f..221bf50 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5372,7 +5372,8 @@ virStorageVolGetPath(virStorageVolPtr vol) int virConnectDomainEventRegister(virConnectPtr conn, virConnectDomainEventCallback cb, - void *opaque) + void *opaque, + void (*freefunc)(void *)) { if (!VIR_IS_CONNECT(conn)) { @@ -5385,7 +5386,7 @@ virConnectDomainEventRegister(virConnectPtr conn, } if ((conn->driver) && (conn->driver->domainEventRegister)) - return conn->driver->domainEventRegister (conn, cb, opaque); + return conn->driver->domainEventRegister (conn, cb, opaque, freefunc); return -1; } diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f9e93f8..e78bbd7 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3384,12 +3384,13 @@ done: static int qemudDomainEventRegister (virConnectPtr conn, void *callback, - void *opaque) + void *opaque, + void (*freefunc)(void *)) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; return virDomainEventCallbackListAdd(conn, driver->domainEventCallbacks, - callback, opaque); + callback, opaque, freefunc); } static int diff --git a/src/remote_internal.c b/src/remote_internal.c index 2ca7930..3c908df 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -4474,13 +4474,14 @@ remoteAuthPolkit (virConnectPtr conn, struct private_data *priv, int in_open, /*----------------------------------------------------------------------*/ static int remoteDomainEventRegister (virConnectPtr conn, - void *callback ATTRIBUTE_UNUSED, - void *opaque ATTRIBUTE_UNUSED) + void *callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED, + void (*freefunc)(void *)) { struct private_data *priv = conn->privateData; if (virDomainEventCallbackListAdd(conn, priv->callbackList, - callback, opaque) < 0) { + callback, opaque, freefunc) < 0) { error (conn, VIR_ERR_RPC, _("adding cb to list")); return -1; }
-- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list