[Qemu-devel] [PATCH]: Fix conditional compilation for OpenBSD

[Qemu-devel] [PATCH]: Fix conditional compilation for OpenBSD

[Michael W. Bombardieri]

Hi,

This morning I tried building Qemu 1.4.0 on my
Lenovo ThinkPad T61 running OpenBSD/i386 5.1.
The thread code didn't build due to a missing
library function. Looking at the code, #if
statements for NetBSD should also be followed
for OpenBSD. Using the following patch I was
able to build Qemu using clang 3.0 (gcc 4.2.1
ran out of memory). Please consider committing
this patch.

- Michael


diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
index 0f30dcc..772d925 100644
--- a/include/qemu/thread-posix.h
+++ b/include/qemu/thread-posix.h
@@ -12,7 +12,7 @@ struct QemuCond {
 };
 
 struct QemuSemaphore {
-#if defined(__APPLE__) || defined(__NetBSD__)
+#if defined(__APPLE__) || defined(__NetBSD__) || defined(__OpenBSD__)
     pthread_mutex_t lock;
     pthread_cond_t cond;
     int count;
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 4489abf..fa8a3d8 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -122,7 +122,7 @@ void qemu_sem_init(QemuSemaphore *sem, int init)
 {
     int rc;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#if defined(__APPLE__) || defined(__NetBSD__) || defined(__OpenBSD__)
     rc = pthread_mutex_init(&sem->lock, NULL);
     if (rc != 0) {
         error_exit(rc, __func__);
@@ -147,7 +147,7 @@ void qemu_sem_destroy(QemuSemaphore *sem)
 {
     int rc;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#if defined(__APPLE__) || defined(__NetBSD__) || defined(__OpenBSD__)
     rc = pthread_cond_destroy(&sem->cond);
     if (rc < 0) {
         error_exit(rc, __func__);
@@ -168,7 +168,7 @@ void qemu_sem_post(QemuSemaphore *sem)
 {
     int rc;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#if defined(__APPLE__) || defined(__NetBSD__) || defined(__OpenBSD__)
     pthread_mutex_lock(&sem->lock);
     if (sem->count == INT_MAX) {
         rc = EINVAL;
@@ -206,7 +206,7 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
     int rc;
     struct timespec ts;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#if defined(__APPLE__) || defined(__NetBSD__) || defined(__OpenBSD__)
     compute_abs_deadline(&ts, ms);
     pthread_mutex_lock(&sem->lock);
     --sem->count;
@@ -249,7 +249,7 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
 
 void qemu_sem_wait(QemuSemaphore *sem)
 {
-#if defined(__APPLE__) || defined(__NetBSD__)
+#if defined(__APPLE__) || defined(__NetBSD__) || defined(__OpenBSD__)
     pthread_mutex_lock(&sem->lock);
     --sem->count;
     while (sem->count < 0) {

Re: [Qemu-devel] [PATCH]: Fix conditional compilation for OpenBSD

[Peter Maydell]

On 31 March 2013 00:32, Michael W. Bombardieri <mb@ii.net> wrote:
> This morning I tried building Qemu 1.4.0 on my
> Lenovo ThinkPad T61 running OpenBSD/i386 5.1.
> The thread code didn't build due to a missing
> library function. Looking at the code, #if
> statements for NetBSD should also be followed
> for OpenBSD. Using the following patch I was
> able to build Qemu using clang 3.0 (gcc 4.2.1
> ran out of memory). Please consider committing
> this patch.

Hi Michael; thanks for the patch. However I think we should
fix this by having a configure check for sem_timedwait [or
whatever the functions we need are], rather than by piling
up another OS ifdef check.

(If you want to have a go at that it would probably be
useful to read the wiki.qemu.org page on submitting
patches; in particular without a Signed-off-by: line
we can't apply a patch at all.)

-- PMM

Re: [Qemu-devel] [PATCH]: Fix conditional compilation for OpenBSD

[Michael W. Bombardieri]

On Sun, Mar 31, 2013 at 12:48:04AM +0000, Peter Maydell wrote:
> 
> Hi Michael; thanks for the patch. However I think we should
> fix this by having a configure check for sem_timedwait [or
> whatever the functions we need are], rather than by piling
> up another OS ifdef check.
> 
> (If you want to have a go at that it would probably be
> useful to read the wiki.qemu.org page on submitting
> patches; in particular without a Signed-off-by: line
> we can't apply a patch at all.)
> 
> -- PMM

What about this?

Re-use the exting the pthread checker code in 'configure', define HAS_PTHREAD
and use this instead of OS specific #if checks. I have built this on

1. Ubuntu 10.04.3 LTS (Linux 2.6.32-36-generic i686)
2. OpenBSD 5.1 (OpenBSD 5.1 GENERIC.MP#188 i386)
3. OpenBSD 5.1 (OpenBSD 5.1 GENERIC#243 amd64)

- Michael


diff --git a/configure b/configure
index f2af714..2e521e2 100755
--- a/configure
+++ b/configure
@@ -2296,6 +2296,10 @@ else
   done
 fi
 
+if [ $pthread = 'yes' ]; then
+  QEMU_CFLAGS="-DHAS_PTHREAD $QEMU_CFLAGS"
+fi
+
 if test "$mingw32" != yes -a "$pthread" = no; then
   echo
   echo "Error: pthread check failed"
diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
index 0f30dcc..f547f29 100644
--- a/include/qemu/thread-posix.h
+++ b/include/qemu/thread-posix.h
@@ -12,7 +12,7 @@ struct QemuCond {
 };
 
 struct QemuSemaphore {
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     pthread_mutex_t lock;
     pthread_cond_t cond;
     int count;
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 4489abf..edf4715 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -122,7 +122,7 @@ void qemu_sem_init(QemuSemaphore *sem, int init)
 {
     int rc;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     rc = pthread_mutex_init(&sem->lock, NULL);
     if (rc != 0) {
         error_exit(rc, __func__);
@@ -147,7 +147,7 @@ void qemu_sem_destroy(QemuSemaphore *sem)
 {
     int rc;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     rc = pthread_cond_destroy(&sem->cond);
     if (rc < 0) {
         error_exit(rc, __func__);
@@ -168,7 +168,7 @@ void qemu_sem_post(QemuSemaphore *sem)
 {
     int rc;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     pthread_mutex_lock(&sem->lock);
     if (sem->count == INT_MAX) {
         rc = EINVAL;
@@ -206,7 +206,7 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
     int rc;
     struct timespec ts;
 
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     compute_abs_deadline(&ts, ms);
     pthread_mutex_lock(&sem->lock);
     --sem->count;
@@ -249,7 +249,7 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
 
 void qemu_sem_wait(QemuSemaphore *sem)
 {
-#if defined(__APPLE__) || defined(__NetBSD__)
+#ifdef HAS_PTHREAD
     pthread_mutex_lock(&sem->lock);
     --sem->count;
     while (sem->count < 0) {

Re: [Qemu-devel] [PATCH]: Fix conditional compilation for OpenBSD

[Peter Maydell]

On 31 March 2013 13:46, Michael W. Bombardieri <mb@ii.net> wrote:
> What about this?
>
> Re-use the exting the pthread checker code in 'configure', define HAS_PTHREAD
> and use this instead of OS specific #if checks. I have built this on
>
> 1. Ubuntu 10.04.3 LTS (Linux 2.6.32-36-generic i686)
> 2. OpenBSD 5.1 (OpenBSD 5.1 GENERIC.MP#188 i386)
> 3. OpenBSD 5.1 (OpenBSD 5.1 GENERIC#243 amd64)

This doesn't look right, because it means we'll use the fallback
code even for OSes like Linux which do have the ability to use
the (preferable) semaphore code. You want a config check for
sem_timedwait, which will succeed on Linux and fail on the BSDs,
and then instead of #if defined(various BSDs) you have
#ifndef CONFIG_SEM_TIMEDWAIT.

> +if [ $pthread = 'yes' ]; then
> +  QEMU_CFLAGS="-DHAS_PTHREAD $QEMU_CFLAGS"
> +fi
> +

Have a look in configure at how we handle the preadv()
check as an example. There's a little test program
which is compiled to see if the function exists, and
we set a shell variable. Then later on we write CONFIG_PREADV=y
to the config-host.mak file. This avoids enormous compiler
command lines for every config check.

Also you still aren't providing a Signed-off-by: line for
your patch, and it would be nice if you could submit
it in the git format-patch format that makes it trivial
to apply.

thanks
-- PMM