On 10/10/2011 05:45 AM, Michal Privoznik wrote:
Currently, push& pop from event queue (both server& client side)
rely on lock from higher levels, e.g. on driver lock (qemu),
private_data (remote), ...; This alone is not sufficient as not
every function that interacts with this queue can/does lock,
esp. in client where we have a different approach, "passing
the buck".
Therefore we need a separate lock just to protect event queue.
For more info see:
https://bugzilla.redhat.com/show_bug.cgi?id=743817
---
src/conf/domain_event.c | 65 +++++++++++++++++++++++++++++++++++++++++------
src/conf/domain_event.h | 1 +
2 files changed, 58 insertions(+), 8 deletions(-)
I'd feel more comfortable if Dan also reviews, but I'll give it a shot.
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 3189346..6cc8168 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -547,6 +547,24 @@ virDomainEventQueuePtr virDomainEventQueueNew(void)
return ret;
}
+static void
+virDomainEventStateLock(virDomainEventStatePtr state)
+{
+ if (!state)
+ return;
+
+ virMutexLock(&state->lock);
Why do these have early returns, implying NULL state is okay...
@@ -1161,18 +1188,26 @@ void
virDomainEventStateQueue(virDomainEventStatePtr state,
virDomainEventPtr event)
{
+ int count;
+
if (state->timer< 0) {
virDomainEventFree(event);
return;
}
+ virDomainEventStateLock(state);
+
if (virDomainEventQueuePush(state->queue, event)< 0) {
VIR_DEBUG("Error adding event to queue");
virDomainEventFree(event);
}
- if (state->queue->count == 1)
+ count = state->queue->count;
+ virDomainEventStateUnlock(state);
...even though uses like this blindly assume that state is non-NULL?
One of the two places is wrong (I'd argue that the lock/unlock functions
should be ATTRIBUTE_NONNULL(1), since it looks like all callers avoid
passing NULL).
Beyond that, looks okay to me.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list