qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools)
@ 2017-07-13 17:03 Peter Maydell
  2017-07-13 18:40 ` Kamil Rytarowski
  2017-07-14  8:34 ` Peter Maydell
  0 siblings, 2 replies; 14+ messages in thread
From: Peter Maydell @ 2017-07-13 17:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches, Kamil Rytarowski, Markus Armbruster

This patchset fixes the build on NetBSD by making sure we only
build the ivshmem-client and ivshmem-server tools if the host OS
actually supports ivshmem. (Previously we were only gating building
of the within-QEMU ivshmem PCI device.)

Most of this patchset is the work of Kamil Rytarowski; I just
split it up into easily-reviewable patches and fixed a few bugs.

Tested on Linux, on NetBSD, and on Linux with a '--disable-linux-user
--disable-system' config (which was the one that was failing with
our previous attempt to fix this bug).

NB: we still have some things we need to fix to be able to put
NetBSD into the build-test set. The next issue is that 'make check'
fails with

MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick tests/check-qdict
sh: arith: syntax error: "RANDOM % 255 + 1"

likely because we're using a bashism there.

thanks
-- PMM

Kamil Rytarowski (2):
  configure: Use an explicit CONFIG_IVSHMEM rather than CONFIG_EVENTFD
  configure: Don't build ivshmem tools unless CONFIG_IVSHMEM is set

Peter Maydell (1):
  configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE

 configure               | 11 +++++++++++
 Makefile                |  2 ++
 Makefile.objs           |  4 ++--
 hw/misc/Makefile.objs   |  2 +-
 tests/Makefile.include  |  4 ++--
 default-configs/pci.mak |  2 +-
 6 files changed, 19 insertions(+), 6 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools)
  2017-07-13 17:03 Peter Maydell
@ 2017-07-13 18:40 ` Kamil Rytarowski
  2017-07-13 20:12   ` Eric Blake
  2017-07-14  8:34 ` Peter Maydell
  1 sibling, 1 reply; 14+ messages in thread
From: Kamil Rytarowski @ 2017-07-13 18:40 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: Markus Armbruster, patches

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

On 13.07.2017 19:03, Peter Maydell wrote:
> This patchset fixes the build on NetBSD by making sure we only
> build the ivshmem-client and ivshmem-server tools if the host OS
> actually supports ivshmem. (Previously we were only gating building
> of the within-QEMU ivshmem PCI device.)
> 
> Most of this patchset is the work of Kamil Rytarowski; I just
> split it up into easily-reviewable patches and fixed a few bugs.
> 

Thank you for improving the patches!

> Tested on Linux, on NetBSD, and on Linux with a '--disable-linux-user
> --disable-system' config (which was the one that was failing with
> our previous attempt to fix this bug).
> 
> NB: we still have some things we need to fix to be able to put
> NetBSD into the build-test set. The next issue is that 'make check'
> fails with
> 
> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick tests/check-qdict
> sh: arith: syntax error: "RANDOM % 255 + 1"
> 
> likely because we're using a bashism there.
> 

This is an extension to a POSIX shell. We were lately adding it as an
extension to our sh(1) in the distribution. Were also thinking about
prompting POSIX to standarize it.

$ echo $((RANDOM % 255))
52

As of now, one portable solution is to parse /dev/urandom, or use a 3rd
party tool like awk(1).

To make the life easier we can try to request bash to run tests. A
portable shebang is "/usr/bin/env bash".

From the NetBSD point of view I see nothing wrong with using bash for tests.

We can also request NetBSD 8.0 for running tests, busy wait till getting
this compatibility issue gone. As of now NetBSD 8.0 is in beta.
Personally I don't intend to invest to much time into pre-8.0 releases.

> thanks
> -- PMM
> 
> Kamil Rytarowski (2):
>   configure: Use an explicit CONFIG_IVSHMEM rather than CONFIG_EVENTFD
>   configure: Don't build ivshmem tools unless CONFIG_IVSHMEM is set
> 
> Peter Maydell (1):
>   configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE
> 
>  configure               | 11 +++++++++++
>  Makefile                |  2 ++
>  Makefile.objs           |  4 ++--
>  hw/misc/Makefile.objs   |  2 +-
>  tests/Makefile.include  |  4 ++--
>  default-configs/pci.mak |  2 +-
>  6 files changed, 19 insertions(+), 6 deletions(-)
> 



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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools)
  2017-07-13 18:40 ` Kamil Rytarowski
