[libvirt] [PATCH 2/5] Remove windows thread implementation in favour of pthreads

[Laine Stump]

On 01/23/2014 02:37 PM, Daniel P. Berrange wrote:
> There are a number of pthreads impls available on Win32
> these days, in particular the mingw64 project has a good
> impl. Delete the native windows thread implementation and
> rely on using pthreads everywhere.
>
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  configure.ac                |  15 +-
>  src/Makefile.am             |   4 -
>  src/util/virthread.c        | 291 +++++++++++++++++++++++++++++++-
>  src/util/virthread.h        |  41 ++++-
>  src/util/virthreadpthread.c | 309 ----------------------------------
>  src/util/virthreadpthread.h |  53 ------
>  src/util/virthreadwin32.c   | 396 --------------------------------------------
>  src/util/virthreadwin32.h   |  53 ------
>  8 files changed, 329 insertions(+), 833 deletions(-)
>  delete mode 100644 src/util/virthreadpthread.c
>  delete mode 100644 src/util/virthreadpthread.h
>  delete mode 100644 src/util/virthreadwin32.c
>  delete mode 100644 src/util/virthreadwin32.h
>
> diff --git a/configure.ac b/configure.ac
> index 3a70375..168eb27 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -270,12 +270,21 @@ AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \
>    posix_memalign prlimit regexec sched_getaffinity setgroups setns \
>    setrlimit symlink sysctlbyname])
>  
> -dnl Availability of pthread functions (if missing, win32 threading is
> -dnl assumed).  Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE.
> -dnl LIB_PTHREAD and LIBMULTITHREAD were set during gl_INIT by gnulib.
> +dnl Availability of pthread functions. Because of $LIB_PTHREAD, we
> +dnl cannot use AC_CHECK_FUNCS_ONCE. LIB_PTHREAD and LIBMULTITHREAD
> +dnl were set during gl_INIT by gnulib.
>  old_LIBS=$LIBS
>  LIBS="$LIBS $LIB_PTHREAD $LIBMULTITHREAD"
> +
> +pthread_found=yes
>  AC_CHECK_FUNCS([pthread_mutexattr_init])
> +AC_CHECK_HEADER([pthread.h],,[pthread_found=no])
> +
> +if test "$ac_cv_func_pthread_mutexattr_init:$pthread_found" != "yes:yes"
> +then
> +  AC_MSG_ERROR([A pthreads impl is required for building libvirt])
> +fi
> +
>  dnl At least mingw64-winpthreads #defines pthread_sigmask to 0,
>  dnl which in turn causes compilation to complain about unused variables.
>  dnl Expose this broken implementation, so we can work around it.
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 8f77658..2298fdb 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -141,8 +141,6 @@ UTIL_SOURCES =							\
>  		util/virsysinfo.c util/virsysinfo.h		\
>  		util/virsystemd.c util/virsystemd.h		\
>  		util/virthread.c util/virthread.h		\
> -		util/virthreadpthread.h				\
> -		util/virthreadwin32.h				\
>  		util/virthreadpool.c util/virthreadpool.h	\
>  		util/virtime.h util/virtime.c			\
>  		util/virtpm.h util/virtpm.c			\
> @@ -165,8 +163,6 @@ util/virkeymaps.h: $(srcdir)/util/keymaps.csv	\
>  	$(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py \
>  	  <$(srcdir)/util/keymaps.csv >$(srcdir)/util/virkeymaps.h
>  
> -EXTRA_DIST += util/virthreadpthread.c util/virthreadwin32.c
> -
>  # Internal generic driver infrastructure
>  NODE_INFO_SOURCES = nodeinfo.h nodeinfo.c
>  DATATYPES_SOURCES = datatypes.h datatypes.c
> diff --git a/src/util/virthread.c b/src/util/virthread.c
> index dd1768e..b60fb4a 100644
> --- a/src/util/virthread.c
> +++ b/src/util/virthread.c
> @@ -23,12 +23,289 @@
>  
>  #include "virthread.h"
>  
> -/* On mingw, we prefer native threading over the sometimes-broken
> - * pthreads-win32 library wrapper.  */
> -#ifdef WIN32
> -# include "virthreadwin32.c"
> -#elif defined HAVE_PTHREAD_MUTEXATTR_INIT
> -# include "virthreadpthread.c"
> +#include <unistd.h>
> +#include <inttypes.h>
> +#if HAVE_SYS_SYSCALL_H
> +# include <sys/syscall.h>
> +#endif
> +
> +#include "viralloc.h"
> +
> +
> +/* Nothing special required for pthreads */
> +int virThreadInitialize(void)
> +{
> +    return 0;
> +}
> +
> +void virThreadOnExit(void)
> +{
> +}
> +
> +int virOnce(virOnceControlPtr once, virOnceFunc init)
> +{
> +    return pthread_once(&once->once, init);
> +}
> +
> +
> +int virMutexInit(virMutexPtr m)
> +{
> +    int ret;
> +    pthread_mutexattr_t attr;
> +    pthread_mutexattr_init(&attr);
> +    pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_NORMAL);
> +    ret = pthread_mutex_init(&m->lock, &attr);
> +    pthread_mutexattr_destroy(&attr);
> +    if (ret != 0) {
> +        errno = ret;
> +        return -1;
> +    }
> +    return 0;
> +}

You just moved the contents of virthreadpthread.c into virthread.c,
right? Is there maybe some way to convince git to make a shorter diff?
(I did something similar in netcf - moved 90% of one file into another,
and renamed the original, and it properly figured out to rename the
original into the new 90%, and create the one I thought I was renaming
from scratch). Not terribly important though, since we all know what
really happened here :-)


> diff --git a/src/util/virthread.h b/src/util/virthread.h
> index 7015d60..94476ee 100644
> --- a/src/util/virthread.h
> +++ b/src/util/virthread.h
> @@ -25,24 +25,57 @@
>  # include "internal.h"
>  # include "virerror.h"
>  
> +# include <pthread.h>
> +
[...]
> +
>  typedef struct virOnceControl virOnceControl;
>  typedef virOnceControl *virOnceControlPtr;
>  
> +struct virOnceControl {
> +    pthread_once_t once;
> +};
> +
> +
> +#define VIR_ONCE_CONTROL_INITIALIZER \

This is now improperly indented.

Oh, and also I just noticed the copyright date hasn't been updated.

I guess as long as it builds on Windows/ming64 I give it a functional
ACK. I don't know enough about problems in "certain" Windows pthreads
implementations to comfortably ACK the removal of the Windows native
threads code, though.