[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/2] [RESENT-INLINE] Remove unnec
Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/2] [RESENT-INLINE] Remove unnecessary CONFIG_EVENTFD preprocessor conditional to satisfy link
Tue, 03 May 2016 08:59:55 +0200
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)
Christopher Friedt <address@hidden> writes:
> The file ivshmem.c unconditionally references event_notifier_init_fd() in
> util/event_notifier-posix.c, even if CONFIG_EVENTFD is not defined. On
> platforms where CONFIG_POSIX is defined, but CONFIG_EVENTFD is not defined,
> that results in an undefined symbol referenced from ivshmem.c and the link
> fails. That applies to Mac OS X, but possibly other BSD-based distros.
ivshmem.c cannot work without CONFIG_EVENTFD, and therefore is not
compiled without it.
A link error for event_notifier_init_fd() from ivshmem.o indicates you
got CONFIG_EVENTFD=y for make (since ivshmem.o gets linked), but
!defined(CONFIG_EVENTFD) for C (or else event_notifier_init_fd() would
exist). Your build tree is messed up, or the makefiles are broken. Try
starting over with a fresh build tree.
> Note: there is nothing specific to eventfd inside and event_notifier_init()
> also fails unconditionally if CONFIG_EVENTFD is not defined.
I'm afraid you're misreading the code.
event_notifier_init() has a working fallback: if the host doesn't
support eventfd (!defined(CONFIG_EVENTFD) or eventfd() fails with
ENOSYS), then we emulate eventfd with a pair of pipes. Possible only
because pipes have special properties.
To support the emulation, we have two file descriptors, @rfd for reading
and @wfd for writing. Design invariant: if they are the same file
descriptor, it must be a proper eventfd, and if they are different, they
must be a pair of pipes. The only way to establish the invariant with
event_notifier_init_fd() is passing it an eventfd, as the function's
contract says. Without CONFIG_EVENTFD, there is no way to use the
function correctly, so defining it would be nothing but a trap.
See also commit 330b583, which added the ifdef.
> Signed-off-by: Christopher Friedt <address@hidden>
> util/event_notifier-posix.c | 2 --
> 1 file changed, 2 deletions(-)
> diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
> index c1f0d79..c9bb34d 100644
> --- a/util/event_notifier-posix.c
> +++ b/util/event_notifier-posix.c
> @@ -21,7 +21,6 @@
> #include <sys/eventfd.h>
> -#ifdef CONFIG_EVENTFD
> * Initialize @e with existing file descriptor @fd.
> * @fd must be a genuine eventfd object, emulation with pipe won't do.
> @@ -31,7 +30,6 @@ void event_notifier_init_fd(EventNotifier *e, int fd)
> e->rfd = fd;
> e->wfd = fd;
> int event_notifier_init(EventNotifier *e, int active)