@ 2017-07-13 20:12   ` Eric Blake
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2017-07-13 20:12 UTC (permalink / raw)
  To: Kamil Rytarowski, Peter Maydell, qemu-devel; +Cc: Markus Armbruster, patches

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

On 07/13/2017 01:40 PM, Kamil Rytarowski wrote:

>> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick tests/check-qdict
>> sh: arith: syntax error: "RANDOM % 255 + 1"
>>
>> likely because we're using a bashism there.

Yes, assuming that $RANDOM exists is a bashism.

>>
> 
> This is an extension to a POSIX shell. We were lately adding it as an
> extension to our sh(1) in the distribution. Were also thinking about
> prompting POSIX to standarize it.
> 
> $ echo $((RANDOM % 255))
> 52
> 
> As of now, one portable solution is to parse /dev/urandom, or use a 3rd
> party tool like awk(1).

It's also portable (although less random) to do:

$(( ${RANDOM:-0} + 1))

(the whole idea of MALLOC_PERTURB_ only works on glibc anyways, so it
really doesn't matter what you set it to on other systems).

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools)
@ 2017-07-14  8:33 Peter Maydell
  2017-07-14  8:33 ` [Qemu-devel] [PATCH 1/3] configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE Peter Maydell
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Peter Maydell @ 2017-07-14  8:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches, Kamil Rytarowski, Markus Armbruster

This patchset fixes the build on NetBSD by making sure we only
build the ivshmem-client and ivshmem-server tools if the host OS
actually supports ivshmem. (Previously we were only gating building
of the within-QEMU ivshmem PCI device.)

Most of this patchset is the work of Kamil Rytarowski; I just
split it up into easily-reviewable patches and fixed a few bugs.

Tested on Linux, on NetBSD, and on Linux with a '--disable-linux-user
--disable-system' config (which was the one that was failing with
our previous attempt to fix this bug).

NB: we still have some things we need to fix to be able to put
NetBSD into the build-test set. The next issue is that 'make check'
fails with

MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick tests/check-qdict
sh: arith: syntax error: "RANDOM % 255 + 1"

likely because we're using a bashism there.

thanks
-- PMM

Kamil Rytarowski (2):
  configure: Use an explicit CONFIG_IVSHMEM rather than CONFIG_EVENTFD
  configure: Don't build ivshmem tools unless CONFIG_IVSHMEM is set

Peter Maydell (1):
  configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE

 configure               | 11 +++++++++++
 Makefile                |  2 ++
 Makefile.objs           |  4 ++--
 hw/misc/Makefile.objs   |  2 +-
 tests/Makefile.include  |  4 ++--
 default-configs/pci.mak |  2 +-
 6 files changed, 19 insertions(+), 6 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 1/3] configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE
  2017-07-14  8:33 [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools) Peter Maydell
@ 2017-07-14  8:33 ` Peter Maydell
  2017-07-20 11:17   ` Markus Armbruster
  2017-07-14  8:33 ` [Qemu-devel] [PATCH 2/3] configure: Use an explicit CONFIG_IVSHMEM rather than CONFIG_EVENTFD Peter Maydell
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2017-07-14  8:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches, Kamil Rytarowski, Markus Armbruster

The current CONFIG_IVSHMEM is confusing, because it looks like it's a
flag for "do we have ivshmem support?", but actually it's a flag for
"is the ivshmem PCI device being compiled?" (and implicitly "do we
have ivshmem support?" is tested with CONFIG_EVENTFD).

Rename it to CONFIG_IVSHMEM_DEVICE to clear this confusion up;
shortly we will add a new CONFIG_IVSHMEM which really does indicate
whether the host can support ivshmem.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/Makefile.objs   | 2 +-
 default-configs/pci.mak | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 7e373db..28c1560 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -23,7 +23,7 @@ common-obj-$(CONFIG_PUV3) += puv3_pm.o
 
 common-obj-$(CONFIG_MACIO) += macio/
 
-obj-$(CONFIG_IVSHMEM) += ivshmem.o
+obj-$(CONFIG_IVSHMEM_DEVICE) += ivshmem.o
 
 obj-$(CONFIG_REALVIEW) += arm_sysctl.o
 obj-$(CONFIG_NSERIES) += cbus.o
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index 53ff109..2451eb2 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -41,6 +41,6 @@ CONFIG_SDHCI=y
 CONFIG_EDU=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
-CONFIG_IVSHMEM=$(CONFIG_EVENTFD)
+CONFIG_IVSHMEM_DEVICE=$(CONFIG_EVENTFD)
 CONFIG_ROCKER=y
 CONFIG_VHOST_USER_SCSI=$(CONFIG_LINUX)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 2/3] configure: Use an explicit CONFIG_IVSHMEM rather than CONFIG_EVENTFD
  2017-07-14  8:33 [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools) Peter Maydell
  2017-07-14  8:33 ` [Qemu-devel] [PATCH 1/3] configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE Peter Maydell
