diff mbox series

[1/3] configure: Rename CONFIG_IVSHMEM to CONFIG_IVSHMEM_DEVICE

Message ID 1500021225-4118-2-git-send-email-peter.maydell@linaro.org
State Accepted
Headers show
Series Fix NetBSD build (don't build ivshmem tools) | expand

Commit Message

Peter Maydell July 14, 2017, 8:33 a.m. UTC
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(-)

-- 
2.7.4

Comments

Markus Armbruster July 20, 2017, 11:17 a.m. UTC | #1
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)
Peter Maydell July 20, 2017, 11:54 a.m. UTC | #2
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
diff mbox series

Patch

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)