Re: [Qemu-devel] [PATCH] ivshmem-server: ivshmem-clean: Install only when eventfd() is available

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Hi
>
> On Fri, May 26, 2017 at 9:49 AM Kamil Rytarowski <address@hidden> wrote:
>
>> Currently ivshmem requires eventfd() which is Linux specific.
>> Do not and build it unconditionally on every Linux/BSD/Solaris.
>>
>>
> I think it should be able to use pipe fallback from event_notifier_init() .

Not easily.  ivshmem fundamentally needs event_notifier_init_fd(), which
is CONFIG_EVENTFD:

commit 330b58368ca16c31efdadcf8263f7f903546af50
Author: Markus Armbruster <address@hidden>
Date:   Tue Mar 15 19:34:20 2016 +0100

    event_notifier: Make event_notifier_init_fd() #ifdef CONFIG_EVENTFD
    
    Event notifiers are designed for eventfd(2).  They can fall back to
    pipes, but according to Paolo, event_notifier_init_fd() really
    requires the real thing, and should therefore be under #ifdef
    CONFIG_EVENTFD.  Do that.
    
    Its only user is ivshmem, which is currently CONFIG_POSIX.  Narrow it
    to CONFIG_EVENTFD.
    
    Cc: Paolo Bonzini <address@hidden>
    Signed-off-by: Markus Armbruster <address@hidden>
    Reviewed-by: Paolo Bonzini <address@hidden>
    Message-Id: <address@hidden>

>> This patch indirectly fixes build failure on NetBSD, where these tools
>> additionally require -lrl for shm_open(3). In future there should be

Do you mean -lrt?

>> added support for NetBSD and the linking addressed appropriately.
>>
>> Signed-off-by: Kamil Rytarowski <address@hidden>
>>
> ---
>>  configure | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/configure b/configure
>> index 1a5ee4b909..84d8660354 100755
>> --- a/configure
>> +++ b/configure
>> @@ -4928,6 +4928,8 @@ if test "$want_tools" = "yes" ; then
>>    tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
>>    if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
>>      tools="qemu-nbd\$(EXESUF) $tools"
>> +  fi
>> +  if [ "$eventfd" != "no" ]; then

"$eventfd" = "yes" would be clearer, wouldn't it?

>>      tools="ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools"
>>    fi
>>  fi

With the -l in the commit message double-checked:
Reviewed-by: Markus Armbruster <address@hidden>

Do you think squashing in the appended diff would make sense?


diff --git a/contrib/ivshmem-client/Makefile.objs 
b/contrib/ivshmem-client/Makefile.objs
index bfab2d2..13d8640 100644
--- a/contrib/ivshmem-client/Makefile.objs
+++ b/contrib/ivshmem-client/Makefile.objs
@@ -1 +1 @@
-ivshmem-client-obj-y = ivshmem-client.o main.o
+ivshmem-client-obj-$(CONFIG_IVSHMEM) = ivshmem-client.o main.o
diff --git a/contrib/ivshmem-server/Makefile.objs 
b/contrib/ivshmem-server/Makefile.objs
index c060dd3..d9469fd 100644
--- a/contrib/ivshmem-server/Makefile.objs
+++ b/contrib/ivshmem-server/Makefile.objs
@@ -1 +1 @@
-ivshmem-server-obj-y = ivshmem-server.o main.o
+ivshmem-server-obj-$(CONFIG_IVSHMEM) = ivshmem-server.o main.o