Re: [Qemu-devel] [PATCH] ivshmem-server: Detect and use if there is required -lrt linking

[Kamil Rytarowski]

On 22.05.2017 08:28, Markus Armbruster wrote:
> Kamil Rytarowski <address@hidden> writes:
> 
>> Hello,
>>
>> Excuse me for delay, I missed this mail.
> 
> No problem.
> 
>> Please see in-line.
>>
>> On 17.05.2017 09:28, Markus Armbruster wrote:
>>> Kamil Rytarowski <address@hidden> writes:
>>>
>>>> ivshmem-server makes use of the POSIX shared memory object interfaces.
>>>> This library is provided on NetBSD in -lrt (POSIX Real-time Library).
>>>> Add ./configure check if there is needed -lrt linking for shm_open()
>>>> and if so use it. Introduce new configure generated variable LIBS_SHMLIB.
>>>>
>>>> This fixes build issue on NetBSD.
>>>>
>>>> Signed-off-by: Kamil Rytarowski <address@hidden>
>>>> ---
>>>>  Makefile  |  1 +
>>>>  configure | 20 ++++++++++++++++++++
>>>>  2 files changed, 21 insertions(+)
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index 31d41a7eae..3248cb53d7 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -473,6 +473,7 @@ ivshmem-client$(EXESUF): $(ivshmem-client-obj-y) 
>>>> $(COMMON_LDADDS)
>>>>    $(call LINK, $^)
>>>>  ivshmem-server$(EXESUF): $(ivshmem-server-obj-y) $(COMMON_LDADDS)
>>>>    $(call LINK, $^)
>>>> +ivshmem-server$(EXESUF): LIBS += $(LIBS_SHMLIB)
>>>>  
>>>>  module_block.h: $(SRC_PATH)/scripts/modules/module_block.py 
>>>> config-host.mak
>>>>    $(call quiet-command,$(PYTHON) $< $@ \
>>>> diff --git a/configure b/configure
>>>> index 7c020c076b..50c3aee746 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -179,6 +179,7 @@ audio_pt_int=""
>>>>  audio_win_int=""
>>>>  cc_i386=i386-pc-linux-gnu-gcc
>>>>  libs_qga=""
>>>> +libs_shmlib=""
>>>>  debug_info="yes"
>>>>  stack_protector=""
>>>>  
>>>> @@ -4133,6 +4134,24 @@ elif compile_prog "" "$pthread_lib -lrt" ; then
>>>>    libs_qga="$libs_qga -lrt"
>>>>  fi
>>>>  
>>>> +##########################################
>>>> +# Do we need librt for shm_open()
>>>> +cat > $TMPC <<EOF
>>>> +#include <sys/mman.h>
>>>> +#include <sys/stat.h>
>>>> +#include <fcntl.h>
>>>> +#include <stddef.h>
>>>> +int main(void) {
>>>> +  return shm_open(NULL, O_RDWR, 0644);
>>>> +}
>>>> +EOF
>>>> +
>>>> +if compile_prog "" "" ; then
>>>> +  :
>>>> +elif compile_prog "" "-lrt" ; then
>>>> +  libs_shmlib="$libs_shmlib -lrt"
>>>> +fi
>>>> +
>>>>  if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a 
>>>> \
>>>>          "$aix" != "yes" -a "$haiku" != "yes" ; then
>>>>      libs_softmmu="-lutil $libs_softmmu"
>>>> @@ -5949,6 +5968,7 @@ echo "EXESUF=$EXESUF" >> $config_host_mak
>>>>  echo "DSOSUF=$DSOSUF" >> $config_host_mak
>>>>  echo "LDFLAGS_SHARED=$LDFLAGS_SHARED" >> $config_host_mak
>>>>  echo "LIBS_QGA+=$libs_qga" >> $config_host_mak
>>>> +echo "LIBS_SHMLIB+=$libs_shmlib" >> $config_host_mak
>>>>  echo "TASN1_LIBS=$tasn1_libs" >> $config_host_mak
>>>>  echo "TASN1_CFLAGS=$tasn1_cflags" >> $config_host_mak
>>>>  echo "POD2MAN=$POD2MAN" >> $config_host_mak
>>>
>>> We already have a test for -lrt.
>>
>> Correct.
>>
>>>  It looks for timer_create() and
>>> clock_gettime().
>>
>>
>> timer_create(2) and clock_settime(2) are in libc on NetBSD.
>>
>>>  If we need -lrt,
>>
>> We need it just for shm_open(3).
>>
>>> we add it to LIBS and to LIBS_QGA.
>>> The latter because we don't use LIBS for qemu-ga:
>>>
>>>     qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
>>>
>>> This patch adds a second test for -lrt, to look for shm_open().  It adds
>>> -lrt to new variable LIBS_SHMLIB, which gets used only for
>>> ivshmem-server:
>>>
>>>     ivshmem-server$(EXESUF): LIBS += $(LIBS_SHMLIB)
>>>
>>> Note that ivshmem-server already uses LIBS.
>>>
>>> Shouldn't we instead widen the existing test for -lrt to cover
>>> shm_open()?
>>>
>>
>> I will prepare patch in the requested way. I don't have preference.
>>
>>> Can you confirm that the existing test does not add -lrt to LIBS on
>>> NetBSD?
>>>
>>
>> config-host.mak:LIBS+=-lm -L/usr/pkg/lib -lgthread-2.0 -pthread
>> -Wl,-R/usr/pkg/lib -lglib-2.0 -lintl   -lz
>> config-host.mak:LIBS_QGA+=-lm -L/usr/pkg/lib -lgthread-2.0 -pthread
>> -Wl,-R/usr/pkg/lib -lglib-2.0 -lintl
>>
>>> tests/ivshmem-test.c also calls shm_open().  Does it work on NetBSD?
>>>
>>
>> Currently it's disabled, as it requires eventfd() (Linux API).
> 
> You're right.
> 
> So is the ivshmem device.  Hmm.  Why would anyone want ivshmem-server on
> NetBSD?  Its purpose is connecting ivshmem-doorbell devices.  Could we
> simply add the obvious CONFIG_IVSHMEM dependence to
> contrib/ivshmem-*/Makefile.objs?
> 

In general I would like to get feature parity on NetBSD, there is no
reason to blacklist this feature for !Linux hosts. However short term
there are higher priorities, to fix build of pristine sources
(ivshmem-server is blocking here, not that I'm right now working on its
correctness on the NetBSD host) and run test suite.

I wanted to avoid overlinking, that would be caused by adding -lrt.
Adding dedicated -lrt check for shm_open(3) made sense to me, but I will
go the requested route.

signature.asc
Description: OpenPGP digital signature