@ 2017-07-14  8:33 ` Peter Maydell
  2017-07-14  8:33 ` [Qemu-devel] [PATCH 3/3] configure: Don't build ivshmem tools unless CONFIG_IVSHMEM is set Peter Maydell
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2017-07-14  8:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches, Kamil Rytarowski, Markus Armbruster

From: Kamil Rytarowski <n54@gmx.com>

Rather than relying on everywhere that cares about whether the host
supports ivshmem using CONFIG_EVENTFD, make configure set an explicit
CONFIG_IVSHMEM.

Signed-off-by: Kamil Rytarowski <n54@gmx.com>
[PMM: split out from another patch, add commit message]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure               | 9 +++++++++
 tests/Makefile.include  | 4 ++--
 default-configs/pci.mak | 2 +-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 5096cbc..dea46c5 100755
--- a/configure
+++ b/configure
@@ -4969,6 +4969,11 @@ qemu_moddir=$libdir$confsuffix
 qemu_datadir=$datadir$confsuffix
 qemu_localedir="$datadir/locale"
 
+# We can only support ivshmem if we have eventfd
+if [ "$eventfd" = "yes" ]; then
+  ivshmem=yes
+fi
+
 tools=""
 if test "$want_tools" = "yes" ; then
   tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
@@ -5955,6 +5960,10 @@ if test "$have_static_assert" = "yes" ; then
   echo "CONFIG_STATIC_ASSERT=y" >> $config_host_mak
 fi
 
+if test "$ivshmem" = "yes" ; then
+  echo "CONFIG_IVSHMEM=y" >> $config_host_mak
+fi
+
 # Hold two types of flag:
 #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
 #                                     a thread we have a handle to
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 42e17e2..6d6cb74 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -201,7 +201,7 @@ gcov-files-pci-y += hw/display/virtio-gpu-pci.c
 gcov-files-pci-$(CONFIG_VIRTIO_VGA) += hw/display/virtio-vga.c
 check-qtest-pci-y += tests/intel-hda-test$(EXESUF)
 gcov-files-pci-y += hw/audio/intel-hda.c hw/audio/hda-codec.c
-check-qtest-pci-$(CONFIG_EVENTFD) += tests/ivshmem-test$(EXESUF)
+check-qtest-pci-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
 gcov-files-pci-y += hw/misc/ivshmem.c
 check-qtest-pci-y += tests/megasas-test$(EXESUF)
 gcov-files-pci-y += hw/scsi/megasas.c
@@ -303,7 +303,7 @@ check-qtest-ppc64-y += tests/test-filter-mirror$(EXESUF)
 check-qtest-ppc64-y += tests/test-filter-redirector$(EXESUF)
 check-qtest-ppc64-y += tests/display-vga-test$(EXESUF)
 check-qtest-ppc64-y += tests/numa-test$(EXESUF)
-check-qtest-ppc64-$(CONFIG_EVENTFD) += tests/ivshmem-test$(EXESUF)
+check-qtest-ppc64-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
 
 check-qtest-sh4-y = tests/endianness-test$(EXESUF)
 
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index 2451eb2..acaa703 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -41,6 +41,6 @@ CONFIG_SDHCI=y
 CONFIG_EDU=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
-CONFIG_IVSHMEM_DEVICE=$(CONFIG_EVENTFD)
+CONFIG_IVSHMEM_DEVICE=$(CONFIG_IVSHMEM)
 CONFIG_ROCKER=y
 CONFIG_VHOST_USER_SCSI=$(CONFIG_LINUX)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 3/3] configure: Don't build ivshmem tools unless CONFIG_IVSHMEM is set
  2017-07-14  8:33 [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools) Peter Maydell
  2017-07-14  8:33 ` [Qemu-devel] [PATCH 1/3] configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE Peter Maydell
  2017-07-14  8:33 ` [Qemu-devel] [PATCH 2/3] configure: Use an explicit CONFIG_IVSHMEM rather than CONFIG_EVENTFD Peter Maydell
