[jhbuild] Add 2 libvirt patches to fix 2 bugs hit by Boxes



commit cd5bf7285db7ff5800c87a94d064859c1d5720a6
Author: Christophe Fergeau <cfergeau redhat com>
Date:   Mon Sep 10 12:47:12 2012 +0200

    Add 2 libvirt patches to fix 2 bugs hit by Boxes
    
    These 2 patches are upstream

 modulesets/gnome-apps-3.6.modules                  |    5 +-
 .../libvirt-0.10.1-libvirt-connection-closed.patch |   57 +++++++++
 patches/libvirt-0.10.1-lost-events.patch           |  124 ++++++++++++++++++++
 3 files changed, 185 insertions(+), 1 deletions(-)
---
diff --git a/modulesets/gnome-apps-3.6.modules b/modulesets/gnome-apps-3.6.modules
index faa31a1..ae94263 100644
--- a/modulesets/gnome-apps-3.6.modules
+++ b/modulesets/gnome-apps-3.6.modules
@@ -87,7 +87,10 @@
     <branch repo="libvirt.org"
             module="libvirt-0.10.1.tar.gz"
             version="0.10.1"
-            hash="sha256:7b179219b92bff35986e2103b2767423d1e9c284052aa81228eae765f01a074d"/>
+            hash="sha256:7b179219b92bff35986e2103b2767423d1e9c284052aa81228eae765f01a074d">
+      <patch file="libvirt-0.10.1-lost-events.patch" strip="1"/>
+      <patch file="libvirt-0.10.1-libvirt-connection-closed.patch" strip="1"/>
+    </branch>
     <dependencies>
       <dep package="device-mapper"/>
       <dep package="libnl"/>
