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

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

[Kamil Rytarowski]

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 <n54@gmx.com>
---
 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
-- 
2.12.2

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

[Philippe Mathieu-Daudé]

On 05/12/2017 09:46 PM, Kamil Rytarowski wrote:
> 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 <n54@gmx.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  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
>

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

[Markus Armbruster]

Kamil Rytarowski <n54@gmx.com> 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 <n54@gmx.com>
> ---
>  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.  It looks for timer_create() and
clock_gettime().  If we need -lrt, 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()?

Can you confirm that the existing test does not add -lrt to LIBS on
NetBSD?

tests/ivshmem-test.c also calls shm_open().  Does it work on NetBSD?

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

[Kamil Rytarowski]

[-- Attachment #1: Type: text/plain, Size: 3943 bytes --]

Hello,

Excuse me for delay, I missed this mail.

Please see in-line.

On 17.05.2017 09:28, Markus Armbruster wrote:
> Kamil Rytarowski <n54@gmx.com> 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 <n54@gmx.com>
>> ---
>>  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).


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Markus Armbruster]

Kamil Rytarowski <n54@gmx.com> 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 <n54@gmx.com> 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 <n54@gmx.com>
>>> ---
>>>  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?

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

[Kamil Rytarowski]

[-- Attachment #1: Type: text/plain, Size: 5082 bytes --]

On 22.05.2017 08:28, Markus Armbruster wrote:
> Kamil Rytarowski <n54@gmx.com> 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 <n54@gmx.com> 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 <n54@gmx.com>
>>>> ---
>>>>  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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Markus Armbruster]

Kamil Rytarowski <n54@gmx.com> writes:

> On 22.05.2017 08:28, Markus Armbruster wrote:
>> Kamil Rytarowski <n54@gmx.com> 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 <n54@gmx.com> 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 <n54@gmx.com>
>>>>> ---
>>>>>  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 :)

> 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.

Your priorities make sense.

> 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.

Avoiding overlinking makes sense, too.

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

[Kamil Rytarowski]

[-- Attachment #1: Type: text/plain, Size: 6239 bytes --]

On 23.05.2017 07:37, Markus Armbruster wrote:
> Kamil Rytarowski <n54@gmx.com> writes:
> 
>> On 22.05.2017 08:28, Markus Armbruster wrote:
>>> Kamil Rytarowski <n54@gmx.com> 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 <n54@gmx.com> 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 <n54@gmx.com>
>>>>>> ---
>>>>>>  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.

>> 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.
> 
> Your priorities make sense.
> 
>> 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.
> 
> Avoiding overlinking makes sense, too.
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Markus Armbruster]

Kamil Rytarowski <n54@gmx.com> writes:

> On 23.05.2017 07:37, Markus Armbruster wrote:
>> Kamil Rytarowski <n54@gmx.com> writes:
>> 
>>> On 22.05.2017 08:28, Markus Armbruster wrote:
>>>> Kamil Rytarowski <n54@gmx.com> 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 <n54@gmx.com> 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 <n54@gmx.com>
>>>>>>> ---
>>>>>>>  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.

[...]

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

[Kamil Rytarowski]

[-- Attachment #1: Type: text/plain, Size: 6568 bytes --]

On 23.05.2017 17:07, Markus Armbruster wrote:
> Kamil Rytarowski <n54@gmx.com> writes:
> 
>> On 23.05.2017 07:37, Markus Armbruster wrote:
>>> Kamil Rytarowski <n54@gmx.com> writes:
>>>
>>>> On 22.05.2017 08:28, Markus Armbruster wrote:
>>>>> Kamil Rytarowski <n54@gmx.com> 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 <n54@gmx.com> 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 <n54@gmx.com>
>>>>>>>> ---
>>>>>>>>  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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Kamil Rytarowski]

Currently ivshmem requires eventfd() which is Linux specific.
Do not and build it unconditionally on every Linux/BSD/Solaris.

This patch indirectly fixes build failure on NetBSD, where these tools
additionally require -lrl for shm_open(3). In future there should be
added support for NetBSD and the linking addressed appropriately.

