On Mon, 2012-03-12 at 14:00 +0100, Michal Privoznik wrote:
On 11.03.2012 19:56, Lee Schermerhorn wrote:
Add a mutex for access to the qemu emulator cache. Not clear that this is actually needed -- driver should be locked across calls [?]. The patch can be dropped if not needed. --- src/qemu/qemu_capabilities.c | 18 +++++++++++++++++- src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_driver.c | 3 +++ 3 files changed, 22 insertions(+), 1 deletion(-)
Index: libvirt-0.9.10/src/qemu/qemu_capabilities.c =================================================================== --- libvirt-0.9.10.orig/src/qemu/qemu_capabilities.c +++ libvirt-0.9.10/src/qemu/qemu_capabilities.c @@ -27,6 +27,7 @@ #include "memory.h" #include "logging.h" #include "virterror_internal.h" +#include "threads.h" #include "util.h" #include "virfile.h" #include "nodeinfo.h" @@ -180,6 +181,11 @@ enum qemuCapsProbes { QEMU_PROBE_SIZE };
+/* + * Use static initializer for tests + */ +static virMutex qemuEmulatorCacheMutex = { PTHREAD_MUTEX_INITIALIZER };
This is not allowed in our code as we build with win32 threads which initialize mutexes completely different. Why do you want to initialize it here anyway ...
Thanks. I didn't know that. As the comment says, I added it for the internal tests. It appeared to me that they don't go through the driver init code where I inserted the call to to the init function below. The tests were hanging at the first attempt to acquire the mutex. It could have been a defect in my patches at that time [that may still be there?]. When I inserted the static initializer, the tests passed. That was back on the 0.8.8 ubuntu natty code base. I'll pull it out and see if they pass w/o it, now that the tests all seem to pass otherwise. They certainly pass w/o this patch applied, but they're all single threaded, right? Bigger question is: is the mutex actually needed at all? I.e., can I assume that the driver is always locked -- in practice, not necessarily for the tests -- when the probes are called? Lee
+ typedef struct _qemuEmulatorCache qemuEmulatorCache; typedef qemuEmulatorCache* qemuEmulatorCachePtr; struct _qemuEmulatorCache { @@ -206,9 +212,17 @@ qemuEmulatorCachedInfoGet(enum qemuCapsP const char *binary, const char *arch);
+int +qemuCapsCacheInit(void) +{ + return virMutexInit(&qemuEmulatorCacheMutex); +} +
if you have created this function?
static void qemuEmulatorCachedInfoRelease(qemuEmulatorCachePtr emulator ATTRIBUTE_UNUSED) -{ } +{ + virMutexUnlock(&qemuEmulatorCacheMutex); +}