[libvirt] [PATCH v2 0/5] Callbacks to detect connection failure

Update of this series https://www.redhat.com/archives/libvir-list/2012-July/msg00917.html Most significant change is adding a separate API to allow the callback to be unregistered

From: "Daniel P. Berrange" <berrange@redhat.com> Define new virConnect{Register,Unregister}CloseCallback() public APIs which allows registering/unregistering 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 | 49 +++++++++++++++---- src/datatypes.c | 3 ++ src/datatypes.h | 5 ++ src/libvirt.c | 111 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + 5 files changed, 160 insertions(+), 10 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index fcef461..d9f0397 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -49,6 +49,24 @@ 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 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. + * + * It is forbidden to call any other libvirt APIs from an + * implementation of this callback, since it can be invoked + * from a context which is not re-entrant safe. Failure to + * abide by this requirement may lead to application deadlocks + * or crashes. + */ +typedef void (*virFreeCallback)(void *opaque); + + /** * virConnect: * @@ -1148,6 +1166,27 @@ 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 virConnectRegisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb); +int virConnectUnregisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb); /* * Capabilities of the connection / driver. @@ -2863,16 +2902,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 699c4a8..37a31d0 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -115,6 +115,9 @@ virReleaseConnect(virConnectPtr conn) { virMutexLock(&conn->lock); + if (conn->closeFreeCallback) + conn->closeFreeCallback(conn->closeOpaque); + virResetError(&conn->err); virURIFree(conn->uri); diff --git a/src/datatypes.h b/src/datatypes.h index 9ad2d01..8ac9171 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 8315b4f..9cfabc5 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18613,6 +18613,117 @@ error: /** + * virConnectRegisterCloseCallback: + * @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 virConnectRegisterCloseCallback(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); + + virCheckNonNullArgGoto(cb, error); + + 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; +} + +/** + * virConnectUnregisterCloseCallback: + * @conn: pointer to connection object + * @cb: pointer to the current registered callback + * + * Unregisters the callback previously set with the + * virConnectRegisterCloseCallback method. The callback + * will no longer received notifications when the connection + * closes. + * + * Returns 0 on success, -1 on error + */ +int virConnectUnregisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb) +{ + VIR_DEBUG("conn=%p", conn); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&conn->lock); + + virCheckNonNullArgGoto(cb, error); + + if (conn->closeCallback != cb) { + virLibConnError(VIR_ERR_OPERATION_INVALID, "%s", + _("A different callback was requested")); + goto error; + } + + conn->closeCallback = NULL; + if (conn->closeFreeCallback) + conn->closeFreeCallback(conn->closeOpaque); + + 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 1a8e58a..5004182 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -547,6 +547,8 @@ LIBVIRT_0.9.13 { LIBVIRT_0.9.14 { global: virDomainGetHostname; + virConnectRegisterCloseCallback; + virConnectUnregisterCloseCallback; } LIBVIRT_0.9.13; # .... define new API here using predicted next version number .... -- 1.7.10.4

On Tue, Jul 24, 2012 at 14:17:01 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Define new virConnect{Register,Unregister}CloseCallback() public APIs which allows registering/unregistering 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'.
The virConnectCloseReason enum also mentions 'client' reason.
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index fcef461..d9f0397 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in ... @@ -1148,6 +1166,27 @@ 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; ... diff --git a/src/libvirt.c b/src/libvirt.c index 8315b4f..9cfabc5 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18613,6 +18613,117 @@ error:
/** + * virConnectRegisterCloseCallback: + * @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 virConnectRegisterCloseCallback(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); + + virCheckNonNullArgGoto(cb, error); + + 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; +} + +/** + * virConnectUnregisterCloseCallback: + * @conn: pointer to connection object + * @cb: pointer to the current registered callback + * + * Unregisters the callback previously set with the + * virConnectRegisterCloseCallback method. The callback + * will no longer received notifications when the connection
s/received/receive/
+ * closes.
Is it worth noting that freecb() is called on opaque or is it obvious enough?
+ * + * Returns 0 on success, -1 on error + */ +int virConnectUnregisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb) +{ + VIR_DEBUG("conn=%p", conn); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + virMutexLock(&conn->lock); + + virCheckNonNullArgGoto(cb, error); + + if (conn->closeCallback != cb) { + virLibConnError(VIR_ERR_OPERATION_INVALID, "%s", + _("A different callback was requested")); + goto error; + } + + conn->closeCallback = NULL; + if (conn->closeFreeCallback) + conn->closeFreeCallback(conn->closeOpaque);
I'd also reset closeFreeCallback and closeOpaque to NULL to avoid accessing them by mistake in the future.
+ + 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 ...
ACK with nits fixed or ignored as not being worth it. Jirka

On 07/26/12 12:57, Jiri Denemark wrote:
On Tue, Jul 24, 2012 at 14:17:01 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Define new virConnect{Register,Unregister}CloseCallback() public APIs which allows registering/unregistering 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'.
This patch misses implementation of python bindings for this API functions. I pushed commit 72f55fac10a901bb0d953698b71e26f01b2db3c that fixes the build (skips generating of this function bodies as the generator can't do it) but they'll need to be implemented in a followup patch. Peter

On Mon, Jul 30, 2012 at 03:52:37PM +0200, Peter Krempa wrote:
On 07/26/12 12:57, Jiri Denemark wrote:
On Tue, Jul 24, 2012 at 14:17:01 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Define new virConnect{Register,Unregister}CloseCallback() public APIs which allows registering/unregistering 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'.
This patch misses implementation of python bindings for this API functions. I pushed commit 72f55fac10a901bb0d953698b71e26f01b2db3c that fixes the build (skips generating of this function bodies as the generator can't do it) but they'll need to be implemented in a followup patch.
Oh damn it, I knew there was something I'd forget. Not sure why I didn't see a python build failure myself, perhaps I'm missing python-devel on the machine I developed this on. 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> 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 0e7e423..a258746 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 Tue, Jul 24, 2012 at 14:17:02 +0100, 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(-)
ACK Jirka

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/libvirt_private.syms | 2 +- src/rpc/virnetclient.c | 35 +++++++++++++++++++++++++++++++---- src/rpc/virnetclient.h | 9 +++++++++ 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 734c881..83ca99f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,7 +1319,7 @@ virNetClientSendNoReply; virNetClientSendNonBlock; virNetClientSendWithReply; virNetClientSendWithReplyStream; -virNetClientSetEOFNotify; +virNetClientSetCloseCallback; virNetClientSetSASLSession; virNetClientSetTLSSession; diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index a258746..aba58ec 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->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 3b8cbb2..c939475 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 Tue, Jul 24, 2012 at 14:17:03 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Allow detection of socket close in virNetClient via an callback
s/an/a/
function, triggered on any condition that causes the socket to be close.
s/close/closed/
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 2 +- src/rpc/virnetclient.c | 35 +++++++++++++++++++++++++++++++---- src/rpc/virnetclient.h | 9 +++++++++ 3 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 734c881..83ca99f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,7 +1319,7 @@ virNetClientSendNoReply; virNetClientSendNonBlock; virNetClientSendWithReply; virNetClientSendWithReplyStream; -virNetClientSetEOFNotify; +virNetClientSetCloseCallback; virNetClientSetSASLSession; virNetClientSetTLSSession;
Oops, how did virNetClientSetEOFNotify sneak in? ACK

On Thu, Jul 26, 2012 at 03:17:21PM +0200, Jiri Denemark wrote:
On Tue, Jul 24, 2012 at 14:17:03 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Allow detection of socket close in virNetClient via an callback
s/an/a/
function, triggered on any condition that causes the socket to be close.
s/close/closed/
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 2 +- src/rpc/virnetclient.c | 35 +++++++++++++++++++++++++++++++---- src/rpc/virnetclient.h | 9 +++++++++ 3 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 734c881..83ca99f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,7 +1319,7 @@ virNetClientSendNoReply; virNetClientSendNonBlock; virNetClientSendWithReply; virNetClientSendWithReplyStream; -virNetClientSetEOFNotify; +virNetClientSetCloseCallback; virNetClientSetSASLSession; virNetClientSetTLSSession;
Oops, how did virNetClientSetEOFNotify sneak in?
A patch approving a global cleanup of the virNetClient syms was approved and I cherry-picked it, not realizing it contained this symbol from a patch not yet approved 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> 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 a69e69a..d41be55 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -320,6 +320,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: * @@ -667,6 +680,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 Tue, Jul 24, 2012 at 14:17:04 +0100, 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(+)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a69e69a..d41be55 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -320,6 +320,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); + } +} +
Hmm, should we lock conn first to avoid possible race condition with virConnectUnregisterCloseCallback()? Jirka