Signed-off-by: Kamil Rytarowski <n54@gmx.com>
---
 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
     tools="ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools"
   fi
 fi
-- 
2.13.0

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

[Marc-André Lureau]

Hi

On Fri, May 26, 2017 at 9:49 AM Kamil Rytarowski <n54@gmx.com> 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() .


> This patch indirectly fixes build failure on NetBSD, where these tools
> additionally require -lrl for shm_open(3). In future there should be
> added support for NetBSD and the linking addressed appropriately.
>
> Signed-off-by: Kamil Rytarowski <n54@gmx.com>
>
---
>  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
>      tools="ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools"
>    fi
>  fi
> --
> 2.13.0
>
>
> --
Marc-André Lureau

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

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Fri, May 26, 2017 at 9:49 AM Kamil Rytarowski <n54@gmx.com> 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 <armbru@redhat.com>
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 <pbonzini@redhat.com>
    Signed-off-by: Markus Armbruster <armbru@redhat.com>
    Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
    Message-Id: <1458066895-20632-6-git-send-email-armbru@redhat.com>

>> 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 <n54@gmx.com>
>>
> ---
>>  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 <armbru@redhat.com>

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

[Qemu-devel] [PATCH] ivshmem-server: ivshmem-client: Build when eventfd() is available

[Kamil Rytarowski]

Currently ivshmem requires eventfd() which is Linux specific.
Do not and build it unconditionally on every Linux/BSD/Solaris.

This patch indirectly fixes build failure on NetBSD, where these tools
additionally require -lrt for shm_open(3). In future there should be
added support for NetBSD and the linking addressed appropriately.

Signed-off-by: Kamil Rytarowski <n54@gmx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 configure                            | 2 ++
 contrib/ivshmem-client/Makefile.objs | 2 +-
 contrib/ivshmem-server/Makefile.objs | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 1a5ee4b909..483307be53 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" = "yes" ]; then
     tools="ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools"
   fi
 fi
diff --git a/contrib/ivshmem-client/Makefile.objs b/contrib/ivshmem-client/Makefile.objs
index bfab2d20dd..13d864082d 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 c060dd3698..d9469fd777 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
-- 
2.11.1

Re: [Qemu-devel] [PATCH] ivshmem-server: ivshmem-client: Build when eventfd() is available

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]

On 05/30/2017 01:20 AM, Kamil Rytarowski wrote:
> Currently ivshmem requires eventfd() which is Linux specific.
> Do not and build it unconditionally on every Linux/BSD/Solaris.
> 
> This patch indirectly fixes build failure on NetBSD, where these tools
> additionally require -lrt for shm_open(3). In future there should be
> added support for NetBSD and the linking addressed appropriately.

Is this a v2 patch? If so, it's best to repost it as a new top-level
thread rather than buried in reply to your v1 patch.  Also, be sure to
include v2 in the subject line ('git send-email -v2' works wonders for
that).

> 
> Signed-off-by: Kamil Rytarowski <n54@gmx.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> ---

It also helps in a v2 submission to list here, after the --- separator,
how it differs from v1.

More submission hints at http://wiki.qemu.org/Contribute/SubmitAPatch

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] ivshmem-server: ivshmem-client: Build when eventfd() is available

[Kamil Rytarowski]

[-- Attachment #1: Type: text/plain, Size: 1079 bytes --]

On 30.05.2017 14:53, Eric Blake wrote:
> On 05/30/2017 01:20 AM, Kamil Rytarowski wrote:
>> Currently ivshmem requires eventfd() which is Linux specific.
>> Do not and build it unconditionally on every Linux/BSD/Solaris.
>>
>> This patch indirectly fixes build failure on NetBSD, where these tools
>> additionally require -lrt for shm_open(3). In future there should be
>> added support for NetBSD and the linking addressed appropriately.
> 
> Is this a v2 patch? If so, it's best to repost it as a new top-level
> thread rather than buried in reply to your v1 patch.  Also, be sure to
> include v2 in the subject line ('git send-email -v2' works wonders for
> that).
> 

I reworked it, fixed title and it's more like v3.

>>
>> Signed-off-by: Kamil Rytarowski <n54@gmx.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> ---
> 
> It also helps in a v2 submission to list here, after the --- separator,
> how it differs from v1.
> 
> More submission hints at http://wiki.qemu.org/Contribute/SubmitAPatch
> 

Do I still need to resend it?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] ivshmem-server: ivshmem-client: Build when eventfd() is available

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On 05/30/2017 08:11 AM, Kamil Rytarowski wrote:

