| Message ID | 20230606134913.93724-1-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/remote/proxy: Remove dubious 'event_notifier-posix.c' include | expand |
Queued, thanks. Paolo
On Tue, 6 Jun 2023 at 14:50, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > event_notifier-posix.c is registered in meson's util_ss[] source > set, which is built as libqemuutil.a.p library. Both tools and > system emulation binaries are linked with qemuutil, so there is > no point in including this source file. > > Introduced in commit bd36adb8df ("multi-process: create IOHUB > object to handle irq"). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > Note, --enable-multiprocess doesn't seem to be covered in CI. > --- > hw/remote/proxy.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c > index 1c7786b52c..2052d721e5 100644 > --- a/hw/remote/proxy.c > +++ b/hw/remote/proxy.c > @@ -22,7 +22,6 @@ > #include "qom/object.h" > #include "qemu/event_notifier.h" > #include "sysemu/kvm.h" > -#include "util/event_notifier-posix.c" Including one .c file from another is definitely very weird; if it is by some chance not incorrect then it needs a big comment describing why it's necessary... -- PMM
On 6/6/23 15:59, Peter Maydell wrote: > On Tue, 6 Jun 2023 at 14:50, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: >> >> event_notifier-posix.c is registered in meson's util_ss[] source >> set, which is built as libqemuutil.a.p library. Both tools and >> system emulation binaries are linked with qemuutil, so there is >> no point in including this source file. >> >> Introduced in commit bd36adb8df ("multi-process: create IOHUB >> object to handle irq"). >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> Note, --enable-multiprocess doesn't seem to be covered in CI. >> --- >> hw/remote/proxy.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c >> index 1c7786b52c..2052d721e5 100644 >> --- a/hw/remote/proxy.c >> +++ b/hw/remote/proxy.c >> @@ -22,7 +22,6 @@ >> #include "qom/object.h" >> #include "qemu/event_notifier.h" >> #include "sysemu/kvm.h" >> -#include "util/event_notifier-posix.c" > > Including one .c file from another is definitely very weird; > if it is by some chance not incorrect then it needs a big > comment describing why it's necessary... Building QEMU configured with --enable-multiprocess still works, so this doesn't look (anymore?) necessary. $ git grep TYPE_PCI_PROXY_DEV hw/remote/proxy.c:218: .name = TYPE_PCI_PROXY_DEV, include/hw/remote/proxy.h:17:#define TYPE_PCI_PROXY_DEV "x-pci-proxy-dev" $ ./qemu-system-x86_64 -device help | fgrep proxy name "x-pci-proxy-dev", bus PCI
On 06/06/2023 15.49, Philippe Mathieu-Daudé wrote: > event_notifier-posix.c is registered in meson's util_ss[] source > set, which is built as libqemuutil.a.p library. Both tools and > system emulation binaries are linked with qemuutil, so there is > no point in including this source file. > > Introduced in commit bd36adb8df ("multi-process: create IOHUB > object to handle irq"). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > Note, --enable-multiprocess doesn't seem to be covered in CI. > --- > hw/remote/proxy.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c > index 1c7786b52c..2052d721e5 100644 > --- a/hw/remote/proxy.c > +++ b/hw/remote/proxy.c > @@ -22,7 +22,6 @@ > #include "qom/object.h" > #include "qemu/event_notifier.h" > #include "sysemu/kvm.h" > -#include "util/event_notifier-posix.c" > > static void probe_pci_info(PCIDevice *dev, Error **errp); > static void proxy_device_reset(DeviceState *dev); Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c index 1c7786b52c..2052d721e5 100644 --- a/hw/remote/proxy.c +++ b/hw/remote/proxy.c @@ -22,7 +22,6 @@ #include "qom/object.h" #include "qemu/event_notifier.h" #include "sysemu/kvm.h" -#include "util/event_notifier-posix.c" static void probe_pci_info(PCIDevice *dev, Error **errp); static void proxy_device_reset(DeviceState *dev);
event_notifier-posix.c is registered in meson's util_ss[] source set, which is built as libqemuutil.a.p library. Both tools and system emulation binaries are linked with qemuutil, so there is no point in including this source file. Introduced in commit bd36adb8df ("multi-process: create IOHUB object to handle irq"). Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- Note, --enable-multiprocess doesn't seem to be covered in CI. --- hw/remote/proxy.c | 1 - 1 file changed, 1 deletion(-)