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