@ 2017-07-14  8:33 ` Peter Maydell
  2017-07-20  9:43 ` [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools) Peter Maydell
  2017-07-20 11:20 ` Markus Armbruster
  4 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2017-07-14  8:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches, Kamil Rytarowski, Markus Armbruster

From: Kamil Rytarowski <n54@gmx.com>

Don't try to build the ivshmem-server and ivshmem-client tools unless
CONFIG_IVSHMEM is set.

This fixes in passing a build bug on NetBSD, which fails to build the
ivshmem tools because they use shm_open() and on NetBSD that requires
linking against -lrt.

Signed-off-by: Kamil Rytarowski <n54@gmx.com>
[PMM: moved some code into earlier patches; minor bugfixes;
 added commit message]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure     | 2 ++
 Makefile      | 2 ++
 Makefile.objs | 4 ++--
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index dea46c5..b54bc2e 100755
--- a/configure
+++ b/configure
@@ -4979,6 +4979,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 [ "$ivshmem" = "yes" ]; then
     tools="ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools"
   fi
 fi
diff --git a/Makefile b/Makefile
index 16a0430..5d5bd6a 100644
--- a/Makefile
+++ b/Makefile
@@ -470,10 +470,12 @@ ifneq ($(EXESUF),)
 qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI)
 endif
 
+ifdef CONFIG_IVSHMEM
 ivshmem-client$(EXESUF): $(ivshmem-client-obj-y) $(COMMON_LDADDS)
 	$(call LINK, $^)
 ivshmem-server$(EXESUF): $(ivshmem-server-obj-y) $(COMMON_LDADDS)
 	$(call LINK, $^)
+endif
 vhost-user-scsi$(EXESUF): $(vhost-user-scsi-obj-y)
 	$(call LINK, $^)
 
diff --git a/Makefile.objs b/Makefile.objs
index bfd5a6c..24a4ea0 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -107,8 +107,8 @@ qga-vss-dll-obj-y = qga/
 
 ######################################################################
 # contrib
-ivshmem-client-obj-y = contrib/ivshmem-client/
-ivshmem-server-obj-y = contrib/ivshmem-server/
+ivshmem-client-obj-$(CONFIG_IVSHMEM) = contrib/ivshmem-client/
+ivshmem-server-obj-$(CONFIG_IVSHMEM) = contrib/ivshmem-server/
 libvhost-user-obj-y = contrib/libvhost-user/
 vhost-user-scsi.o-cflags := $(LIBISCSI_CFLAGS)
 vhost-user-scsi.o-libs := $(LIBISCSI_LIBS)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools)
  2017-07-13 17:03 Peter Maydell
  2017-07-13 18:40 ` Kamil Rytarowski
@ 2017-07-14  8:34 ` Peter Maydell
  1 sibling, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2017-07-14  8:34 UTC (permalink / raw)
  To: QEMU Developers; +Cc: Kamil Rytarowski, Markus Armbruster, patches@linaro.org

On 13 July 2017 at 18:03, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchset fixes the build on NetBSD by making sure we only
> build the ivshmem-client and ivshmem-server tools if the host OS
> actually supports ivshmem. (Previously we were only gating building
> of the within-QEMU ivshmem PCI device.)

Markus pointed out that I'd accidentally only sent out the
cover letter -- I've just resent the whole set.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools)
  2017-07-14  8:33 [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools) Peter Maydell
                   ` (2 preceding siblings ...)
  2017-07-14  8:33 ` [Qemu-devel] [PATCH 3/3] configure: Don't build ivshmem tools unless CONFIG_IVSHMEM is set Peter Maydell
@ 2017-07-20  9:43 ` Peter Maydell
  2017-07-20 11:20 ` Markus Armbruster
  4 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2017-07-20  9:43 UTC (permalink / raw)
  To: QEMU Developers; +Cc: Kamil Rytarowski, Markus Armbruster, patches@linaro.org

Ping for review? It would be nice to get the NetBSD host into
the build-test rotation...

thanks
-- PMM

On 14 July 2017 at 09:33, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchset fixes the build on NetBSD by making sure we only
> build the ivshmem-client and ivshmem-server tools if the host OS
> actually supports ivshmem. (Previously we were only gating building
> of the within-QEMU ivshmem PCI device.)
>
> Most of this patchset is the work of Kamil Rytarowski; I just
> split it up into easily-reviewable patches and fixed a few bugs.
>
> Tested on Linux, on NetBSD, and on Linux with a '--disable-linux-user
> --disable-system' config (which was the one that was failing with
> our previous attempt to fix this bug).
>
> NB: we still have some things we need to fix to be able to put
> NetBSD into the build-test set. The next issue is that 'make check'
> fails with
>
> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick tests/check-qdict
> sh: arith: syntax error: "RANDOM % 255 + 1"
>
> likely because we're using a bashism there.
>
> thanks
> -- PMM
>
> Kamil Rytarowski (2):
>   configure: Use an explicit CONFIG_IVSHMEM rather than CONFIG_EVENTFD
>   configure: Don't build ivshmem tools unless CONFIG_IVSHMEM is set
>
> Peter Maydell (1):
>   configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE
>
>  configure               | 11 +++++++++++
>  Makefile                |  2 ++
>  Makefile.objs           |  4 ++--
>  hw/misc/Makefile.objs   |  2 +-
>  tests/Makefile.include  |  4 ++--
>  default-configs/pci.mak |  2 +-
>  6 files changed, 19 insertions(+), 6 deletions(-)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE
  2017-07-14  8:33 ` [Qemu-devel] [PATCH 1/3] configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE Peter Maydell