On Thu, Jul 26, 2012 at 03:23:48PM +0200, Jiri Denemark wrote:
On Tue, Jul 24, 2012 at 14:17:04 +0100, 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(+)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a69e69a..d41be55 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -320,6 +320,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); + } +} +
Hmm, should we lock conn first to avoid possible race condition with virConnectUnregisterCloseCallback()?
Yes, that's a good point. 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> 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/datatypes.h | 2 ++ src/libvirt.c | 3 ++- src/remote/remote_driver.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/datatypes.h b/src/datatypes.h index 8ac9171..a303cdc 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -191,6 +191,8 @@ struct _virConnect { virConnectCloseFunc closeCallback; void *closeOpaque; virFreeCallback closeFreeCallback; + bool closeDispatch; + unsigned closeUnregisterCount; int refs; /* reference count */ }; diff --git a/src/libvirt.c b/src/libvirt.c index 160ace7..d352cfd 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18711,7 +18711,8 @@ int virConnectUnregisterCloseCallback(virConnectPtr conn, } conn->closeCallback = NULL; - if (conn->closeFreeCallback) + conn->closeUnregisterCount++; + if (!conn->closeDispatch && conn->closeFreeCallback) conn->closeFreeCallback(conn->closeOpaque); conn->closeFreeCallback = NULL; conn->closeOpaque = NULL; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a69e69a..9ac27b2 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -320,6 +320,32 @@ enum virDrvOpenRemoteFlags { }; +static void remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED, + int reason, + void *opaque) +{ + virConnectPtr conn = opaque; + + virMutexLock(&conn->lock); + if (conn->closeCallback) { + virConnectCloseFunc closeCallback = conn->closeCallback; + void *closeOpaque = conn->closeOpaque; + virFreeCallback closeFreeCallback = conn->closeFreeCallback; + unsigned closeUnregisterCount = conn->closeUnregisterCount; + + VIR_DEBUG("Triggering connection close callback %p reason=%d", + conn->closeCallback, reason); + conn->closeDispatch = true; + virMutexUnlock(&conn->lock); + closeCallback(conn, reason, closeOpaque); + virMutexLock(&conn->lock); + conn->closeDispatch = false; + if (conn->closeUnregisterCount != closeUnregisterCount) + closeFreeCallback(closeOpaque); + } + virMutexUnlock(&conn->lock); +} + /* * URIs that this driver needs to handle: * @@ -667,6 +693,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 Fri, Jul 27, 2012 at 11:39:55 +0100, 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/datatypes.h | 2 ++ src/libvirt.c | 3 ++- src/remote/remote_driver.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/src/datatypes.h b/src/datatypes.h index 8ac9171..a303cdc 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -191,6 +191,8 @@ struct _virConnect { virConnectCloseFunc closeCallback; void *closeOpaque; virFreeCallback closeFreeCallback; + bool closeDispatch; + unsigned closeUnregisterCount;
int refs; /* reference count */ }; diff --git a/src/libvirt.c b/src/libvirt.c index 160ace7..d352cfd 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18711,7 +18711,8 @@ int virConnectUnregisterCloseCallback(virConnectPtr conn, }
conn->closeCallback = NULL; - if (conn->closeFreeCallback) + conn->closeUnregisterCount++; + if (!conn->closeDispatch && conn->closeFreeCallback) conn->closeFreeCallback(conn->closeOpaque); conn->closeFreeCallback = NULL; conn->closeOpaque = NULL; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a69e69a..9ac27b2 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -320,6 +320,32 @@ enum virDrvOpenRemoteFlags { };
+static void remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED, + int reason, + void *opaque) +{ + virConnectPtr conn = opaque; + + virMutexLock(&conn->lock); + if (conn->closeCallback) { + virConnectCloseFunc closeCallback = conn->closeCallback; + void *closeOpaque = conn->closeOpaque; + virFreeCallback closeFreeCallback = conn->closeFreeCallback; + unsigned closeUnregisterCount = conn->closeUnregisterCount; + + VIR_DEBUG("Triggering connection close callback %p reason=%d", + conn->closeCallback, reason); + conn->closeDispatch = true; + virMutexUnlock(&conn->lock); + closeCallback(conn, reason, closeOpaque); + virMutexLock(&conn->lock); + conn->closeDispatch = false; + if (conn->closeUnregisterCount != closeUnregisterCount) + closeFreeCallback(closeOpaque); + } + virMutexUnlock(&conn->lock); +} + /* * URIs that this driver needs to handle: *
Looks good. However, do we need to protect against the following scenario: - remoteClientCloseFunc locks conn, remembers stuff, unlocks conn - virConnectUnregisterCloseCallback - virConnectRegisterCloseCallback - virConnectUnregisterCloseCallback - remoteClientCloseFunc finishes Now the second registered callback won't be unregistered. It doesn't seem like something that could happen in the real world, though. Jirka

On Fri, Jul 27, 2012 at 06:20:04PM +0200, Jiri Denemark wrote:
On Fri, Jul 27, 2012 at 11:39:55 +0100, 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/datatypes.h | 2 ++ src/libvirt.c | 3 ++- src/remote/remote_driver.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/src/datatypes.h b/src/datatypes.h index 8ac9171..a303cdc 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -191,6 +191,8 @@ struct _virConnect { virConnectCloseFunc closeCallback; void *closeOpaque; virFreeCallback closeFreeCallback; + bool closeDispatch; + unsigned closeUnregisterCount;
int refs; /* reference count */ }; diff --git a/src/libvirt.c b/src/libvirt.c index 160ace7..d352cfd 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18711,7 +18711,8 @@ int virConnectUnregisterCloseCallback(virConnectPtr conn, }
conn->closeCallback = NULL; - if (conn->closeFreeCallback) + conn->closeUnregisterCount++; + if (!conn->closeDispatch && conn->closeFreeCallback) conn->closeFreeCallback(conn->closeOpaque); conn->closeFreeCallback = NULL; conn->closeOpaque = NULL; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a69e69a..9ac27b2 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -320,6 +320,32 @@ enum virDrvOpenRemoteFlags { };
+static void remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED, + int reason, + void *opaque) +{ + virConnectPtr conn = opaque; + + virMutexLock(&conn->lock); + if (conn->closeCallback) { + virConnectCloseFunc closeCallback = conn->closeCallback; + void *closeOpaque = conn->closeOpaque; + virFreeCallback closeFreeCallback = conn->closeFreeCallback; + unsigned closeUnregisterCount = conn->closeUnregisterCount; + + VIR_DEBUG("Triggering connection close callback %p reason=%d", + conn->closeCallback, reason); + conn->closeDispatch = true; + virMutexUnlock(&conn->lock); + closeCallback(conn, reason, closeOpaque); + virMutexLock(&conn->lock); + conn->closeDispatch = false; + if (conn->closeUnregisterCount != closeUnregisterCount) + closeFreeCallback(closeOpaque); + } + virMutexUnlock(&conn->lock); +} + /* * URIs that this driver needs to handle: *
Looks good. However, do we need to protect against the following scenario:
- remoteClientCloseFunc locks conn, remembers stuff, unlocks conn - virConnectUnregisterCloseCallback - virConnectRegisterCloseCallback - virConnectUnregisterCloseCallback - remoteClientCloseFunc finishes
Now the second registered callback won't be unregistered. It doesn't seem like something that could happen in the real world, though.
I doubt anyone will ever call UnregisterCloseCallback, let alone try to register a new one. One of the things I want todo, however, is to add glib style signal handling into virObject. This would enable use to safely handle the above situation in the future. 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 Mon, Jul 30, 2012 at 10:08:27 +0100, Daniel P. Berrange wrote:
On Fri, Jul 27, 2012 at 06:20:04PM +0200, Jiri Denemark wrote:
On Fri, Jul 27, 2012 at 11:39:55 +0100, 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/datatypes.h | 2 ++ src/libvirt.c | 3 ++- src/remote/remote_driver.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/src/datatypes.h b/src/datatypes.h index 8ac9171..a303cdc 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -191,6 +191,8 @@ struct _virConnect { virConnectCloseFunc closeCallback; void *closeOpaque; virFreeCallback closeFreeCallback; + bool closeDispatch; + unsigned closeUnregisterCount;
int refs; /* reference count */ }; diff --git a/src/libvirt.c b/src/libvirt.c index 160ace7..d352cfd 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18711,7 +18711,8 @@ int virConnectUnregisterCloseCallback(virConnectPtr conn, }
conn->closeCallback = NULL; - if (conn->closeFreeCallback) + conn->closeUnregisterCount++; + if (!conn->closeDispatch && conn->closeFreeCallback) conn->closeFreeCallback(conn->closeOpaque); conn->closeFreeCallback = NULL; conn->closeOpaque = NULL; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a69e69a..9ac27b2 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -320,6 +320,32 @@ enum virDrvOpenRemoteFlags { };
+static void remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED, + int reason, + void *opaque) +{ + virConnectPtr conn = opaque; + + virMutexLock(&conn->lock); + if (conn->closeCallback) { + virConnectCloseFunc closeCallback = conn->closeCallback; + void *closeOpaque = conn->closeOpaque; + virFreeCallback closeFreeCallback = conn->closeFreeCallback; + unsigned closeUnregisterCount = conn->closeUnregisterCount; + + VIR_DEBUG("Triggering connection close callback %p reason=%d", + conn->closeCallback, reason); + conn->closeDispatch = true; + virMutexUnlock(&conn->lock); + closeCallback(conn, reason, closeOpaque); + virMutexLock(&conn->lock); + conn->closeDispatch = false; + if (conn->closeUnregisterCount != closeUnregisterCount) + closeFreeCallback(closeOpaque); + } + virMutexUnlock(&conn->lock); +} + /* * URIs that this driver needs to handle: *
Looks good. However, do we need to protect against the following scenario:
- remoteClientCloseFunc locks conn, remembers stuff, unlocks conn - virConnectUnregisterCloseCallback - virConnectRegisterCloseCallback - virConnectUnregisterCloseCallback - remoteClientCloseFunc finishes
Now the second registered callback won't be unregistered. It doesn't seem like something that could happen in the real world, though.
I doubt anyone will ever call UnregisterCloseCallback, let alone try to register a new one.
Yeah, I doubt it too. Good thing is, we won't crash in that case and the possible leak is just client-side. I think this is good enough for now, ACK. Jirka

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 Tue, Jul 24, 2012 at 14:17:05 +0100, 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; + /* 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");
I'd rather print "...due to unknown reason" since new reasons do not have to be errors.
+ 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",
ACK Jirka
participants (3)
-
Daniel P. Berrange
-
Jiri Denemark
-
Peter Krempa