[libvirt] [PATCH 1/5] Add public API to register a callback to be invoked on connection close
From: "Daniel P. Berrange" <berrange@redhat.com> Define a new virConnectSetCloseCallback() public API which allows registering a callback to be invoked when the connection to a hypervisor is closed. The callback is provided with the reason for the close, which may be 'error', 'eof' or 'keepalive'. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/libvirt/libvirt.h.in | 40 +++++++++++++++++++++++-------- src/datatypes.c | 4 ++++ src/datatypes.h | 5 ++++ src/libvirt.c | 53 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++++ 5 files changed, 98 insertions(+), 10 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e34438c..74b3f90 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -49,6 +49,17 @@ extern "C" { * defines VIR_ENUM_SENTINELS. Enumerations for bit values do not * have a *_LAST value, but additional bits may be defined. */ +/* + * virFreeCallback: + * @opaque: opaque user data provided at registration + * + * Type for a domain event callback when the event is deregistered and + * need to be freed, @opaque is provided along with the callback at + * registration time + */ +typedef void (*virFreeCallback)(void *opaque); + + /** * virConnect: * @@ -1148,6 +1159,25 @@ int virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count); +typedef enum { + VIR_CONNECT_CLOSE_REASON_ERROR = 1, /* Misc I/O error */ + VIR_CONNECT_CLOSE_REASON_EOF = 2, /* End-of-file from server */ + VIR_CONNECT_CLOSE_REASON_KEEPALIVE = 3, /* Keepalive timer triggered */ + VIR_CONNECT_CLOSE_REASON_CLIENT = 4, /* Client requested it */ + +# ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_CLOSE_REASON_LAST +# endif +} virConnectCloseReason; + +typedef void (*virConnectCloseFunc)(virConnectPtr conn, + int reason, + void *opaque); + +int virConnectSetCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb); /* * Capabilities of the connection / driver. @@ -2861,16 +2891,6 @@ typedef int (*virConnectDomainEventCallback)(virConnectPtr conn, int detail, void *opaque); -/* - * virFreeCallback: - * @opaque: opaque user data provided at registration - * - * Type for a domain event callback when the event is deregistered and - * need to be freed, @opaque is provided along with the callback at - * registration time - */ -typedef void (*virFreeCallback)(void *opaque); - int virConnectDomainEventRegister(virConnectPtr conn, virConnectDomainEventCallback cb, void *opaque, diff --git a/src/datatypes.c b/src/datatypes.c index d718170..5d415b8 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -115,6 +115,10 @@ virReleaseConnect(virConnectPtr conn) { virMutexLock(&conn->lock); + if (conn->closeOpaque && + conn->closeFreeCallback) + conn->closeFreeCallback(conn->closeOpaque); + virResetError(&conn->err); virURIFree(conn->uri); diff --git a/src/datatypes.h b/src/datatypes.h index fc284d2..af054ac 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -187,6 +187,11 @@ struct _virConnect { virErrorFunc handler; /* associated handlet */ void *userData; /* the user data */ + /* Per-connection close callback */ + virConnectCloseFunc closeCallback; + void *closeOpaque; + virFreeCallback closeFreeCallback; + int refs; /* reference count */ }; diff --git a/src/libvirt.c b/src/libvirt.c index df78e8a..8acb87f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18613,6 +18613,59 @@ error: /** + * virConnectSetCloseCallback: + * @conn: pointer to connection object + * @cb: callback to invoke upon close + * @opaque: user data to pass to @cb + * @freecb: callback to free @opaque + * + * Registers a callback to be invoked when the connection + * is closed. This callback is invoked when there is any + * condition that causes the socket connection to the + * hypervisor to be closed. + * + * This function is only applicable to hypervisor drivers + * which maintain a persistent open connection. Drivers + * which open a new connection for every operation will + * not invoke this. + * + * The @freecb must not invoke any other libvirt public + * APIs, since it is not called from a re-entrant safe + * context. + * + * Returns 0 on success, -1 on error + */ +int virConnectSetCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ + VIR_DEBUG("conn=%p", conn); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&conn->lock); + + if (conn->closeOpaque && + conn->closeFreeCallback) + conn->closeFreeCallback(conn->closeOpaque); + + conn->closeCallback = cb; + conn->closeOpaque = opaque; + conn->closeFreeCallback = freecb; + + virMutexUnlock(&conn->lock); + + return 0; +} + +/** * virDomainSetBlockIoTune: * @dom: pointer to domain object * @disk: path to the block device, or device shorthand diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 2913a81..dab8725 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -544,4 +544,10 @@ LIBVIRT_0.9.13 { virDomainSnapshotRef; } LIBVIRT_0.9.11; + +LIBVIRT_0.9.14 { + global: + virConnectSetCloseCallback; +} LIBVIRT_0.9.13; + # .... define new API here using predicted next version number .... -- 1.7.10.4
From: "Daniel P. Berrange" <berrange@redhat.com> Currently if the keepalive timer triggers, the 'markClose' flag is set on the virNetClient. A controlled shutdown will then be performed. If an I/O error occurs during read or write of the connection an error is raised back to the caller, but the connection isn't marked for close. This patch ensures that all I/O error scenarios always result in the connection being marked for close. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetclient.c | 62 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 11 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index f877934..33d4209 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -101,6 +101,7 @@ struct _virNetClient { virKeepAlivePtr keepalive; bool wantClose; + int closeReason; }; @@ -108,6 +109,8 @@ static void virNetClientIOEventLoopPassTheBuck(virNetClientPtr client, virNetClientCallPtr thiscall); static int virNetClientQueueNonBlocking(virNetClientPtr client, virNetMessagePtr msg); +static void virNetClientCloseInternal(virNetClientPtr client, + int reason); static void virNetClientLock(virNetClientPtr client) @@ -261,7 +264,7 @@ virNetClientKeepAliveStop(virNetClientPtr client) static void virNetClientKeepAliveDeadCB(void *opaque) { - virNetClientClose(opaque); + virNetClientCloseInternal(opaque, VIR_CONNECT_CLOSE_REASON_KEEPALIVE); } static int @@ -484,16 +487,26 @@ void virNetClientFree(virNetClientPtr client) static void +virNetClientMarkClose(virNetClientPtr client, + int reason) +{ + VIR_DEBUG("client=%p, reason=%d", client, reason); + virNetSocketRemoveIOCallback(client->sock); + client->wantClose = true; + client->closeReason = reason; +} + + +static void virNetClientCloseLocked(virNetClientPtr client) { virKeepAlivePtr ka; - VIR_DEBUG("client=%p, sock=%p", client, client->sock); + VIR_DEBUG("client=%p, sock=%p, reason=%d", client, client->sock, client->closeReason); if (!client->sock) return; - virNetSocketRemoveIOCallback(client->sock); virNetSocketFree(client->sock); client->sock = NULL; virNetTLSSessionFree(client->tls); @@ -518,16 +531,21 @@ virNetClientCloseLocked(virNetClientPtr client) } } -void virNetClientClose(virNetClientPtr client) +static void virNetClientCloseInternal(virNetClientPtr client, + int reason) { VIR_DEBUG("client=%p", client); if (!client) return; + if (!client->sock || + client->wantClose) + return; + virNetClientLock(client); - client->wantClose = true; + virNetClientMarkClose(client, reason); /* If there is a thread polling for data on the socket, wake the thread up * otherwise try to pass the buck to a possibly waiting thread. If no @@ -548,6 +566,12 @@ void virNetClientClose(virNetClientPtr client) } +void virNetClientClose(virNetClientPtr client) +{ + virNetClientCloseInternal(client, VIR_CONNECT_CLOSE_REASON_CLIENT); +} + + #if HAVE_SASL void virNetClientSetSASLSession(virNetClientPtr client, virNetSASLSessionPtr sasl) @@ -1351,7 +1375,7 @@ static int virNetClientIOEventLoop(virNetClientPtr client, } if (virKeepAliveTrigger(client->keepalive, &msg)) { - client->wantClose = true; + virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_KEEPALIVE); } else if (msg && virNetClientQueueNonBlocking(client, msg) < 0) { VIR_WARN("Could not queue keepalive request"); virNetMessageFree(msg); @@ -1374,18 +1398,23 @@ static int virNetClientIOEventLoop(virNetClientPtr client, if (saferead(client->wakeupReadFD, &ignore, sizeof(ignore)) != sizeof(ignore)) { virReportSystemError(errno, "%s", _("read on wakeup fd failed")); + virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_ERROR); goto error; } } if (fds[0].revents & POLLOUT) { - if (virNetClientIOHandleOutput(client) < 0) + if (virNetClientIOHandleOutput(client) < 0) { + virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_ERROR); goto error; + } } if (fds[0].revents & POLLIN) { - if (virNetClientIOHandleInput(client) < 0) + if (virNetClientIOHandleInput(client) < 0) { + virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_ERROR); goto error; + } } /* Iterate through waiting calls and if any are @@ -1410,6 +1439,7 @@ static int virNetClientIOEventLoop(virNetClientPtr client, } if (fds[0].revents & (POLLHUP | POLLERR)) { + virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_EOF); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("received hangup / error event on socket")); goto error; @@ -1441,6 +1471,9 @@ static void virNetClientIOUpdateCallback(virNetClientPtr client, { int events = 0; + if (client->wantClose) + return; + if (enableCallback) { events |= VIR_EVENT_HANDLE_READABLE; virNetClientCallMatchPredicate(client->waitDispatch, @@ -1623,6 +1656,8 @@ void virNetClientIncomingEvent(virNetSocketPtr sock, virNetClientLock(client); + VIR_DEBUG("client=%p wantclose=%d", client, client ? client->wantClose : false); + if (!client->sock) goto done; @@ -1635,18 +1670,21 @@ void virNetClientIncomingEvent(virNetSocketPtr sock, if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) { VIR_DEBUG("%s : VIR_EVENT_HANDLE_HANGUP or " "VIR_EVENT_HANDLE_ERROR encountered", __FUNCTION__); - virNetSocketRemoveIOCallback(sock); + virNetClientMarkClose(client, + (events & VIR_EVENT_HANDLE_HANGUP) ? + VIR_CONNECT_CLOSE_REASON_EOF : + VIR_CONNECT_CLOSE_REASON_ERROR); goto done; } if (events & VIR_EVENT_HANDLE_WRITABLE) { if (virNetClientIOHandleOutput(client) < 0) - virNetSocketRemoveIOCallback(sock); + virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_ERROR); } if (events & VIR_EVENT_HANDLE_READABLE) { if (virNetClientIOHandleInput(client) < 0) - virNetSocketRemoveIOCallback(sock); + virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_ERROR); } /* Remove completed calls or signal their threads. */ @@ -1656,6 +1694,8 @@ void virNetClientIncomingEvent(virNetSocketPtr sock, virNetClientIOUpdateCallback(client, true); done: + if (client->wantClose) + virNetClientCloseLocked(client); virNetClientUnlock(client); } -- 1.7.10.4
On 07/19/2012 09:04 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Currently if the keepalive timer triggers, the 'markClose' flag is set on the virNetClient. A controlled shutdown will then be performed. If an I/O error occurs during read or write of the connection an error is raised back to the caller, but the connection isn't marked for close. This patch ensures that all I/O error scenarios always result in the connection being marked for close.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetclient.c | 62 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 11 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index f877934..33d4209 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -101,6 +101,7 @@ struct _virNetClient {
virKeepAlivePtr keepalive; bool wantClose; + int closeReason;
Hmm, I know in 1/5 that I mentioned that starting the enum at 0 instead of 1 would allow VIR_ENUM translation; but here, if the reasons start at 1, then you can merge 'wantClose' and 'closeReason' into a single int variable, 0 when the connection is open, and non-zero with the reason when you want to close. Everything else looks okay. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Allow detection of socket close in virNetClient via an callback function, triggered on any condition that causes the socket to be close. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetclient.c | 35 +++++++++++++++++++++++++++++++---- src/rpc/virnetclient.h | 9 +++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 33d4209..6bab93f 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -102,6 +102,10 @@ struct _virNetClient { virKeepAlivePtr keepalive; bool wantClose; int closeReason; + + virNetClientCloseFunc closeCb; + void *closeOpaque; + virFreeCallback closeFf; }; @@ -125,6 +129,19 @@ static void virNetClientUnlock(virNetClientPtr client) } +void virNetClientSetCloseCallback(virNetClientPtr client, + virNetClientCloseFunc cb, + void *opaque, + virFreeCallback ff) +{ + virNetClientLock(client); + client->closeCb = cb; + client->closeOpaque = opaque; + client->closeFf = ff; + virNetClientUnlock(client); +} + + static void virNetClientIncomingEvent(virNetSocketPtr sock, int events, void *opaque); @@ -463,6 +480,9 @@ void virNetClientFree(virNetClientPtr client) return; } + if (client->closeFf && client->closeOpaque) + client->closeFf(client->closeOpaque); + for (i = 0 ; i < client->nprograms ; i++) virNetClientProgramFree(client->programs[i]); VIR_FREE(client->programs); @@ -519,12 +539,19 @@ virNetClientCloseLocked(virNetClientPtr client) client->keepalive = NULL; client->wantClose = false; - if (ka) { + if (ka || client->closeCb) { + virNetClientCloseFunc closeCb = client->closeCb; + void *closeOpaque = client->closeOpaque; + int closeReason = client->closeReason; client->refs++; virNetClientUnlock(client); - virKeepAliveStop(ka); - virKeepAliveFree(ka); + if (ka) { + virKeepAliveStop(ka); + virKeepAliveFree(ka); + } + if (closeCb) + closeCb(client, closeReason, closeOpaque); virNetClientLock(client); client->refs--; @@ -534,7 +561,7 @@ virNetClientCloseLocked(virNetClientPtr client) static void virNetClientCloseInternal(virNetClientPtr client, int reason) { - VIR_DEBUG("client=%p", client); + VIR_DEBUG("client=%p wantclose=%d", client, client ? client->wantClose : false); if (!client) return; diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 13b4f96..5c6eb5f 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -51,6 +51,15 @@ virNetClientPtr virNetClientNewSSH(const char *nodename, virNetClientPtr virNetClientNewExternal(const char **cmdargv); +typedef void (*virNetClientCloseFunc)(virNetClientPtr client, + int reason, + void *opaque); + +void virNetClientSetCloseCallback(virNetClientPtr client, + virNetClientCloseFunc cb, + void *opaque, + virFreeCallback ff); + void virNetClientRef(virNetClientPtr client); int virNetClientGetFD(virNetClientPtr client); -- 1.7.10.4
On 07/19/2012 09:04 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Allow detection of socket close in virNetClient via an callback function, triggered on any condition that causes the socket to be close.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
@@ -463,6 +480,9 @@ void virNetClientFree(virNetClientPtr client) return; }
+ if (client->closeFf && client->closeOpaque) + client->closeFf(client->closeOpaque);
Again, you should not be forcing closeOpaque to be non-NULL. It's opaque, after all.
@@ -534,7 +561,7 @@ virNetClientCloseLocked(virNetClientPtr client) static void virNetClientCloseInternal(virNetClientPtr client, int reason) { - VIR_DEBUG("client=%p", client); + VIR_DEBUG("client=%p wantclose=%d", client, client ? client->wantClose : false);
Passing 'false' to %d looks odd, but works. If, per my 2/5 comments, you merge wantClose into closeReason, then you'd have an int instead of a bool to print here. ACK, once you fix the non-NULL opaque limitation. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Jul 19, 2012 at 11:57:17AM -0600, Eric Blake wrote:
On 07/19/2012 09:04 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Allow detection of socket close in virNetClient via an callback function, triggered on any condition that causes the socket to be close.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
@@ -463,6 +480,9 @@ void virNetClientFree(virNetClientPtr client) return; }
+ if (client->closeFf && client->closeOpaque) + client->closeFf(client->closeOpaque);
Again, you should not be forcing closeOpaque to be non-NULL. It's opaque, after all.
If opaque is NULL, then there's nothing that needs free'ing.
@@ -534,7 +561,7 @@ virNetClientCloseLocked(virNetClientPtr client) static void virNetClientCloseInternal(virNetClientPtr client, int reason) { - VIR_DEBUG("client=%p", client); + VIR_DEBUG("client=%p wantclose=%d", client, client ? client->wantClose : false);
Passing 'false' to %d looks odd, but works. If, per my 2/5 comments, you merge wantClose into closeReason, then you'd have an int instead of a bool to print here.
ACK, once you fix the non-NULL opaque limitation.
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 :|
On 07/20/2012 08:18 AM, Daniel P. Berrange wrote:
@@ -463,6 +480,9 @@ void virNetClientFree(virNetClientPtr client) return; }
+ if (client->closeFf && client->closeOpaque) + client->closeFf(client->closeOpaque);
Again, you should not be forcing closeOpaque to be non-NULL. It's opaque, after all.
If opaque is NULL, then there's nothing that needs free'ing.
Maybe nothing to free, but there may still be side effects (such as logging) in the closeFf that the caller still wants to have happen. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Update the remote driver to use the virNetClient close callback to trigger the virConnectPtr close callbacks Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/remote/remote_driver.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index acae5d0..9db64ef 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -324,6 +324,19 @@ enum virDrvOpenRemoteFlags { }; +static void remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED, + int reason, + void *opaque) +{ + virConnectPtr conn = opaque; + + if (conn->closeCallback) { + VIR_DEBUG("Triggering connection close callback %p reason=%d", + conn->closeCallback, reason); + conn->closeCallback(conn, reason, conn->closeOpaque); + } +} + /* * URIs that this driver needs to handle: * @@ -671,6 +684,11 @@ doRemoteOpen (virConnectPtr conn, #endif /* WIN32 */ } /* switch (transport) */ + + virNetClientSetCloseCallback(priv->client, + remoteClientCloseFunc, + conn, NULL); + if (!(priv->remoteProgram = virNetClientProgramNew(REMOTE_PROGRAM, REMOTE_PROTOCOL_VERSION, remoteDomainEvents, -- 1.7.10.4
On 07/19/2012 09:04 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Update the remote driver to use the virNetClient close callback to trigger the virConnectPtr close callbacks
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/remote/remote_driver.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Use a driver close callback to trigger shutdown of the events demo program Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- examples/domain-events/events-c/event-test.c | 33 ++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index ef6e77a..f2427ad 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -16,6 +16,8 @@ # define ATTRIBUTE_UNUSED __attribute__((__unused__)) #endif +int run = 1; + /* Prototypes */ const char *eventToString(int event); int myEventAddHandleFunc (int fd, int event, @@ -39,6 +41,31 @@ void usage(const char *pname); /* Callback functions */ + +static void connectClose(virConnectPtr conn ATTRIBUTE_UNUSED, + int reason, + void *opaque ATTRIBUTE_UNUSED) +{ + switch (reason) { + case VIR_CONNECT_CLOSE_REASON_ERROR: + fprintf(stderr, "Connection closed due to I/O error\n"); + break; + case VIR_CONNECT_CLOSE_REASON_EOF: + fprintf(stderr, "Connection closed due to end of file\n"); + break; + case VIR_CONNECT_CLOSE_REASON_KEEPALIVE: + fprintf(stderr, "Connection closed due to keepalive timeout\n"); + break; + case VIR_CONNECT_CLOSE_REASON_CLIENT: + fprintf(stderr, "Connection closed due to client request\n"); + break; + default: + fprintf(stderr, "Connection closed due to unknown error\n"); + break; + }; + run = 0; +} + const char *eventToString(int event) { const char *ret = ""; switch ((virDomainEventType) event) { @@ -380,7 +407,6 @@ void usage(const char *pname) printf("%s uri\n", pname); } -int run = 1; static void stop(int sig) { @@ -426,6 +452,9 @@ int main(int argc, char **argv) return -1; } + virConnectSetCloseCallback(dconn, + connectClose, NULL, NULL); + sigaction(SIGTERM, &action_stop, NULL); sigaction(SIGINT, &action_stop, NULL); @@ -513,7 +542,7 @@ int main(int argc, char **argv) run = 0; } - while (run && virConnectIsAlive(dconn) == 1) { + while (run) { if (virEventRunDefaultImpl() < 0) { virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to run event loop: %s\n", -- 1.7.10.4
On 07/19/2012 09:04 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Use a driver close callback to trigger shutdown of the events demo program
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- examples/domain-events/events-c/event-test.c | 33 ++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index ef6e77a..f2427ad 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -16,6 +16,8 @@ # define ATTRIBUTE_UNUSED __attribute__((__unused__)) #endif
+int run = 1;
bool? Then again, <stdbool.h> isn't available everywhere, and I think our examples can work with C89 even though the rest of our project requires C99.
@@ -380,7 +407,6 @@ void usage(const char *pname) printf("%s uri\n", pname); }
-int run = 1;
not to mention it was just code motion.
@@ -426,6 +452,9 @@ int main(int argc, char **argv) return -1; }
+ virConnectSetCloseCallback(dconn, + connectClose, NULL, NULL);
:) I was right - passing a NULL opaque is useful! ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Jul 19, 2012 at 01:12:33PM -0600, Eric Blake wrote:
On 07/19/2012 09:04 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Use a driver close callback to trigger shutdown of the events demo program
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- examples/domain-events/events-c/event-test.c | 33 ++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index ef6e77a..f2427ad 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -16,6 +16,8 @@ # define ATTRIBUTE_UNUSED __attribute__((__unused__)) #endif
+int run = 1;
bool? Then again, <stdbool.h> isn't available everywhere, and I think our examples can work with C89 even though the rest of our project requires C99.
@@ -380,7 +407,6 @@ void usage(const char *pname) printf("%s uri\n", pname); }
-int run = 1;
not to mention it was just code motion.
@@ -426,6 +452,9 @@ int main(int argc, char **argv) return -1; }
+ virConnectSetCloseCallback(dconn, + connectClose, NULL, NULL);
:) I was right - passing a NULL opaque is useful!
And pasing NULL is already allowed. 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 :|
On 07/19/2012 09:04 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Define a new virConnectSetCloseCallback() public API which allows registering a callback to be invoked when the connection to a hypervisor is closed. The callback is provided with the reason for the close, which may be 'error', 'eof' or 'keepalive'.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/libvirt/libvirt.h.in | 40 +++++++++++++++++++++++-------- src/datatypes.c | 4 ++++ src/datatypes.h | 5 ++++ src/libvirt.c | 53 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++++ 5 files changed, 98 insertions(+), 10 deletions(-)
+typedef enum { + VIR_CONNECT_CLOSE_REASON_ERROR = 1, /* Misc I/O error */
Any reason you skipped 0? It will make wrapping this in a VIR_ENUM harder.
+int virConnectSetCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb);
Do we want a 'flags' argument?
+++ b/src/datatypes.c @@ -115,6 +115,10 @@ virReleaseConnect(virConnectPtr conn) {
virMutexLock(&conn->lock);
+ if (conn->closeOpaque &&
NACK to this half of the condition. A client should be allowed to pass NULL as their opaque data.
+int virConnectSetCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ + VIR_DEBUG("conn=%p", conn); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&conn->lock); + + if (conn->closeOpaque && + conn->closeFreeCallback) + conn->closeFreeCallback(conn->closeOpaque);
Again, no need to check closeOpaque, NULL is valid there.
+ + conn->closeCallback = cb; + conn->closeOpaque = opaque; + conn->closeFreeCallback = freecb;
So the user can call this as many times as they want to override or uninstall an existing registered callback? I guess that works. ACK, once you allow a NULL opaque pointer, and answer why a flags is not useful (or else add a flags). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Jul 19, 2012 at 11:46:28AM -0600, Eric Blake wrote:
On 07/19/2012 09:04 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Define a new virConnectSetCloseCallback() public API which allows registering a callback to be invoked when the connection to a hypervisor is closed. The callback is provided with the reason for the close, which may be 'error', 'eof' or 'keepalive'.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/libvirt/libvirt.h.in | 40 +++++++++++++++++++++++-------- src/datatypes.c | 4 ++++ src/datatypes.h | 5 ++++ src/libvirt.c | 53 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++++ 5 files changed, 98 insertions(+), 10 deletions(-)
+typedef enum { + VIR_CONNECT_CLOSE_REASON_ERROR = 1, /* Misc I/O error */
Any reason you skipped 0? It will make wrapping this in a VIR_ENUM harder.
No, that's a mistake.
+int virConnectSetCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb);
Do we want a 'flags' argument?
No, I don't think so for the callbacks
+++ b/src/datatypes.c @@ -115,6 +115,10 @@ virReleaseConnect(virConnectPtr conn) {
virMutexLock(&conn->lock);
+ if (conn->closeOpaque &&
NACK to this half of the condition. A client should be allowed to pass NULL as their opaque data.
This doesn't prevent them passing NULL. The 'virFreeFunc' callback is for free'ing the 'opaque' data. If the opaque data is NULL, there is nothing that requires free'ing.
+int virConnectSetCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ + VIR_DEBUG("conn=%p", conn); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&conn->lock); + + if (conn->closeOpaque && + conn->closeFreeCallback) + conn->closeFreeCallback(conn->closeOpaque);
Again, no need to check closeOpaque, NULL is valid there.
Same point as above.
+ + conn->closeCallback = cb; + conn->closeOpaque = opaque; + conn->closeFreeCallback = freecb;
So the user can call this as many times as they want to override or uninstall an existing registered callback? I guess that works.
Alternatively I could raise an error, probably nicer than silently overriding an existing callback
ACK, once you allow a NULL opaque pointer, and answer why a flags is not useful (or else add a flags).
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 :|
From: "Daniel P. Berrange" <berrange@redhat.com> Define a new virConnectSetCloseCallback() public API which allows registering a callback to be invoked when the connection to a hypervisor is closed. The callback is provided with the reason for the close, which may be 'error', 'eof' or 'keepalive'. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/libvirt/libvirt.h.in | 40 +++++++++++++++++++++------- src/datatypes.c | 4 +++ src/datatypes.h | 5 ++++ src/libvirt.c | 60 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++++ 5 files changed, 105 insertions(+), 10 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e34438c..85d8c8a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -49,6 +49,17 @@ extern "C" { * defines VIR_ENUM_SENTINELS. Enumerations for bit values do not * have a *_LAST value, but additional bits may be defined. */ +/* + * virFreeCallback: + * @opaque: opaque user data provided at registration + * + * Type for a domain event callback when the event is deregistered and + * need to be freed, @opaque is provided along with the callback at + * registration time + */ +typedef void (*virFreeCallback)(void *opaque); + + /** * virConnect: * @@ -1148,6 +1159,25 @@ int virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count); +typedef enum { + VIR_CONNECT_CLOSE_REASON_ERROR = 0, /* Misc I/O error */ + VIR_CONNECT_CLOSE_REASON_EOF = 1, /* End-of-file from server */ + VIR_CONNECT_CLOSE_REASON_KEEPALIVE = 2, /* Keepalive timer triggered */ + VIR_CONNECT_CLOSE_REASON_CLIENT = 3, /* Client requested it */ + +# ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_CLOSE_REASON_LAST +# endif +} virConnectCloseReason; + +typedef void (*virConnectCloseFunc)(virConnectPtr conn, + int reason, + void *opaque); + +int virConnectSetCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb); /* * Capabilities of the connection / driver. @@ -2861,16 +2891,6 @@ typedef int (*virConnectDomainEventCallback)(virConnectPtr conn, int detail, void *opaque); -/* - * virFreeCallback: - * @opaque: opaque user data provided at registration - * - * Type for a domain event callback when the event is deregistered and - * need to be freed, @opaque is provided along with the callback at - * registration time - */ -typedef void (*virFreeCallback)(void *opaque); - int virConnectDomainEventRegister(virConnectPtr conn, virConnectDomainEventCallback cb, void *opaque, diff --git a/src/datatypes.c b/src/datatypes.c index d718170..5d415b8 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -115,6 +115,10 @@ virReleaseConnect(virConnectPtr conn) { virMutexLock(&conn->lock); + if (conn->closeOpaque && + conn->closeFreeCallback) + conn->closeFreeCallback(conn->closeOpaque); + virResetError(&conn->err); virURIFree(conn->uri); diff --git a/src/datatypes.h b/src/datatypes.h index fc284d2..af054ac 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -187,6 +187,11 @@ struct _virConnect { virErrorFunc handler; /* associated handlet */ void *userData; /* the user data */ + /* Per-connection close callback */ + virConnectCloseFunc closeCallback; + void *closeOpaque; + virFreeCallback closeFreeCallback; + int refs; /* reference count */ }; diff --git a/src/libvirt.c b/src/libvirt.c index df78e8a..7a4a17c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18613,6 +18613,66 @@ error: /** + * virConnectSetCloseCallback: + * @conn: pointer to connection object + * @cb: callback to invoke upon close + * @opaque: user data to pass to @cb + * @freecb: callback to free @opaque + * + * Registers a callback to be invoked when the connection + * is closed. This callback is invoked when there is any + * condition that causes the socket connection to the + * hypervisor to be closed. + * + * This function is only applicable to hypervisor drivers + * which maintain a persistent open connection. Drivers + * which open a new connection for every operation will + * not invoke this. + * + * The @freecb must not invoke any other libvirt public + * APIs, since it is not called from a re-entrant safe + * context. + * + * Returns 0 on success, -1 on error + */ +int virConnectSetCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ + VIR_DEBUG("conn=%p", conn); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&conn->lock); + + if (conn->closeCallback) { + virLibConnError(VIR_ERR_OPERATION_INVALID, "%s", + _("A close callback is already registered")); + goto error; + } + + conn->closeCallback = cb; + conn->closeOpaque = opaque; + conn->closeFreeCallback = freecb; + + virMutexUnlock(&conn->lock); + + return 0; + +error: + virMutexUnlock(&conn->lock); + virDispatchError(NULL); + return -1; +} + +/** * virDomainSetBlockIoTune: * @dom: pointer to domain object * @disk: path to the block device, or device shorthand diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 2913a81..dab8725 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -544,4 +544,10 @@ LIBVIRT_0.9.13 { virDomainSnapshotRef; } LIBVIRT_0.9.11; + +LIBVIRT_0.9.14 { + global: + virConnectSetCloseCallback; +} LIBVIRT_0.9.13; + # .... define new API here using predicted next version number .... -- 1.7.10.4
On 07/20/2012 08:21 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Define a new virConnectSetCloseCallback() public API which allows registering a callback to be invoked when the connection to a hypervisor is closed. The callback is provided with the reason for the close, which may be 'error', 'eof' or 'keepalive'.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/libvirt/libvirt.h.in | 40 +++++++++++++++++++++------- src/datatypes.c | 4 +++ src/datatypes.h | 5 ++++ src/libvirt.c | 60 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++++ 5 files changed, 105 insertions(+), 10 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e34438c..85d8c8a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -49,6 +49,17 @@ extern "C" { * defines VIR_ENUM_SENTINELS. Enumerations for bit values do not * have a *_LAST value, but additional bits may be defined. */
+/* + * virFreeCallback: + * @opaque: opaque user data provided at registration + * + * Type for a domain event callback when the event is deregistered and + * need to be freed, @opaque is provided along with the callback at + * registration time + */ +typedef void (*virFreeCallback)(void *opaque);
We're now using this for more than just domain events; the comment needs a slight update. Maybe: Type for a callback cleanup function to be paired with a callback. This function will be called as a final chance to clean up the @opaque registered with the primary callback, at the time when the primary callback is deregistered.
+++ b/src/datatypes.c @@ -115,6 +115,10 @@ virReleaseConnect(virConnectPtr conn) {
virMutexLock(&conn->lock);
+ if (conn->closeOpaque && + conn->closeFreeCallback) + conn->closeFreeCallback(conn->closeOpaque);
I'm still not sure I like filtering on NULL opaque. Our other uses of a virFreeCallback include src/fdstream.c:virFDStreamRemoveCallback, and there we call fdst->ff without regards to whether fdst->opaque is null.
+int virConnectSetCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ + VIR_DEBUG("conn=%p", conn); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&conn->lock); + + if (conn->closeCallback) { + virLibConnError(VIR_ERR_OPERATION_INVALID, "%s", + _("A close callback is already registered")); + goto error; + }
Do we want to allow deregistration of a close callback (we have virConnectDomainEventDeregister, after all). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange -
Eric Blake