[libvirt] [PATCH] event: ignore attempts to replace the event loop impl

Daniel P. Berrange posted 1 patch 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170901124920.12815-1-berrange@redhat.com
src/util/virevent.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
[libvirt] [PATCH] event: ignore attempts to replace the event loop impl
Posted by Daniel P. Berrange 6 years, 7 months ago
Although not previously explicitly documented, the expectation for
the libvirt event loop is that an implementation is registered early
in application startup, before calling any libvirt APIs and then
run forever after. Replacing a previously registered event loop is
not safe & subject to races even if virConnectClose has been called
on open handles, due to delayed deregistration of callbacks during
conenction close.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 src/util/virevent.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/util/virevent.c b/src/util/virevent.c
index e0fd35e41..51d8714df 100644
--- a/src/util/virevent.c
+++ b/src/util/virevent.c
@@ -220,6 +220,13 @@ virEventRemoveTimeout(int timer)
  * existing event loop implementation, then the
  * virEventRegisterDefaultImpl() method can be used to setup
  * the generic libvirt implementation.
+ *
+ * Once registered, the event loop implementation cannot be
+ * changed, and must be run continuously. Note that callbacks
+ * may remain registered for a short time even after calling
+ * virConnectClose on all open connections, so it is not safe
+ * to stop running the event loop immediately after closing
+ * the connection.
  */
 void virEventRegisterImpl(virEventAddHandleFunc addHandle,
                           virEventUpdateHandleFunc updateHandle,
@@ -233,6 +240,12 @@ void virEventRegisterImpl(virEventAddHandleFunc addHandle,
               addHandle, updateHandle, removeHandle,
               addTimeout, updateTimeout, removeTimeout);
 
+    if (addHandleImpl || updateHandleImpl || removeHandleImpl ||
+        addTimeoutImpl || updateTimeoutImpl || removeHandleImpl) {
+        VIR_WARN("Ignoring attempt to replace registered event loop");
+        return;
+    }
+
     addHandleImpl = addHandle;
     updateHandleImpl = updateHandle;
     removeHandleImpl = removeHandle;
-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] event: ignore attempts to replace the event loop impl
Posted by Andrea Bolognani 6 years, 7 months ago
On Fri, 2017-09-01 at 13:49 +0100, Daniel P. Berrange wrote:
> Although not previously explicitly documented, the expectation for
> the libvirt event loop is that an implementation is registered early
> in application startup, before calling any libvirt APIs and then
> run forever after. Replacing a previously registered event loop is
> not safe & subject to races even if virConnectClose has been called
> on open handles, due to delayed deregistration of callbacks during
> conenction close.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  src/util/virevent.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list