[libvirt] [PATCH RFC 8/8] add qemu cache mutex
Lee Schermerhorn
Lee.Schermerhorn at hp.com
Mon Mar 12 14:16:29 UTC 2012
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);
> > +}
> >
More information about the libvir-list
mailing list