diff --git a/patches/libvirt-0.10.1-libvirt-connection-closed.patch b/patches/libvirt-0.10.1-libvirt-connection-closed.patch
new file mode 100644
index 0000000..00e64b3
--- /dev/null
+++ b/patches/libvirt-0.10.1-libvirt-connection-closed.patch
@@ -0,0 +1,57 @@
+From 712c9a0932f0b676cfbe5bf8ef3222fcf17d3dec Mon Sep 17 00:00:00 2001
+From: Christophe Fergeau <cfergeau redhat com>
+Date: Mon, 10 Sep 2012 12:17:07 +0200
+Subject: [PATCH 2/2] Fix unwanted closing of libvirt client connection
+
+e5a1bee07 introduced a regression in Boxes: when Boxes is left idle
+(it's still doing some libvirt calls in the background), the
+libvirt connection gets closed after a few minutes. What happens is
+that this code in virNetClientIOHandleOutput gets triggered:
+
+if (!thecall)
+    return -1; /* Shouldn't happen, but you never know... */
+
+and after the changes in e5a1bee07, this causes the libvirt connection
+to be closed.
+
+Upon further investigation, what happens is that
+virNetClientIOHandleOutput is called from gvir_event_handle_dispatch
+in libvirt-glib, which is triggered because the client fd became
+writable. However, between the times gvir_event_handle_dispatch
+is called, and the time the client lock is grabbed and
+virNetClientIOHandleOutput is called, another thread runs and
+completes the current call. 'thecall' is then NULL when the first
+thread gets to run virNetClientIOHandleOutput.
+
+After describing this situation on IRC, danpb suggested this:
+
+11:37 < danpb> In that case I think the correct thing would be to change
+               'return -1' above to 'return 0' since that's not actually an
+               error - its a rare, but expected event
+
+which is what this patch is doing. I've tested it against master
+libvirt, and I didn't get disconnected in ~10 minutes while this
+happens in less than 5 minutes without this patch.
+---
+ src/rpc/virnetclient.c | 5 ++++-
+ 1 file changed, 4 insertions(+), 1 deletion(-)
+
+diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
+index 43a9814..727ed67 100644
+--- a/src/rpc/virnetclient.c
++++ b/src/rpc/virnetclient.c
+@@ -1205,7 +1205,10 @@ virNetClientIOHandleOutput(virNetClientPtr client)
+         thecall = thecall->next;
+ 
+     if (!thecall)
+-        return -1; /* Shouldn't happen, but you never know... */
++        return 0; /* This can happen if another thread raced with us and
++                   * completed the call between the time this thread woke
++                   * up from poll()ing and the time we locked the client
++                   */
+ 
+     while (thecall) {
+         ssize_t ret = virNetClientIOWriteMessage(client, thecall);
+-- 
+1.7.11.4
+
diff --git a/patches/libvirt-0.10.1-lost-events.patch b/patches/libvirt-0.10.1-lost-events.patch
new file mode 100644
index 0000000..c4062a6
--- /dev/null
+++ b/patches/libvirt-0.10.1-lost-events.patch
@@ -0,0 +1,124 @@
+From 826fa2876e13aa0256b6200bb4d6063d79a65e63 Mon Sep 17 00:00:00 2001
+From: Christophe Fergeau <cfergeau redhat com>
+Date: Thu, 6 Sep 2012 08:16:46 +0200
+Subject: [PATCH 1/2] events: Fix domain event race on client disconnect
+
+GNOME Boxes sometimes stops getting domain events from libvirtd, even
+after restarting it. Further investigation in libvirtd shows that
+events are properly queued with virDomainEventStateQueue, but the
+timer virDomainEventTimer which flushes the events and sends them to
+the clients never gets called. Looking at the event queue in gdb
+shows that it's non-empty and that its size increases with each new
+events.
+
+virDomainEventTimer is set up in virDomainEventStateRegister[ID]
+when going from 0 client connecte to 1 client connected, but is
+initially disabled. The timer is removed in
+virDomainEventStateRegister[ID] when the last client is disconnected
+(going from 1 client connected to 0).
+
+This timer (which handles sending the events to the clients) is
+enabled in virDomainEventStateQueue when queueing an event on an
+empty queue (queue containing 0 events). It's disabled in
+virDomainEventStateFlush after flushing the queue (ie removing all
+the elements from it). This way, no extra work is done when the queue
+is empty, and when the next event comes up, the timer will get
+reenabled because the queue will go from 0 event to 1 event, which
+triggers enabling the timer.
+
+However, with this Boxes bug, we have a client connected (Boxes), a
+non-empty queue (there are events waiting to be sent), but a disabled
+timer, so something went wrong.
+
+When Boxes connects (it's the only client connecting to the libvirtd
+instance I used for debugging), the event timer is not set as expected
+(state->timer == -1 when virDomainEventStateRegisterID is called),
+but at the same time the event queue is not empty. In other words,
+we had no clients connected, but pending events. This also explains
+why the timer never gets enabled as this is only done when an event
+is queued on an empty queue.
+
+I think this can happen if an event gets queued using
+virDomainEventStateQueue and the client disconnection happens before
+the event timer virDomainEventTimer gets a chance to run and flush
+the event. In this situation, virDomainEventStateDeregister[ID] will
+get called with a non-empty event queue, the timer will be destroyed
+if this was the only client connected. Then, when other clients connect
+at a later time, they will never get notified about domain events as
+the event timer will never get enabled because the timer is only
+enabled if the event queue is empty when virDomainEventStateRegister[ID]
+gets called, which will is no longer the case.
+
+To avoid this issue, this commit makes sure to remove all events from
+the event queue when the last client in unregistered. As there is
+no longer anyone interested in receiving these events, these events
+are stale so there is no need to keep them around. A client connecting
+later will have no interest in getting events that happened before it
+got connected.
+---
+ src/conf/domain_event.c | 24 +++++++++++++++++++++---
+ 1 file changed, 21 insertions(+), 3 deletions(-)
+
+diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
+index 43ecdcf..7ab973b 100644
+--- a/src/conf/domain_event.c
++++ b/src/conf/domain_event.c
+@@ -525,13 +525,13 @@ void virDomainEventFree(virDomainEventPtr event)
+ }
+ 
+ /**
+- * virDomainEventQueueFree:
++ * virDomainEventQueueClear:
+  * @queue: pointer to the queue
+  *
+- * Free the memory in the queue. We process this like a list here
++ * Removes all elements from the queue
+  */
+ static void
+-virDomainEventQueueFree(virDomainEventQueuePtr queue)
++virDomainEventQueueClear(virDomainEventQueuePtr queue)
+ {
+     int i;
+     if (!queue)
+@@ -541,6 +541,22 @@ virDomainEventQueueFree(virDomainEventQueuePtr queue)
+         virDomainEventFree(queue->events[i]);
+     }
+     VIR_FREE(queue->events);
++    queue->count = 0;
++}
++
++/**
++ * virDomainEventQueueFree:
++ * @queue: pointer to the queue
++ *
++ * Free the memory in the queue. We process this like a list here
++ */
++static void
++virDomainEventQueueFree(virDomainEventQueuePtr queue)
++{
++    if (!queue)
++        return;
++
++    virDomainEventQueueClear(queue);
+     VIR_FREE(queue);
+ }
+ 
+@@ -1559,6 +1575,7 @@ virDomainEventStateDeregister(virConnectPtr conn,
+         state->timer != -1) {
+         virEventRemoveTimeout(state->timer);
+         state->timer = -1;
++        virDomainEventQueueClear(state->queue);
+     }
+ 
+     virDomainEventStateUnlock(state);
+@@ -1596,6 +1613,7 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
+         state->timer != -1) {
+         virEventRemoveTimeout(state->timer);
+         state->timer = -1;
++        virDomainEventQueueClear(state->queue);
+     }
+ 
+     virDomainEventStateUnlock(state);
+-- 
+1.7.11.4
+



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]