>>
>> Is this a v2 patch? If so, it's best to repost it as a new top-level
>> thread rather than buried in reply to your v1 patch.  Also, be sure to
>> include v2 in the subject line ('git send-email -v2' works wonders for
>> that).
>>

> 
> I reworked it, fixed title and it's more like v3.
> 

> Do I still need to resend it?
> 

A human maintainer can often (eventually) figure out what's going on if
you don't, but resending is easier if you want to ensure the automated
patch tools don't lose things.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] ivshmem-server: ivshmem-client: Build when eventfd() is available

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 05/30/2017 08:11 AM, Kamil Rytarowski wrote:
>
>>>
>>> Is this a v2 patch? If so, it's best to repost it as a new top-level
>>> thread rather than buried in reply to your v1 patch.  Also, be sure to
>>> include v2 in the subject line ('git send-email -v2' works wonders for
>>> that).
>>>
>
>> 
>> I reworked it, fixed title and it's more like v3.
>> 
>
>> Do I still need to resend it?
>> 
>
> A human maintainer can often (eventually) figure out what's going on if
> you don't, but resending is easier if you want to ensure the automated
> patch tools don't lose things.

Yes.

However, ivshmem doesn't have a maintainer.  Since the patch is really
simple, the easiest path forward could be sending a v3 with
cc: qemu-trivial@nongnu.org.

Re: [Qemu-devel] [PATCH] ivshmem-server: ivshmem-client: Build when eventfd() is available

[Kamil Rytarowski]

[-- Attachment #1: Type: text/plain, Size: 920 bytes --]

On 31.05.2017 13:11, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 05/30/2017 08:11 AM, Kamil Rytarowski wrote:
>>
>>>>
>>>> Is this a v2 patch? If so, it's best to repost it as a new top-level
>>>> thread rather than buried in reply to your v1 patch.  Also, be sure to
>>>> include v2 in the subject line ('git send-email -v2' works wonders for
>>>> that).
>>>>
>>
>>>
>>> I reworked it, fixed title and it's more like v3.
>>>
>>
>>> Do I still need to resend it?
>>>
>>
>> A human maintainer can often (eventually) figure out what's going on if
>> you don't, but resending is easier if you want to ensure the automated
>> patch tools don't lose things.
> 
> Yes.
> 
> However, ivshmem doesn't have a maintainer.  Since the patch is really
> simple, the easiest path forward could be sending a v3 with
> cc: qemu-trivial@nongnu.org.
> 

OK, I will do this.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] ivshmem-server: ivshmem-client: Build when eventfd() is available

[Markus Armbruster]

Kamil Rytarowski <n54@gmx.com> writes:

> On 31.05.2017 13:11, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>>> On 05/30/2017 08:11 AM, Kamil Rytarowski wrote:
>>>
>>>>>
>>>>> Is this a v2 patch? If so, it's best to repost it as a new top-level
>>>>> thread rather than buried in reply to your v1 patch.  Also, be sure to
>>>>> include v2 in the subject line ('git send-email -v2' works wonders for
>>>>> that).
>>>>>
>>>
>>>>
>>>> I reworked it, fixed title and it's more like v3.
>>>>
>>>
>>>> Do I still need to resend it?
>>>>
>>>
>>> A human maintainer can often (eventually) figure out what's going on if
>>> you don't, but resending is easier if you want to ensure the automated
>>> patch tools don't lose things.
>> 
>> Yes.
>> 
>> However, ivshmem doesn't have a maintainer.  Since the patch is really
>> simple, the easiest path forward could be sending a v3 with
>> cc: qemu-trivial@nongnu.org.
>> 
>
> OK, I will do this.

Thank you for your patience (and the patch, of course).