@ 2017-07-20 11:17   ` Markus Armbruster
  2017-07-20 11:54     ` Peter Maydell
  0 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2017-07-20 11:17 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, Kamil Rytarowski, patches

Peter Maydell <peter.maydell@linaro.org> writes:

> The current CONFIG_IVSHMEM is confusing, because it looks like it's a
> flag for "do we have ivshmem support?", but actually it's a flag for
> "is the ivshmem PCI device being compiled?" (and implicitly "do we
> have ivshmem support?" is tested with CONFIG_EVENTFD).
>
> Rename it to CONFIG_IVSHMEM_DEVICE to clear this confusion up;
> shortly we will add a new CONFIG_IVSHMEM which really does indicate
> whether the host can support ivshmem.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

I don't really care how we name this macro, but I can't help to
wonder...  We have many CONFIG_<dev>.  The pci.mak context even shows
some.  Why is <dev> = IVSHMEM confusing?  Why is <dev> = EDU *not*
confusing?

> ---
>  hw/misc/Makefile.objs   | 2 +-
>  default-configs/pci.mak | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 7e373db..28c1560 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -23,7 +23,7 @@ common-obj-$(CONFIG_PUV3) += puv3_pm.o
>  
>  common-obj-$(CONFIG_MACIO) += macio/
>  
> -obj-$(CONFIG_IVSHMEM) += ivshmem.o
> +obj-$(CONFIG_IVSHMEM_DEVICE) += ivshmem.o
>  
>  obj-$(CONFIG_REALVIEW) += arm_sysctl.o
>  obj-$(CONFIG_NSERIES) += cbus.o
> diff --git a/default-configs/pci.mak b/default-configs/pci.mak
> index 53ff109..2451eb2 100644
> --- a/default-configs/pci.mak
> +++ b/default-configs/pci.mak
> @@ -41,6 +41,6 @@ CONFIG_SDHCI=y
>  CONFIG_EDU=y
>  CONFIG_VGA=y
>  CONFIG_VGA_PCI=y
> -CONFIG_IVSHMEM=$(CONFIG_EVENTFD)
> +CONFIG_IVSHMEM_DEVICE=$(CONFIG_EVENTFD)
>  CONFIG_ROCKER=y
>  CONFIG_VHOST_USER_SCSI=$(CONFIG_LINUX)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools)
  2017-07-14  8:33 [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools) Peter Maydell
                   ` (3 preceding siblings ...)
  2017-07-20  9:43 ` [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools) Peter Maydell
@ 2017-07-20 11:20 ` Markus Armbruster
  2017-07-20 15:35   ` Peter Maydell
  4 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2017-07-20 11:20 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, Kamil Rytarowski, patches

Peter Maydell <peter.maydell@linaro.org> writes:

> This patchset fixes the build on NetBSD by making sure we only
> build the ivshmem-client and ivshmem-server tools if the host OS
> actually supports ivshmem. (Previously we were only gating building
> of the within-QEMU ivshmem PCI device.)
>
> Most of this patchset is the work of Kamil Rytarowski; I just
> split it up into easily-reviewable patches and fixed a few bugs.
>
> Tested on Linux, on NetBSD, and on Linux with a '--disable-linux-user
> --disable-system' config (which was the one that was failing with
> our previous attempt to fix this bug).
>
> NB: we still have some things we need to fix to be able to put
> NetBSD into the build-test set. The next issue is that 'make check'
> fails with
>
> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick tests/check-qdict
> sh: arith: syntax error: "RANDOM % 255 + 1"
>
> likely because we're using a bashism there.

