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

[Kamil Rytarowski]

On 23.05.2017 17:07, Markus Armbruster wrote:
> Kamil Rytarowski <address@hidden> writes:
> 
>> On 23.05.2017 07:37, Markus Armbruster wrote:
>>> Kamil Rytarowski <address@hidden> writes:
>>>
>>>> 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
>>>
>>> Feature parity is a worthy goal, but compiling ivshmem-server without
>>> ivshmem-doorbell doesn't gets you a feature, it gets you a binary you
>>> can't use for its intended purpose :)
>>>
>>
>> $ qemu-system-x86_64 -device ? 2>&1 |grep ivsh
>> name "ivshmem", bus PCI, desc "Inter-VM shared memory (legacy)"
>> name "ivshmem-doorbell", bus PCI, desc "Inter-VM shared memory"
>> name "ivshmem-plain", bus PCI, desc "Inter-VM shared memory"
>>
>> This will need definitely more work. We can disable ivshmem and hide the
>> problem, but it will be unveiled in future again and we will be back to
>> this discussion.
>>
>> Also people can use NetBSD libc on Linux and share the same issue.
> 
> CONFIG_IVSHMEM defaults to CONFIG_EVENTFD.  If you compile with a
> toolchain that doesn't provide eventfd(), configure should deselect
> CONFIG_EVENTFD and thus CONFIG_IVSHMEM automatically.
> 
> Having contrib/ivshmem* depend on CONFIG_IVSHMEM makes total sense.
> Looking forward to your patch.
> 
> [...]
> 

OK, I will prepare this way the needed patch.

signature.asc
Description: OpenPGP digital signature