diff mbox

[2/2] Remove unnecessary CONFIG_EVENTFD preprocessor conditional to satisfy link

Message ID 1462236478-61645-3-git-send-email-chrisfriedt@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Christopher Friedt May 3, 2016, 12:47 a.m. UTC
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.

Note: there is nothing specific to eventfd inside and event_notifier_init() also fails unconditionally if CONFIG_EVENTFD is not defined.
Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
---
 util/event_notifier-posix.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Peter Maydell May 3, 2016, 1:01 a.m. UTC | #1
[ccing somebody else who ran into this, since I've figured out why.]

On 3 May 2016 at 01:47, Christopher Friedt <chrisfriedt@gmail.com> wrote:
> The file ivshmem.c unconditionally references event_notifier_init_fd()
> in util/event_notifier-posix.c, even if CONFIG_EVENTFD is not defined.

Yes, but ivshmem.c is only built if CONFIG_IVSHMEM, and
CONFIG_IVSHMEM is set (in default-configs/pci.mak) to CONFIG_EVENTFD.
So if CONFIG_EVENTFD is not defined then we should never build ivshmem.o.

The problem here is that you've run into a bug in QEMU's makefiles,
where a change in an included .mak file in default-configs fails
to cause the config-devices.mak file to be rebuilt. In commit
330b583 we changed pci.mak so that CONFIG_IVSHMEM is set to
CONFIG_EVENTFD rather than CONFIG_POSIX, so if your config-devices.mak
predates that commit then it will have incorrectly not been recreated
and so your build will still try to build ivshmem.c.

You can check whether this is true by looking at
(for instance) x86_64-softmmu/config-devices.mak in your build
tree: if it has a line "CONFIG_IVSHMEM=$(CONFIG_POSIX)" in it
then it's the out of date version.

You should be able to fix this by deleting */config-devices.mak
from your build tree (or by blowing away the build tree entirely
and recreating it.) Then try rebuilding -- ivshmem.c should not
be compiled.

We really must track down this dependency bug, it is very confusing
when it bites people.

thanks
-- PMM
Christopher Friedt May 3, 2016, 1:14 a.m. UTC | #2
On Mon, May 2, 2016 at 9:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> [ccing somebody else who ran into this, since I've figured out why.]
>
> On 3 May 2016 at 01:47, Christopher Friedt <chrisfriedt@gmail.com> wrote:
>> The file ivshmem.c unconditionally references event_notifier_init_fd()
>> in util/event_notifier-posix.c, even if CONFIG_EVENTFD is not defined.
>
> Yes, but ivshmem.c is only built if CONFIG_IVSHMEM, and
> CONFIG_IVSHMEM is set (in default-configs/pci.mak) to CONFIG_EVENTFD.
> So if CONFIG_EVENTFD is not defined then we should never build ivshmem.o.
>
> The problem here is that you've run into a bug in QEMU's makefiles,

That would explain things.

> where a change in an included .mak file in default-configs fails
> to cause the config-devices.mak file to be rebuilt. In commit
> 330b583 we changed pci.mak so that CONFIG_IVSHMEM is set to
> CONFIG_EVENTFD rather than CONFIG_POSIX, so if your config-devices.mak
> predates that commit then it will have incorrectly not been recreated
> and so your build will still try to build ivshmem.c.
>
> You can check whether this is true by looking at
> (for instance) x86_64-softmmu/config-devices.mak in your build
> tree: if it has a line "CONFIG_IVSHMEM=$(CONFIG_POSIX)" in it
> then it's the out of date version.

$ grep "CONFIG_IVSHMEM" x86_64-softmmu/config-devices.mak
CONFIG_IVSHMEM=$(CONFIG_EVENTFD)

I have been re-configuring on every build, just to eliminate that
potential error source.

> You should be able to fix this by deleting */config-devices.mak
> from your build tree (or by blowing away the build tree entirely
> and recreating it.) Then try rebuilding -- ivshmem.c should not
> be compiled.

I'll give that a go anyway.
Peter Maydell May 3, 2016, 1:18 a.m. UTC | #3
On 3 May 2016 at 02:14, Christopher Friedt <chrisfriedt@gmail.com> wrote:
> On Mon, May 2, 2016 at 9:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> [ccing somebody else who ran into this, since I've figured out why.]
>>
>> On 3 May 2016 at 01:47, Christopher Friedt <chrisfriedt@gmail.com> wrote:
>>> The file ivshmem.c unconditionally references event_notifier_init_fd()
>>> in util/event_notifier-posix.c, even if CONFIG_EVENTFD is not defined.
>>
>> Yes, but ivshmem.c is only built if CONFIG_IVSHMEM, and
>> CONFIG_IVSHMEM is set (in default-configs/pci.mak) to CONFIG_EVENTFD.
>> So if CONFIG_EVENTFD is not defined then we should never build ivshmem.o.
>>
>> The problem here is that you've run into a bug in QEMU's makefiles,
>
> That would explain things.
>
>> where a change in an included .mak file in default-configs fails
>> to cause the config-devices.mak file to be rebuilt. In commit
>> 330b583 we changed pci.mak so that CONFIG_IVSHMEM is set to
>> CONFIG_EVENTFD rather than CONFIG_POSIX, so if your config-devices.mak
>> predates that commit then it will have incorrectly not been recreated
>> and so your build will still try to build ivshmem.c.
>>
>> You can check whether this is true by looking at
>> (for instance) x86_64-softmmu/config-devices.mak in your build
>> tree: if it has a line "CONFIG_IVSHMEM=$(CONFIG_POSIX)" in it
>> then it's the out of date version.
>
> $ grep "CONFIG_IVSHMEM" x86_64-softmmu/config-devices.mak
> CONFIG_IVSHMEM=$(CONFIG_EVENTFD)

Hmm. (Is it set the same for every config-devices.mak for
every target you're trying to build?)

Next things to check: is CONFIG_EVENTFD definitely not set
in config-host.mak (it shouldn't be)? hw/misc/Makefile.objs
ought to say "obj-$(CONFIG_IVSHMEM) += ivshmem.o", and that
should mean that we don't try to build it...

thanks
-- PMM
Christopher Friedt May 3, 2016, 1:25 a.m. UTC | #4
On Mon, May 2, 2016 at 9:18 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 May 2016 at 02:14, Christopher Friedt <chrisfriedt@gmail.com> wrote:
>> On Mon, May 2, 2016 at 9:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> [ccing somebody else who ran into this, since I've figured out why.]
>>>
>>> On 3 May 2016 at 01:47, Christopher Friedt <chrisfriedt@gmail.com> wrote:
>>>> The file ivshmem.c unconditionally references event_notifier_init_fd()
>>>> in util/event_notifier-posix.c, even if CONFIG_EVENTFD is not defined.
>>>
>>> Yes, but ivshmem.c is only built if CONFIG_IVSHMEM, and
>>> CONFIG_IVSHMEM is set (in default-configs/pci.mak) to CONFIG_EVENTFD.
>>> So if CONFIG_EVENTFD is not defined then we should never build ivshmem.o.
>>>
>>> The problem here is that you've run into a bug in QEMU's makefiles,
>>
>> That would explain things.



> Hmm. (Is it set the same for every config-devices.mak for
> every target you're trying to build?)

Ack - too late! I manually removed all of the *-softmmu/ directories
after doing a make clean, because they were still hanging around.
Rebuilding worked fine without PATCH 2/2.

Thanks!
Peter Maydell May 3, 2016, 1:27 a.m. UTC | #5
On 3 May 2016 at 02:25, Christopher Friedt <chrisfriedt@gmail.com> wrote:
> Ack - too late! I manually removed all of the *-softmmu/ directories
> after doing a make clean, because they were still hanging around.
> Rebuilding worked fine without PATCH 2/2.

Oh well. If blowing away the build dirs fixed it then it seems
pretty likely that the makefile bug is what you were running into.

thanks
-- PMM
diff mbox

Patch

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>
 #endif
 
-#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;
 }
-#endif
 
 int event_notifier_init(EventNotifier *e, int active)
 {