On Thu, 25 Jun 2020 at 19:37, David CARLIER <devne...@gmail.com> wrote:
>
> From 25adbdcdc17ef51a41759f16576901338ed8a469 Mon Sep 17 00:00:00 2001
> From: David Carlier <devne...@gmail.com>
> Date: Thu, 25 Jun 2020 19:32:42 +0000
> Subject: [PATCH] build: haiku system build fix
>
> Most of missing features resides in the bsd library.
> Also defining constant equivalence.
>
> Signed-off-by: David Carlier <devne...@gmail.com>
> --
Hi; thanks for this patch. I have some review comments
below, mostly relating to better ways to implement
the compatibility fixes.
> diff --git a/os-posix.c b/os-posix.c
> index 3cd52e1e70..ca07b7702d 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -42,6 +42,8 @@
> #include <sys/prctl.h>
> #endif
>
> +#include <sys/mman.h>
I think this either should not be necessary, or if it's
fixing something the fix should be elsewhere. For
POSIX-compatible host OSes, sys/mman.h is included
via include/sysemu/os-posix.h which in turn is pulled in
by include/qemu/osdep.h, which is always included by every
C file in QEMU. I think looking at configure that Haiku
host builds should set CONFIG_POSIX, so I would expect
that sys/mman.h has already been included here.
> +
> /*
> * Must set all three of these at once.
> * Legal combinations are unset by name by uid
> @@ -339,10 +341,12 @@ int os_mlock(void)
> {
> int ret = 0;
>
> +#if !defined(__HAIKU__)
> ret = mlockall(MCL_CURRENT | MCL_FUTURE);
> if (ret < 0) {
> error_report("mlockall: %s", strerror(errno));
> }
>
> +#endif
If Haiku doesn't support an mlockall() equivalent, then the
correct implementation of os_mlock() for it is to return
-ENOSYS -- compare the Win32 stub version in
include/sysemu/os-win32.h.
Also, in general we prefer configure to do feature tests,
rather than for the source code to have OS-specific ifdefs.
So here you'd want a configure test for "does the system
provide mlockall()?", and then the implementation would
be guarded with CONFIG_MLOCKALL.
> return ret;
> }
> diff --git a/util/compatfd.c b/util/compatfd.c
> index c296f55d14..ee47dd8089 100644
> --- a/util/compatfd.c
> +++ b/util/compatfd.c
> @@ -16,7 +16,9 @@
> #include "qemu/osdep.h"
> #include "qemu/thread.h"
>
> +#if defined(CONFIG_SIGNALFD)
> #include <sys/syscall.h>
> +#endif
>
> struct sigfd_compat_info
> {
> diff --git a/util/drm.c b/util/drm.c
> index a23ff24538..8540578c09 100644
> --- a/util/drm.c
> +++ b/util/drm.c
> @@ -38,9 +38,11 @@ int qemu_drm_rendernode_open(const char *rendernode)
>
> fd = -1;
> while ((e = readdir(dir))) {
> +#if !defined(__HAIKU__)
> if (e->d_type != DT_CHR) {
> continue;
> }
> +#endif
Does Haiku really provide /dev/dri ? If not then rather than
making this code which can't be used on this host OS compile
it would probably be better to just arrange that it doesn't
get compiled. I suspect that the answer here is that the
$(CONFIG_POSIX) guard on compiling drm.o in util/Makefile.objs
should really be a $(CONFIG_LINUX).
>
> if (strncmp(e->d_name, "renderD", 7)) {
> continue;
> diff --git a/util/main-loop.c b/util/main-loop.c
> index eda63fe4e0..1ce3081ced 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -85,6 +85,9 @@ static int qemu_signal_init(Error **errp)
> * by sigwait() in the signal thread. Otherwise, the cpu thread will
> * not catch it reliably.
> */
> +#ifndef SIGIO
> +#define SIGIO SIGPOLL
> +#endif
If Haiku's SIGPOLL really behaves like Linux's SIGIO then
include/qemu/osdep.h is the best place to put this kind
of fixup, so that all files get the benefit of it.
> sigemptyset(&set);
> sigaddset(&set, SIG_IPI);
> sigaddset(&set, SIGIO);
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 39ddc77c85..a18d90a68a 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -38,7 +38,12 @@
> #include "qemu/sockets.h"
> #include "qemu/thread.h"
> #include <libgen.h>
> +#if !defined(__HAIKU__)
> #include <sys/signal.h>
> +#else
> +#include <kernel/image.h>
> +#include <signal.h>
> +#endif
> #include "qemu/cutils.h"
>
> #ifdef CONFIG_LINUX
> @@ -390,6 +395,21 @@ void qemu_init_exec_dir(const char *argv0)
> }
> }
> }
> +#elif defined(__HAIKU__)
> + {
> + image_info ii;
> + int32_t c = 0;
> +
> + *buf = '\0';
> + while (get_next_image_info(0, &c, &ii) == B_OK) {
> + if (ii.type == B_APP_IMAGE) {
> + strncpy(buf, ii.name, sizeof(buf));
> + buf[sizeof(buf) - 1] = '\0';
> + p = buf;
> + break;
> + }
> + }
Your indentation here has got messed up.
> + }
> #endif
> /* If we don't have any way of figuring out the actual executable
> location then try argv[0]. */
> diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
> index 2e8b43bdf5..c29fc2cbf9 100644
> --- a/util/qemu-openpty.c
> +++ b/util/qemu-openpty.c
> @@ -35,7 +35,7 @@
> #include "qemu/osdep.h"
> #include "qemu-common.h"
>
> -#if defined(__GLIBC__)
> +#if defined(__GLIBC__) || defined(__HAIKU__)
> # include <pty.h>
> #elif defined CONFIG_BSD
> # include <termios.h>
This would be neater if we dropped the GLIBC/HAIKU check
in favour of a configure "is openpty() in pty.h?" test.
thanks
-- PMM
