[libvirt] [PATCH 29/38] virlog: Introduce an API mutex that serializes all setters

[Daniel P. Berrange]

On Thu, Mar 31, 2016 at 07:49:02PM +0200, Erik Skultety wrote:
> If the API isn't closed, a situation with 2 setters where one is about to
> define a set of outputs and the other is already defining a new one, may occur.
> By resetting all outputs, all file descriptors are closed. However, the other
> setter may still have a dangling reference to a file descriptor which may have
> already been closed.
> ---
>  src/libvirt_private.syms |  2 ++
>  src/util/virlog.c        | 15 +++++++++++++++
>  src/util/virlog.h        |  2 ++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index cc40b46..14047f5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1741,6 +1741,8 @@ virLockSpaceReleaseResourcesForOwner;
>  
>  
>  # util/virlog.h
> +virLogAPILock;
> +virLogAPIUnlock;
>  virLogDefineFilters;
>  virLogDefineOutputs;
>  virLogFilterListFree;
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 769dcec..6aa9c91 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -128,6 +128,21 @@ static void virLogOutputToFd(virLogSourcePtr src,
>                               void *data);
>  
>  
> +/* Setters need to be serialized on API entry point */
> +static virMutex virLogAPIMutex;
> +
> +void
> +virLogAPILock(void)
> +{
> +    virMutexLock(&virLogAPIMutex);
> +}
> +
> +void
> +virLogAPIUnlock(void)
> +{
> +    virMutexUnlock(&virLogAPIMutex);
> +}
> +
>  /*
>   * Logs accesses must be serialized though a mutex
>   */
> diff --git a/src/util/virlog.h b/src/util/virlog.h
> index 1c55a48..f5c0a4f 100644
> --- a/src/util/virlog.h
> +++ b/src/util/virlog.h
> @@ -203,6 +203,8 @@ extern void virLogFilterListFree(virLogFilterPtr *list, int count);
>   * Internal logging API
>   */
>  
> +extern void virLogAPILock(void);
> +extern void virLogAPIUnlock(void);
>  extern void virLogLock(void);
>  extern void virLogUnlock(void);

I'm not really seeing the reason why we need a second mutex instead of
just ensuring we acquire the existing mutex in the right places.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|