Regardless of my inability to find CONFIG_IVSHMEM confusing (see PATCH
1), series
Reviewed-by: Markus Armbruster <armbru@redhat.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE
  2017-07-20 11:17   ` Markus Armbruster
@ 2017-07-20 11:54     ` Peter Maydell
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2017-07-20 11:54 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: QEMU Developers, Kamil Rytarowski, patches@linaro.org

On 20 July 2017 at 12:17, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> The current CONFIG_IVSHMEM is confusing, because it looks like it's a
>> flag for "do we have ivshmem support?", but actually it's a flag for
>> "is the ivshmem PCI device being compiled?" (and implicitly "do we
>> have ivshmem support?" is tested with CONFIG_EVENTFD).
>>
>> Rename it to CONFIG_IVSHMEM_DEVICE to clear this confusion up;
>> shortly we will add a new CONFIG_IVSHMEM which really does indicate
>> whether the host can support ivshmem.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> I don't really care how we name this macro, but I can't help to
> wonder...  We have many CONFIG_<dev>.  The pci.mak context even shows
> some.  Why is <dev> = IVSHMEM confusing?  Why is <dev> = EDU *not*
> confusing?

Well, this did confuse the people involved in trying to fix
this bug first time round, because we tried to add
an "ifdef CONFIG_IVSHMEM" guard to the Makefile lines that
defnie the rules for ivshmem-client and ivshmem-server,
and it it fails in obscure and confusing ways because
CONFIG_IVSHMEM doesn't mean "ivshmem OK", it means only
"this particular foo-softmmu build has the ivshmem device in it".

Basically, it looks like a global config flag but it isn't
(because ivshmem has globally built parts, ie the tools,
as well as the device itself, and configure tests that control
whether it can be built or not) whereas nobody thinks CONFIG_EDU
is something that should control global parts of the build
because the device is only the device and the host config
doesn't matter.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools)
  2017-07-20 11:20 ` Markus Armbruster
@ 2017-07-20 15:35   ` Peter Maydell
  2017-07-20 16:59     ` Kamil Rytarowski
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2017-07-20 15:35 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: QEMU Developers, Kamil Rytarowski, patches@linaro.org

On 20 July 2017 at 12:20, Markus Armbruster <armbru@redhat.com> wrote:
> Regardless of my inability to find CONFIG_IVSHMEM confusing (see PATCH
> 1), series
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks; patchset applied to master.

-- PMM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools)
  2017-07-20 15:35   ` Peter Maydell
@ 2017-07-20 16:59     ` Kamil Rytarowski
  0 siblings, 0 replies; 14+ messages in thread
From: Kamil Rytarowski @ 2017-07-20 16:59 UTC (permalink / raw)
  To: Peter Maydell, Markus Armbruster; +Cc: QEMU Developers, patches@linaro.org

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

On 20.07.2017 17:35, Peter Maydell wrote:
> On 20 July 2017 at 12:20, Markus Armbruster <armbru@redhat.com> wrote:
>> Regardless of my inability to find CONFIG_IVSHMEM confusing (see PATCH
>> 1), series
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> Thanks; patchset applied to master.
> 
> -- PMM
> 

Thank you for this!


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-07-20 16:57 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-14  8:33 [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools) Peter Maydell
2017-07-14  8:33 ` [Qemu-devel] [PATCH 1/3] configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE Peter Maydell
2017-07-20 11:17   ` Markus Armbruster
2017-07-20 11:54     ` Peter Maydell
2017-07-14  8:33 ` [Qemu-devel] [PATCH 2/3] configure: Use an explicit CONFIG_IVSHMEM rather than CONFIG_EVENTFD Peter Maydell
2017-07-14  8:33 ` [Qemu-devel] [PATCH 3/3] configure: Don't build ivshmem tools unless CONFIG_IVSHMEM is set Peter Maydell
2017-07-20  9:43 ` [Qemu-devel] [PATCH 0/3] Fix NetBSD build (don't build ivshmem tools) Peter Maydell
2017-07-20 11:20 ` Markus Armbruster
2017-07-20 15:35   ` Peter Maydell
2017-07-20 16:59     ` Kamil Rytarowski
  -- strict thread matches above, loose matches on Subject: below --
2017-07-13 17:03 Peter Maydell
2017-07-13 18:40 ` Kamil Rytarowski
2017-07-13 20:12   ` Eric Blake
2017-07-14  8:34 ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).