[Qemu-devel] [PATCH v2 27/29] Include sysemu/sysemu.h a lot less

Markus Armbruster posted 29 patches 6 years, 6 months ago
Maintainers: Giuseppe Lettieri <g.lettieri@iet.unipi.it>, Andrzej Zaborowski <balrogg@gmail.com>, Stafford Horne <shorne@gmail.com>, Xiao Guangrong <xiaoguangrong.eric@gmail.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, Riku Voipio <riku.voipio@iki.fi>, Michael Walle <michael@walle.cc>, Juan Quintela <quintela@redhat.com>, Pierre Morel <pmorel@linux.ibm.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Markus Armbruster <armbru@redhat.com>, BALATON Zoltan <balaton@eik.bme.hu>, Stefan Weil <sw@weilnetz.de>, Corey Minyard <minyard@acm.org>, Leif Lindholm <leif.lindholm@linaro.org>, Beniamino Galvani <b.galvani@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Amit Shah <amit@kernel.org>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Collin Walling <walling@linux.ibm.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Yuval Shaia <yuval.shaia@oracle.com>, Liu Yuan <namei.unix@gmail.com>, Alex Williamson <alex.williamson@redhat.com>, Peter Chubb <peter.chubb@nicta.com.au>, Anthony Perard <anthony.perard@citrix.com>, Andrew Jeffery <andrew@aj.id.au>, Alistair Francis <Alistair.Francis@wdc.com>, Richard Henderson <rth@twiddle.net>, Luigi Rizzo <rizzo@iet.unipi.it>, Anthony Green <green@moxielogic.com>, Shannon Zhao <shannon.zhaosl@gmail.com>, Peter Lieven <pl@kamp.de>, Jason Wang <jasowang@redhat.com>, Laurent Vivier <laurent@vivier.eu>, Corey Minyard <cminyard@mvista.com>, Vincenzo Maffione <v.maffione@gmail.com>, James Hogan <jhogan@kernel.org>, Gonglei <arei.gonglei@huawei.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Fabien Chouteau <chouteau@adacore.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Marek Vasut <marex@denx.de>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Alberto Garcia <berto@igalia.com>, Laurent Vivier <lvivier@redhat.com>, Max Filippov <jcmvbkbc@gmail.com>, KONRAD Frederic <frederic.konrad@adacore.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Keith Busch <keith.busch@intel.com>, Stefano Stabellini <sstabellini@kernel.org>, zhanghailiang <zhang.zhanghailiang@huawei.com>, Eric Blake <eblake@redhat.com>, David Hildenbrand <david@redhat.com>, Paul Burton <pburton@wavecomp.com>, Jan Kiszka <jan.kiszka@web.de>, Igor Mitsyanko <i.mitsyanko@gmail.com>, Joel Stanley <joel@jms.id.au>, Stefan Berger <stefanb@linux.ibm.com>, Peter Maydell <peter.maydell@linaro.org>, Jia Liu <proljc@gmail.com>, Igor Mammedov <imammedo@redhat.com>, Helge Deller <deller@gmx.de>, Fam Zheng <fam@euphon.net>, Jiri Pirko <jiri@resnulli.us>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Magnus Damm <magnus.damm@gmail.com>, Samuel Thibault <samuel.thibault@ens-lyon.org>, Stefan Hajnoczi <stefanha@redhat.com>, Xie Changlong <xiechanglong.d@gmail.com>, Thomas Huth <huth@tuxfamily.org>, Marcelo Tosatti <mtosatti@redhat.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, John Snow <jsnow@redhat.com>, Guan Xuetao <gxt@mprc.pku.edu.cn>, Palmer Dabbelt <palmer@sifive.com>, Gerd Hoffmann <kraxel@redhat.com>, Andrey Smirnov <andrew.smirnov@gmail.com>, Thomas Huth <thuth@redhat.com>, Hannes Reinecke <hare@suse.com>, Jiri Slaby <jslaby@suse.cz>, Kevin Wolf <kwolf@redhat.com>, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>, Subbaraya Sundeep <sundeep.lkml@gmail.com>, Andrew Baumann <Andrew.Baumann@microsoft.com>, Alistair Francis <alistair@alistair23.me>, Rob Herring <robh@kernel.org>, Halil Pasic <pasic@linux.ibm.com>, Paolo Bonzini <pbonzini@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Cornelia Huck <cohuck@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Eduardo Habkost <ehabkost@redhat.com>, Radoslaw Biernacki <radoslaw.biernacki@linaro.org>, Eric Farman <farman@linux.ibm.com>, Wen Congyang <wencongyang2@huawei.com>, Max Reitz <mreitz@redhat.com>, Chris Wulff <crwulff@gmail.com>, Eric Auger <eric.auger@redhat.com>, Greg Kurz <groug@kaod.org>, Paul Durrant <paul.durrant@citrix.com>, Antony Pavlov <antonynpavlov@gmail.com>, Michael Roth <mdroth@linux.vnet.ibm.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Jean-Christophe Dubois <jcd@tribudubois.net>, Tony Krowiak <akrowiak@linux.ibm.com>, Ben Warren <ben@skyportsystems.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Aleksandar Rikalo <arikalo@wavecomp.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Laszlo Ersek <lersek@redhat.com>, Artyom Tarasenko <atar4qemu@gmail.com>
There is a newer version of this series
[Qemu-devel] [PATCH v2 27/29] Include sysemu/sysemu.h a lot less
Posted by Markus Armbruster 6 years, 6 months ago
In my "build everything" tree, changing sysemu/sysemu.h triggers a
recompile of some 5400 out of 6600 objects (not counting tests and
objects that don't depend on qemu/osdep.h).

hw/qdev-core.h includes sysemu/sysemu.h since recent commit e965ffa70a
"qdev: add qdev_add_vm_change_state_handler()".  This is a bad idea:
hw/qdev-core.h is widely included.

Move the declaration of qdev_add_vm_change_state_handler() to
sysemu/sysemu.h, and drop the problematic include from hw/qdev-core.h.

Touching sysemu/sysemu.h now recompiles some 1800 objects.
qemu/uuid.h also drops from 5400 to 1800.  A few more headers show
smaller improvement: qemu/notify.h drops from 5600 to 5200,
qemu/timer.h from 5600 to 4500, and qapi/qapi-types-run-state.h from
5500 to 5000.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 accel/kvm/kvm-all.c               | 1 +
 backends/hostmem.c                | 1 +
 cpus.c                            | 1 +
 hw/arm/allwinner-a10.c            | 1 +
 hw/arm/aspeed_soc.c               | 1 +
 hw/arm/kzm.c                      | 1 +
 hw/arm/msf2-soc.c                 | 1 +
 hw/arm/stm32f205_soc.c            | 1 +
 hw/char/serial-isa.c              | 1 +
 hw/char/xen_console.c             | 1 +
 hw/core/numa.c                    | 1 +
 hw/core/vm-change-state-handler.c | 1 +
 hw/display/qxl-render.c           | 1 +
 hw/i386/xen/xen-hvm.c             | 1 +
 hw/i386/xen/xen-mapcache.c        | 1 +
 hw/intc/ioapic.c                  | 1 +
 hw/pci/pci.c                      | 1 +
 hw/riscv/sifive_e.c               | 1 +
 hw/riscv/sifive_u.c               | 1 +
 hw/riscv/spike.c                  | 1 +
 hw/riscv/virt.c                   | 1 +
 hw/sparc64/niagara.c              | 2 +-
 hw/usb/hcd-ehci.h                 | 1 +
 hw/xen/xen-common.c               | 1 +
 hw/xen/xen_devconfig.c            | 1 +
 hw/xenpv/xen_machine_pv.c         | 1 +
 include/hw/qdev-core.h            | 5 -----
 include/sysemu/sysemu.h           | 3 +++
 migration/global_state.c          | 1 +
 migration/migration.c             | 1 +
 migration/savevm.c                | 1 +
 31 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index e1a44eccf5..fc38d0b9e3 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -29,6 +29,7 @@
 #include "exec/gdbstub.h"
 #include "sysemu/kvm_int.h"
 #include "sysemu/cpus.h"
+#include "sysemu/sysemu.h"
 #include "qemu/bswap.h"
 #include "exec/memory.h"
 #include "exec/ram_addr.h"
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 463102aa15..6d333dc23c 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -12,6 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "sysemu/hostmem.h"
+#include "sysemu/sysemu.h"
 #include "hw/boards.h"
 #include "qapi/error.h"
 #include "qapi/qapi-builtin-visit.h"
diff --git a/cpus.c b/cpus.c
index e70cc58e31..a20a9a29c1 100644
--- a/cpus.c
+++ b/cpus.c
@@ -41,6 +41,7 @@
 #include "sysemu/kvm.h"
 #include "sysemu/hax.h"
 #include "sysemu/hvf.h"
+#include "sysemu/sysemu.h"
 #include "sysemu/whpx.h"
 #include "exec/exec-all.h"
 
diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index 3b0d3eccdd..73810a4440 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -23,6 +23,7 @@
 #include "hw/sysbus.h"
 #include "hw/arm/allwinner-a10.h"
 #include "hw/misc/unimp.h"
+#include "sysemu/sysemu.h"
 
 static void aw_a10_init(Object *obj)
 {
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index c6fb3700f2..9ee8104832 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -22,6 +22,7 @@
 #include "qemu/error-report.h"
 #include "hw/i2c/aspeed_i2c.h"
 #include "net/net.h"
+#include "sysemu/sysemu.h"
 
 #define ASPEED_SOC_IOMEM_SIZE       0x00200000
 
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index 59d2102dc5..2f052e1f8c 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -24,6 +24,7 @@
 #include "hw/net/lan9118.h"
 #include "hw/char/serial.h"
 #include "sysemu/qtest.h"
+#include "sysemu/sysemu.h"
 
 /* Memory map for Kzm Emulation Baseboard:
  * 0x00000000-0x7fffffff See i.MX31 SOC for support
diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
index 8ae763f99f..76cc3e09b0 100644
--- a/hw/arm/msf2-soc.c
+++ b/hw/arm/msf2-soc.c
@@ -30,6 +30,7 @@
 #include "hw/irq.h"
 #include "hw/arm/msf2-soc.h"
 #include "hw/misc/unimp.h"
+#include "sysemu/sysemu.h"
 
 #define MSF2_TIMER_BASE       0x40004000
 #define MSF2_SYSREG_BASE      0x40038000
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index be8b7df679..f5a5c2d80c 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -29,6 +29,7 @@
 #include "exec/address-spaces.h"
 #include "hw/arm/stm32f205_soc.h"
 #include "hw/qdev-properties.h"
+#include "sysemu/sysemu.h"
 
 /* At the moment only Timer 2 to 5 are modelled */
 static const uint32_t timer_addr[STM_NUM_TIMERS] = { 0x40000000, 0x40000400,
diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
index 5a0ae02ee7..9e31c51bb6 100644
--- a/hw/char/serial-isa.c
+++ b/hw/char/serial-isa.c
@@ -26,6 +26,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
+#include "sysemu/sysemu.h"
 #include "hw/char/serial.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev-properties.h"
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 8cc9328b3f..63153dfde4 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -24,6 +24,7 @@
 #include <termios.h>
 
 #include "qapi/error.h"
+#include "sysemu/sysemu.h"
 #include "chardev/char-fe.h"
 #include "hw/xen/xen-legacy-backend.h"
 
diff --git a/hw/core/numa.c b/hw/core/numa.c
index 450c522dd8..7a63ddc4c6 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 #include "sysemu/hostmem.h"
 #include "sysemu/numa.h"
+#include "sysemu/sysemu.h"
 #include "exec/cpu-common.h"
 #include "exec/ramlist.h"
 #include "qemu/bitmap.h"
diff --git a/hw/core/vm-change-state-handler.c b/hw/core/vm-change-state-handler.c
index e27ea45977..9068d51c9a 100644
--- a/hw/core/vm-change-state-handler.c
+++ b/hw/core/vm-change-state-handler.c
@@ -17,6 +17,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/qdev-core.h"
+#include "sysemu/sysemu.h"
 
 static int qdev_get_dev_tree_depth(DeviceState *dev)
 {
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index 14ad2b352d..473e333475 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -21,6 +21,7 @@
 
 #include "qemu/osdep.h"
 #include "qxl.h"
+#include "sysemu/sysemu.h"
 #include "trace.h"
 
 static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 3e15ffc828..ca4659b20f 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -25,6 +25,7 @@
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
 #include "qemu/range.h"
+#include "sysemu/sysemu.h"
 #include "sysemu/xen-mapcache.h"
 #include "trace.h"
 #include "exec/address-spaces.h"
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index dc73c86c61..09656f9f11 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -17,6 +17,7 @@
 #include "hw/xen/xen-legacy-backend.h"
 #include "qemu/bitmap.h"
 
+#include "sysemu/sysemu.h"
 #include "sysemu/xen-mapcache.h"
 #include "trace.h"
 
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index dddd231337..1ede055387 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -30,6 +30,7 @@
 #include "hw/pci/msi.h"
 #include "hw/qdev-properties.h"
 #include "sysemu/kvm.h"
+#include "sysemu/sysemu.h"
 #include "hw/i386/apic-msidef.h"
 #include "hw/i386/x86-iommu.h"
 #include "trace.h"
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 4b6ffab13d..aa05c2b9b2 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -35,6 +35,7 @@
 #include "monitor/monitor.h"
 #include "net/net.h"
 #include "sysemu/numa.h"
+#include "sysemu/sysemu.h"
 #include "hw/loader.h"
 #include "qemu/error-report.h"
 #include "qemu/range.h"
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 862903d6b5..792d75a1a3 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -46,6 +46,7 @@
 #include "hw/riscv/boot.h"
 #include "chardev/char.h"
 #include "sysemu/arch_init.h"
+#include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
 
 static const struct MemmapEntry {
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 0b3c5dff97..9910fa6708 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -44,6 +44,7 @@
 #include "chardev/char.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/device_tree.h"
+#include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
 
 #include <libfdt.h>
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 9cc786b6b6..7c04bd554f 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -40,6 +40,7 @@
 #include "sysemu/arch_init.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/qtest.h"
+#include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
 
 #include <libfdt.h>
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index c72198b720..9bced28486 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -37,6 +37,7 @@
 #include "chardev/char.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/device_tree.h"
+#include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
 #include "hw/pci/pci.h"
 #include "hw/pci-host/gpex.h"
diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
index 1efcff628a..167143bffe 100644
--- a/hw/sparc64/niagara.c
+++ b/hw/sparc64/niagara.c
@@ -35,7 +35,7 @@
 #include "sysemu/block-backend.h"
 #include "qemu/error-report.h"
 #include "sysemu/qtest.h"
-
+#include "sysemu/sysemu.h"
 
 typedef struct NiagaraBoardState {
     MemoryRegion hv_ram;
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 0298238f0b..fdbcfdcbeb 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -21,6 +21,7 @@
 #include "qemu/timer.h"
 #include "hw/usb.h"
 #include "sysemu/dma.h"
+#include "sysemu/sysemu.h"
 #include "hw/pci/pci.h"
 #include "hw/sysbus.h"
 
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index 32503cfc1c..76621da2f5 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -14,6 +14,7 @@
 #include "hw/xen/xen-legacy-backend.h"
 #include "chardev/char.h"
 #include "sysemu/accel.h"
+#include "sysemu/sysemu.h"
 #include "migration/misc.h"
 #include "migration/global_state.h"
 
diff --git a/hw/xen/xen_devconfig.c b/hw/xen/xen_devconfig.c
index 315dbc9c51..46ee4a7f02 100644
--- a/hw/xen/xen_devconfig.c
+++ b/hw/xen/xen_devconfig.c
@@ -2,6 +2,7 @@
 #include "hw/xen/xen-legacy-backend.h"
 #include "qemu/option.h"
 #include "sysemu/blockdev.h"
+#include "sysemu/sysemu.h"
 
 /* ------------------------------------------------------------- */
 
diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
index 524d608eab..3a8af1a1e0 100644
--- a/hw/xenpv/xen_machine_pv.c
+++ b/hw/xenpv/xen_machine_pv.c
@@ -28,6 +28,7 @@
 #include "hw/xen/xen-legacy-backend.h"
 #include "hw/xen/xen-bus.h"
 #include "sysemu/block-backend.h"
+#include "sysemu/sysemu.h"
 
 static void xen_init_pv(MachineState *machine)
 {
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e5b62dd2fc..de70b7a19a 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -5,7 +5,6 @@
 #include "qemu/bitmap.h"
 #include "qom/object.h"
 #include "hw/hotplug.h"
-#include "sysemu/sysemu.h"
 
 enum {
     DEV_NVECTORS_UNSPECIFIED = -1,
@@ -451,8 +450,4 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
 void device_listener_register(DeviceListener *listener);
 void device_listener_unregister(DeviceListener *listener);
 
-VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
-                                                     VMChangeStateHandler *cb,
-                                                     void *opaque);
-
 #endif
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 227202999d..908f158677 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -29,6 +29,9 @@ VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
                                                      void *opaque);
 VMChangeStateEntry *qemu_add_vm_change_state_handler_prio(
         VMChangeStateHandler *cb, void *opaque, int priority);
+VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
+                                                     VMChangeStateHandler *cb,
+                                                     void *opaque);
 void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
 void vm_state_notify(int running, RunState state);
 
diff --git a/migration/global_state.c b/migration/global_state.c
index 2c8c447239..7cba868979 100644
--- a/migration/global_state.c
+++ b/migration/global_state.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
+#include "sysemu/sysemu.h"
 #include "qapi/error.h"
 #include "migration.h"
 #include "migration/global_state.h"
diff --git a/migration/migration.c b/migration/migration.c
index 3a6340f602..2986b8b164 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -21,6 +21,7 @@
 #include "exec.h"
 #include "fd.h"
 #include "socket.h"
+#include "sysemu/sysemu.h"
 #include "rdma.h"
 #include "ram.h"
 #include "migration/global_state.h"
diff --git a/migration/savevm.c b/migration/savevm.c
index b8f734537a..33da39f0ea 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -57,6 +57,7 @@
 #include "io/channel-buffer.h"
 #include "io/channel-file.h"
 #include "sysemu/replay.h"
+#include "sysemu/sysemu.h"
 #include "qjson.h"
 #include "migration/colo.h"
 #include "qemu/bitmap.h"
-- 
2.21.0


Re: [Qemu-devel] [PATCH v2 27/29] Include sysemu/sysemu.h a lot less
Posted by Alistair Francis 6 years, 6 months ago
On Tue, Aug 6, 2019 at 8:22 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> In my "build everything" tree, changing sysemu/sysemu.h triggers a
> recompile of some 5400 out of 6600 objects (not counting tests and
> objects that don't depend on qemu/osdep.h).
>
> hw/qdev-core.h includes sysemu/sysemu.h since recent commit e965ffa70a
> "qdev: add qdev_add_vm_change_state_handler()".  This is a bad idea:
> hw/qdev-core.h is widely included.
>
> Move the declaration of qdev_add_vm_change_state_handler() to
> sysemu/sysemu.h, and drop the problematic include from hw/qdev-core.h.
>
> Touching sysemu/sysemu.h now recompiles some 1800 objects.
> qemu/uuid.h also drops from 5400 to 1800.  A few more headers show
> smaller improvement: qemu/notify.h drops from 5600 to 5200,
> qemu/timer.h from 5600 to 4500, and qapi/qapi-types-run-state.h from
> 5500 to 5000.
>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/kvm/kvm-all.c               | 1 +
>  backends/hostmem.c                | 1 +
>  cpus.c                            | 1 +
>  hw/arm/allwinner-a10.c            | 1 +
>  hw/arm/aspeed_soc.c               | 1 +
>  hw/arm/kzm.c                      | 1 +
>  hw/arm/msf2-soc.c                 | 1 +
>  hw/arm/stm32f205_soc.c            | 1 +
>  hw/char/serial-isa.c              | 1 +
>  hw/char/xen_console.c             | 1 +
>  hw/core/numa.c                    | 1 +
>  hw/core/vm-change-state-handler.c | 1 +
>  hw/display/qxl-render.c           | 1 +
>  hw/i386/xen/xen-hvm.c             | 1 +
>  hw/i386/xen/xen-mapcache.c        | 1 +
>  hw/intc/ioapic.c                  | 1 +
>  hw/pci/pci.c                      | 1 +
>  hw/riscv/sifive_e.c               | 1 +
>  hw/riscv/sifive_u.c               | 1 +
>  hw/riscv/spike.c                  | 1 +
>  hw/riscv/virt.c                   | 1 +
>  hw/sparc64/niagara.c              | 2 +-
>  hw/usb/hcd-ehci.h                 | 1 +
>  hw/xen/xen-common.c               | 1 +
>  hw/xen/xen_devconfig.c            | 1 +
>  hw/xenpv/xen_machine_pv.c         | 1 +
>  include/hw/qdev-core.h            | 5 -----
>  include/sysemu/sysemu.h           | 3 +++
>  migration/global_state.c          | 1 +
>  migration/migration.c             | 1 +
>  migration/savevm.c                | 1 +
>  31 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index e1a44eccf5..fc38d0b9e3 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -29,6 +29,7 @@
>  #include "exec/gdbstub.h"
>  #include "sysemu/kvm_int.h"
>  #include "sysemu/cpus.h"
> +#include "sysemu/sysemu.h"
>  #include "qemu/bswap.h"
>  #include "exec/memory.h"
>  #include "exec/ram_addr.h"
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index 463102aa15..6d333dc23c 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -12,6 +12,7 @@
>
>  #include "qemu/osdep.h"
>  #include "sysemu/hostmem.h"
> +#include "sysemu/sysemu.h"
>  #include "hw/boards.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-builtin-visit.h"
> diff --git a/cpus.c b/cpus.c
> index e70cc58e31..a20a9a29c1 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -41,6 +41,7 @@
>  #include "sysemu/kvm.h"
>  #include "sysemu/hax.h"
>  #include "sysemu/hvf.h"
> +#include "sysemu/sysemu.h"
>  #include "sysemu/whpx.h"
>  #include "exec/exec-all.h"
>
> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
> index 3b0d3eccdd..73810a4440 100644
> --- a/hw/arm/allwinner-a10.c
> +++ b/hw/arm/allwinner-a10.c
> @@ -23,6 +23,7 @@
>  #include "hw/sysbus.h"
>  #include "hw/arm/allwinner-a10.h"
>  #include "hw/misc/unimp.h"
> +#include "sysemu/sysemu.h"
>
>  static void aw_a10_init(Object *obj)
>  {
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index c6fb3700f2..9ee8104832 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -22,6 +22,7 @@
>  #include "qemu/error-report.h"
>  #include "hw/i2c/aspeed_i2c.h"
>  #include "net/net.h"
> +#include "sysemu/sysemu.h"
>
>  #define ASPEED_SOC_IOMEM_SIZE       0x00200000
>
> diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
> index 59d2102dc5..2f052e1f8c 100644
> --- a/hw/arm/kzm.c
> +++ b/hw/arm/kzm.c
> @@ -24,6 +24,7 @@
>  #include "hw/net/lan9118.h"
>  #include "hw/char/serial.h"
>  #include "sysemu/qtest.h"
> +#include "sysemu/sysemu.h"
>
>  /* Memory map for Kzm Emulation Baseboard:
>   * 0x00000000-0x7fffffff See i.MX31 SOC for support
> diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
> index 8ae763f99f..76cc3e09b0 100644
> --- a/hw/arm/msf2-soc.c
> +++ b/hw/arm/msf2-soc.c
> @@ -30,6 +30,7 @@
>  #include "hw/irq.h"
>  #include "hw/arm/msf2-soc.h"
>  #include "hw/misc/unimp.h"
> +#include "sysemu/sysemu.h"
>
>  #define MSF2_TIMER_BASE       0x40004000
>  #define MSF2_SYSREG_BASE      0x40038000
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index be8b7df679..f5a5c2d80c 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -29,6 +29,7 @@
>  #include "exec/address-spaces.h"
>  #include "hw/arm/stm32f205_soc.h"
>  #include "hw/qdev-properties.h"
> +#include "sysemu/sysemu.h"
>
>  /* At the moment only Timer 2 to 5 are modelled */
>  static const uint32_t timer_addr[STM_NUM_TIMERS] = { 0x40000000, 0x40000400,
> diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
> index 5a0ae02ee7..9e31c51bb6 100644
> --- a/hw/char/serial-isa.c
> +++ b/hw/char/serial-isa.c
> @@ -26,6 +26,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu/module.h"
> +#include "sysemu/sysemu.h"
>  #include "hw/char/serial.h"
>  #include "hw/isa/isa.h"
>  #include "hw/qdev-properties.h"
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index 8cc9328b3f..63153dfde4 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -24,6 +24,7 @@
>  #include <termios.h>
>
>  #include "qapi/error.h"
> +#include "sysemu/sysemu.h"
>  #include "chardev/char-fe.h"
>  #include "hw/xen/xen-legacy-backend.h"
>
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 450c522dd8..7a63ddc4c6 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -25,6 +25,7 @@
>  #include "qemu/osdep.h"
>  #include "sysemu/hostmem.h"
>  #include "sysemu/numa.h"
> +#include "sysemu/sysemu.h"
>  #include "exec/cpu-common.h"
>  #include "exec/ramlist.h"
>  #include "qemu/bitmap.h"
> diff --git a/hw/core/vm-change-state-handler.c b/hw/core/vm-change-state-handler.c
> index e27ea45977..9068d51c9a 100644
> --- a/hw/core/vm-change-state-handler.c
> +++ b/hw/core/vm-change-state-handler.c
> @@ -17,6 +17,7 @@
>
>  #include "qemu/osdep.h"
>  #include "hw/qdev-core.h"
> +#include "sysemu/sysemu.h"
>
>  static int qdev_get_dev_tree_depth(DeviceState *dev)
>  {
> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> index 14ad2b352d..473e333475 100644
> --- a/hw/display/qxl-render.c
> +++ b/hw/display/qxl-render.c
> @@ -21,6 +21,7 @@
>
>  #include "qemu/osdep.h"
>  #include "qxl.h"
> +#include "sysemu/sysemu.h"
>  #include "trace.h"
>
>  static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index 3e15ffc828..ca4659b20f 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -25,6 +25,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/range.h"
> +#include "sysemu/sysemu.h"
>  #include "sysemu/xen-mapcache.h"
>  #include "trace.h"
>  #include "exec/address-spaces.h"
> diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
> index dc73c86c61..09656f9f11 100644
> --- a/hw/i386/xen/xen-mapcache.c
> +++ b/hw/i386/xen/xen-mapcache.c
> @@ -17,6 +17,7 @@
>  #include "hw/xen/xen-legacy-backend.h"
>  #include "qemu/bitmap.h"
>
> +#include "sysemu/sysemu.h"
>  #include "sysemu/xen-mapcache.h"
>  #include "trace.h"
>
> diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
> index dddd231337..1ede055387 100644
> --- a/hw/intc/ioapic.c
> +++ b/hw/intc/ioapic.c
> @@ -30,6 +30,7 @@
>  #include "hw/pci/msi.h"
>  #include "hw/qdev-properties.h"
>  #include "sysemu/kvm.h"
> +#include "sysemu/sysemu.h"
>  #include "hw/i386/apic-msidef.h"
>  #include "hw/i386/x86-iommu.h"
>  #include "trace.h"
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 4b6ffab13d..aa05c2b9b2 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -35,6 +35,7 @@
>  #include "monitor/monitor.h"
>  #include "net/net.h"
>  #include "sysemu/numa.h"
> +#include "sysemu/sysemu.h"
>  #include "hw/loader.h"
>  #include "qemu/error-report.h"
>  #include "qemu/range.h"
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 862903d6b5..792d75a1a3 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -46,6 +46,7 @@
>  #include "hw/riscv/boot.h"
>  #include "chardev/char.h"
>  #include "sysemu/arch_init.h"
> +#include "sysemu/sysemu.h"
>  #include "exec/address-spaces.h"
>
>  static const struct MemmapEntry {
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 0b3c5dff97..9910fa6708 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -44,6 +44,7 @@
>  #include "chardev/char.h"
>  #include "sysemu/arch_init.h"
>  #include "sysemu/device_tree.h"
> +#include "sysemu/sysemu.h"
>  #include "exec/address-spaces.h"
>
>  #include <libfdt.h>
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 9cc786b6b6..7c04bd554f 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -40,6 +40,7 @@
>  #include "sysemu/arch_init.h"
>  #include "sysemu/device_tree.h"
>  #include "sysemu/qtest.h"
> +#include "sysemu/sysemu.h"
>  #include "exec/address-spaces.h"
>
>  #include <libfdt.h>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index c72198b720..9bced28486 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -37,6 +37,7 @@
>  #include "chardev/char.h"
>  #include "sysemu/arch_init.h"
>  #include "sysemu/device_tree.h"
> +#include "sysemu/sysemu.h"
>  #include "exec/address-spaces.h"
>  #include "hw/pci/pci.h"
>  #include "hw/pci-host/gpex.h"
> diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
> index 1efcff628a..167143bffe 100644
> --- a/hw/sparc64/niagara.c
> +++ b/hw/sparc64/niagara.c
> @@ -35,7 +35,7 @@
>  #include "sysemu/block-backend.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/qtest.h"
> -
> +#include "sysemu/sysemu.h"
>
>  typedef struct NiagaraBoardState {
>      MemoryRegion hv_ram;
> diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
> index 0298238f0b..fdbcfdcbeb 100644
> --- a/hw/usb/hcd-ehci.h
> +++ b/hw/usb/hcd-ehci.h
> @@ -21,6 +21,7 @@
>  #include "qemu/timer.h"
>  #include "hw/usb.h"
>  #include "sysemu/dma.h"
> +#include "sysemu/sysemu.h"
>  #include "hw/pci/pci.h"
>  #include "hw/sysbus.h"
>
> diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
> index 32503cfc1c..76621da2f5 100644
> --- a/hw/xen/xen-common.c
> +++ b/hw/xen/xen-common.c
> @@ -14,6 +14,7 @@
>  #include "hw/xen/xen-legacy-backend.h"
>  #include "chardev/char.h"
>  #include "sysemu/accel.h"
> +#include "sysemu/sysemu.h"
>  #include "migration/misc.h"
>  #include "migration/global_state.h"
>
> diff --git a/hw/xen/xen_devconfig.c b/hw/xen/xen_devconfig.c
> index 315dbc9c51..46ee4a7f02 100644
> --- a/hw/xen/xen_devconfig.c
> +++ b/hw/xen/xen_devconfig.c
> @@ -2,6 +2,7 @@
>  #include "hw/xen/xen-legacy-backend.h"
>  #include "qemu/option.h"
>  #include "sysemu/blockdev.h"
> +#include "sysemu/sysemu.h"
>
>  /* ------------------------------------------------------------- */
>
> diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
> index 524d608eab..3a8af1a1e0 100644
> --- a/hw/xenpv/xen_machine_pv.c
> +++ b/hw/xenpv/xen_machine_pv.c
> @@ -28,6 +28,7 @@
>  #include "hw/xen/xen-legacy-backend.h"
>  #include "hw/xen/xen-bus.h"
>  #include "sysemu/block-backend.h"
> +#include "sysemu/sysemu.h"
>
>  static void xen_init_pv(MachineState *machine)
>  {
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index e5b62dd2fc..de70b7a19a 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -5,7 +5,6 @@
>  #include "qemu/bitmap.h"
>  #include "qom/object.h"
>  #include "hw/hotplug.h"
> -#include "sysemu/sysemu.h"
>
>  enum {
>      DEV_NVECTORS_UNSPECIFIED = -1,
> @@ -451,8 +450,4 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
>  void device_listener_register(DeviceListener *listener);
>  void device_listener_unregister(DeviceListener *listener);
>
> -VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
> -                                                     VMChangeStateHandler *cb,
> -                                                     void *opaque);
> -
>  #endif
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 227202999d..908f158677 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -29,6 +29,9 @@ VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
>                                                       void *opaque);
>  VMChangeStateEntry *qemu_add_vm_change_state_handler_prio(
>          VMChangeStateHandler *cb, void *opaque, int priority);
> +VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
> +                                                     VMChangeStateHandler *cb,
> +                                                     void *opaque);
>  void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
>  void vm_state_notify(int running, RunState state);
>
> diff --git a/migration/global_state.c b/migration/global_state.c
> index 2c8c447239..7cba868979 100644
> --- a/migration/global_state.c
> +++ b/migration/global_state.c
> @@ -13,6 +13,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/cutils.h"
>  #include "qemu/error-report.h"
> +#include "sysemu/sysemu.h"
>  #include "qapi/error.h"
>  #include "migration.h"
>  #include "migration/global_state.h"
> diff --git a/migration/migration.c b/migration/migration.c
> index 3a6340f602..2986b8b164 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -21,6 +21,7 @@
>  #include "exec.h"
>  #include "fd.h"
>  #include "socket.h"
> +#include "sysemu/sysemu.h"
>  #include "rdma.h"
>  #include "ram.h"
>  #include "migration/global_state.h"
> diff --git a/migration/savevm.c b/migration/savevm.c
> index b8f734537a..33da39f0ea 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -57,6 +57,7 @@
>  #include "io/channel-buffer.h"
>  #include "io/channel-file.h"
>  #include "sysemu/replay.h"
> +#include "sysemu/sysemu.h"
>  #include "qjson.h"
>  #include "migration/colo.h"
>  #include "qemu/bitmap.h"
> --
> 2.21.0
>
>

Re: [Qemu-devel] [PATCH v2 27/29] Include sysemu/sysemu.h a lot less
Posted by Stefan Hajnoczi 6 years, 6 months ago
On Tue, Aug 06, 2019 at 05:14:33PM +0200, Markus Armbruster wrote:
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index e5b62dd2fc..de70b7a19a 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -5,7 +5,6 @@
>  #include "qemu/bitmap.h"
>  #include "qom/object.h"
>  #include "hw/hotplug.h"
> -#include "sysemu/sysemu.h"
>  
>  enum {
>      DEV_NVECTORS_UNSPECIFIED = -1,
> @@ -451,8 +450,4 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
>  void device_listener_register(DeviceListener *listener);
>  void device_listener_unregister(DeviceListener *listener);
>  
> -VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
> -                                                     VMChangeStateHandler *cb,
> -                                                     void *opaque);
> -
>  #endif
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 227202999d..908f158677 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -29,6 +29,9 @@ VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
>                                                       void *opaque);
>  VMChangeStateEntry *qemu_add_vm_change_state_handler_prio(
>          VMChangeStateHandler *cb, void *opaque, int priority);
> +VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
> +                                                     VMChangeStateHandler *cb,
> +                                                     void *opaque);
>  void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
>  void vm_state_notify(int running, RunState state);
>  

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH v2 27/29] Include sysemu/sysemu.h a lot less
Posted by Philippe Mathieu-Daudé 6 years, 6 months ago
On 8/6/19 5:14 PM, Markus Armbruster wrote:
> In my "build everything" tree, changing sysemu/sysemu.h triggers a
> recompile of some 5400 out of 6600 objects (not counting tests and
> objects that don't depend on qemu/osdep.h).
> 
> hw/qdev-core.h includes sysemu/sysemu.h since recent commit e965ffa70a
> "qdev: add qdev_add_vm_change_state_handler()".  This is a bad idea:
> hw/qdev-core.h is widely included.
> 
> Move the declaration of qdev_add_vm_change_state_handler() to
> sysemu/sysemu.h, and drop the problematic include from hw/qdev-core.h.
> 
> Touching sysemu/sysemu.h now recompiles some 1800 objects.
> qemu/uuid.h also drops from 5400 to 1800.  A few more headers show
> smaller improvement: qemu/notify.h drops from 5600 to 5200,
> qemu/timer.h from 5600 to 4500, and qapi/qapi-types-run-state.h from
> 5500 to 5000.
> 
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  accel/kvm/kvm-all.c               | 1 +
>  backends/hostmem.c                | 1 +
>  cpus.c                            | 1 +
>  hw/arm/allwinner-a10.c            | 1 +
>  hw/arm/aspeed_soc.c               | 1 +
>  hw/arm/kzm.c                      | 1 +
>  hw/arm/msf2-soc.c                 | 1 +
>  hw/arm/stm32f205_soc.c            | 1 +
>  hw/char/serial-isa.c              | 1 +
>  hw/char/xen_console.c             | 1 +
>  hw/core/numa.c                    | 1 +
>  hw/core/vm-change-state-handler.c | 1 +
>  hw/display/qxl-render.c           | 1 +
>  hw/i386/xen/xen-hvm.c             | 1 +
>  hw/i386/xen/xen-mapcache.c        | 1 +
>  hw/intc/ioapic.c                  | 1 +
>  hw/pci/pci.c                      | 1 +
>  hw/riscv/sifive_e.c               | 1 +
>  hw/riscv/sifive_u.c               | 1 +
>  hw/riscv/spike.c                  | 1 +
>  hw/riscv/virt.c                   | 1 +
>  hw/sparc64/niagara.c              | 2 +-
>  hw/usb/hcd-ehci.h                 | 1 +
>  hw/xen/xen-common.c               | 1 +
>  hw/xen/xen_devconfig.c            | 1 +
>  hw/xenpv/xen_machine_pv.c         | 1 +
>  include/hw/qdev-core.h            | 5 -----
>  include/sysemu/sysemu.h           | 3 +++
>  migration/global_state.c          | 1 +
>  migration/migration.c             | 1 +
>  migration/savevm.c                | 1 +
>  31 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index e1a44eccf5..fc38d0b9e3 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -29,6 +29,7 @@
>  #include "exec/gdbstub.h"
>  #include "sysemu/kvm_int.h"
>  #include "sysemu/cpus.h"
> +#include "sysemu/sysemu.h"
>  #include "qemu/bswap.h"
>  #include "exec/memory.h"
>  #include "exec/ram_addr.h"

Include missing in net/netmap.c:

  CC      net/netmap.o
net/netmap.c: In function 'netmap_update_fd_handler':
net/netmap.c:108:5: error: implicit declaration of function
'qemu_set_fd_handler' [-Werror=implicit-function-declaration]
     qemu_set_fd_handler(s->nmd->fd,
     ^~~~~~~~~~~~~~~~~~~
net/netmap.c:108:5: error: nested extern declaration of
'qemu_set_fd_handler' [-Werror=nested-externs]

Re: [Qemu-devel] [PATCH v2 27/29] Include sysemu/sysemu.h a lot less
Posted by Markus Armbruster 6 years, 6 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 8/6/19 5:14 PM, Markus Armbruster wrote:
>> In my "build everything" tree, changing sysemu/sysemu.h triggers a
>> recompile of some 5400 out of 6600 objects (not counting tests and
>> objects that don't depend on qemu/osdep.h).
>> 
>> hw/qdev-core.h includes sysemu/sysemu.h since recent commit e965ffa70a
>> "qdev: add qdev_add_vm_change_state_handler()".  This is a bad idea:
>> hw/qdev-core.h is widely included.
>> 
>> Move the declaration of qdev_add_vm_change_state_handler() to
>> sysemu/sysemu.h, and drop the problematic include from hw/qdev-core.h.
>> 
>> Touching sysemu/sysemu.h now recompiles some 1800 objects.
>> qemu/uuid.h also drops from 5400 to 1800.  A few more headers show
>> smaller improvement: qemu/notify.h drops from 5600 to 5200,
>> qemu/timer.h from 5600 to 4500, and qapi/qapi-types-run-state.h from
>> 5500 to 5000.
>> 
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  accel/kvm/kvm-all.c               | 1 +
>>  backends/hostmem.c                | 1 +
>>  cpus.c                            | 1 +
>>  hw/arm/allwinner-a10.c            | 1 +
>>  hw/arm/aspeed_soc.c               | 1 +
>>  hw/arm/kzm.c                      | 1 +
>>  hw/arm/msf2-soc.c                 | 1 +
>>  hw/arm/stm32f205_soc.c            | 1 +
>>  hw/char/serial-isa.c              | 1 +
>>  hw/char/xen_console.c             | 1 +
>>  hw/core/numa.c                    | 1 +
>>  hw/core/vm-change-state-handler.c | 1 +
>>  hw/display/qxl-render.c           | 1 +
>>  hw/i386/xen/xen-hvm.c             | 1 +
>>  hw/i386/xen/xen-mapcache.c        | 1 +
>>  hw/intc/ioapic.c                  | 1 +
>>  hw/pci/pci.c                      | 1 +
>>  hw/riscv/sifive_e.c               | 1 +
>>  hw/riscv/sifive_u.c               | 1 +
>>  hw/riscv/spike.c                  | 1 +
>>  hw/riscv/virt.c                   | 1 +
>>  hw/sparc64/niagara.c              | 2 +-
>>  hw/usb/hcd-ehci.h                 | 1 +
>>  hw/xen/xen-common.c               | 1 +
>>  hw/xen/xen_devconfig.c            | 1 +
>>  hw/xenpv/xen_machine_pv.c         | 1 +
>>  include/hw/qdev-core.h            | 5 -----
>>  include/sysemu/sysemu.h           | 3 +++
>>  migration/global_state.c          | 1 +
>>  migration/migration.c             | 1 +
>>  migration/savevm.c                | 1 +
>>  31 files changed, 32 insertions(+), 6 deletions(-)
>> 
>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> index e1a44eccf5..fc38d0b9e3 100644
>> --- a/accel/kvm/kvm-all.c
>> +++ b/accel/kvm/kvm-all.c
>> @@ -29,6 +29,7 @@
>>  #include "exec/gdbstub.h"
>>  #include "sysemu/kvm_int.h"
>>  #include "sysemu/cpus.h"
>> +#include "sysemu/sysemu.h"
>>  #include "qemu/bswap.h"
>>  #include "exec/memory.h"
>>  #include "exec/ram_addr.h"
>
> Include missing in net/netmap.c:
>
>   CC      net/netmap.o
> net/netmap.c: In function 'netmap_update_fd_handler':
> net/netmap.c:108:5: error: implicit declaration of function
> 'qemu_set_fd_handler' [-Werror=implicit-function-declaration]
>      qemu_set_fd_handler(s->nmd->fd,
>      ^~~~~~~~~~~~~~~~~~~
> net/netmap.c:108:5: error: nested extern declaration of
> 'qemu_set_fd_handler' [-Werror=nested-externs]

Can you tell me offhand what I have to install so configure enables
CONFIG_NETMAP?

Re: [Qemu-devel] [PATCH v2 27/29] Include sysemu/sysemu.h a lot less
Posted by Philippe Mathieu-Daudé 6 years, 6 months ago
On 8/7/19 10:16 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> On 8/6/19 5:14 PM, Markus Armbruster wrote:
>>> In my "build everything" tree, changing sysemu/sysemu.h triggers a
>>> recompile of some 5400 out of 6600 objects (not counting tests and
>>> objects that don't depend on qemu/osdep.h).
>>>
>>> hw/qdev-core.h includes sysemu/sysemu.h since recent commit e965ffa70a
>>> "qdev: add qdev_add_vm_change_state_handler()".  This is a bad idea:
>>> hw/qdev-core.h is widely included.
>>>
>>> Move the declaration of qdev_add_vm_change_state_handler() to
>>> sysemu/sysemu.h, and drop the problematic include from hw/qdev-core.h.
>>>
>>> Touching sysemu/sysemu.h now recompiles some 1800 objects.
>>> qemu/uuid.h also drops from 5400 to 1800.  A few more headers show
>>> smaller improvement: qemu/notify.h drops from 5600 to 5200,
>>> qemu/timer.h from 5600 to 4500, and qapi/qapi-types-run-state.h from
>>> 5500 to 5000.
>>>
>>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>  accel/kvm/kvm-all.c               | 1 +
>>>  backends/hostmem.c                | 1 +
>>>  cpus.c                            | 1 +
>>>  hw/arm/allwinner-a10.c            | 1 +
>>>  hw/arm/aspeed_soc.c               | 1 +
>>>  hw/arm/kzm.c                      | 1 +
>>>  hw/arm/msf2-soc.c                 | 1 +
>>>  hw/arm/stm32f205_soc.c            | 1 +
>>>  hw/char/serial-isa.c              | 1 +
>>>  hw/char/xen_console.c             | 1 +
>>>  hw/core/numa.c                    | 1 +
>>>  hw/core/vm-change-state-handler.c | 1 +
>>>  hw/display/qxl-render.c           | 1 +
>>>  hw/i386/xen/xen-hvm.c             | 1 +
>>>  hw/i386/xen/xen-mapcache.c        | 1 +
>>>  hw/intc/ioapic.c                  | 1 +
>>>  hw/pci/pci.c                      | 1 +
>>>  hw/riscv/sifive_e.c               | 1 +
>>>  hw/riscv/sifive_u.c               | 1 +
>>>  hw/riscv/spike.c                  | 1 +
>>>  hw/riscv/virt.c                   | 1 +
>>>  hw/sparc64/niagara.c              | 2 +-
>>>  hw/usb/hcd-ehci.h                 | 1 +
>>>  hw/xen/xen-common.c               | 1 +
>>>  hw/xen/xen_devconfig.c            | 1 +
>>>  hw/xenpv/xen_machine_pv.c         | 1 +
>>>  include/hw/qdev-core.h            | 5 -----
>>>  include/sysemu/sysemu.h           | 3 +++
>>>  migration/global_state.c          | 1 +
>>>  migration/migration.c             | 1 +
>>>  migration/savevm.c                | 1 +
>>>  31 files changed, 32 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>>> index e1a44eccf5..fc38d0b9e3 100644
>>> --- a/accel/kvm/kvm-all.c
>>> +++ b/accel/kvm/kvm-all.c
>>> @@ -29,6 +29,7 @@
>>>  #include "exec/gdbstub.h"
>>>  #include "sysemu/kvm_int.h"
>>>  #include "sysemu/cpus.h"
>>> +#include "sysemu/sysemu.h"
>>>  #include "qemu/bswap.h"
>>>  #include "exec/memory.h"
>>>  #include "exec/ram_addr.h"
>>
>> Include missing in net/netmap.c:
>>
>>   CC      net/netmap.o
>> net/netmap.c: In function 'netmap_update_fd_handler':
>> net/netmap.c:108:5: error: implicit declaration of function
>> 'qemu_set_fd_handler' [-Werror=implicit-function-declaration]
>>      qemu_set_fd_handler(s->nmd->fd,
>>      ^~~~~~~~~~~~~~~~~~~
>> net/netmap.c:108:5: error: nested extern declaration of
>> 'qemu_set_fd_handler' [-Werror=nested-externs]
> 
> Can you tell me offhand what I have to install so configure enables
> CONFIG_NETMAP?

The steps are listed in tests/docker/dockerfiles/debian-amd64.docker,
but you can get to this point running:

  $ make docker-image-debian-amd64 V=1 DEBUG=1

This will build the docker image with netmap (so you don't have to mess
with your workstation setup), then build QEMU within the image.

[Qemu-devel] Is network backend netmap worth keeping? (was: [PATCH v2 27/29] Include sysemu/sysemu.h a lot less)
Posted by Markus Armbruster 6 years, 6 months ago
Please excuse the attention-grabbing subject.

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 8/7/19 10:16 PM, Markus Armbruster wrote:
[...]
>> Can you tell me offhand what I have to install so configure enables
>> CONFIG_NETMAP?
>
> The steps are listed in tests/docker/dockerfiles/debian-amd64.docker,
> but you can get to this point running:
>
>   $ make docker-image-debian-amd64 V=1 DEBUG=1
>
> This will build the docker image with netmap (so you don't have to mess
> with your workstation setup), then build QEMU within the image.

So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
build and install netmap software from sources.  Which pretty much
ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
The commit message points to <http://info.iet.unipi.it/~luigi/netmap/>,
which gives me "connection timed out" right now.

On the other hand, it's covered in MAINTAINERS, and has seen
non-janitorial activity as late as Dec 2018 (commit c693fc748a).

Luigi, Giuseppe, Vincenzo, what's the status of the netmap project?

Why is the QEMU netmap backend worth keeping?

Who is using the netmap backend?

How do they obtain a netmap-enabled QEMU?  Compile it from sources
themselves?

Would it make sense to have netmap packaged in common Linux distros?

Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Jason Wang 6 years, 6 months ago
On 2019/8/8 下午12:48, Markus Armbruster wrote:
> Please excuse the attention-grabbing subject.
>
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>
>> On 8/7/19 10:16 PM, Markus Armbruster wrote:
> [...]
>>> Can you tell me offhand what I have to install so configure enables
>>> CONFIG_NETMAP?
>> The steps are listed in tests/docker/dockerfiles/debian-amd64.docker,
>> but you can get to this point running:
>>
>>    $ make docker-image-debian-amd64 V=1 DEBUG=1
>>
>> This will build the docker image with netmap (so you don't have to mess
>> with your workstation setup), then build QEMU within the image.
> So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
> build and install netmap software from sources.  Which pretty much
> ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
> The commit message points to <http://info.iet.unipi.it/~luigi/netmap/>,
> which gives me "connection timed out" right now.
>
> On the other hand, it's covered in MAINTAINERS, and has seen
> non-janitorial activity as late as Dec 2018 (commit c693fc748a).
>
> Luigi, Giuseppe, Vincenzo, what's the status of the netmap project?
>
> Why is the QEMU netmap backend worth keeping?
>
> Who is using the netmap backend?


Netmap was supported by freebsd: 
https://www.freebsd.org/cgi/man.cgi?query=netmap&sektion=4. So I guess 
there should be real users.


>
> How do they obtain a netmap-enabled QEMU?  Compile it from sources
> themselves?


Yes.


>
> Would it make sense to have netmap packaged in common Linux distros?
>

It requires Linux kernel support which is tough task especially Linux 
has AF_XDP support recently.

Thanks


Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Philippe Mathieu-Daudé 6 years, 6 months ago
On 8/8/19 7:38 AM, Jason Wang wrote:
> 
> On 2019/8/8 下午12:48, Markus Armbruster wrote:
>> Please excuse the attention-grabbing subject.
>>
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>
>>> On 8/7/19 10:16 PM, Markus Armbruster wrote:
>> [...]
>>>> Can you tell me offhand what I have to install so configure enables
>>>> CONFIG_NETMAP?
>>> The steps are listed in tests/docker/dockerfiles/debian-amd64.docker,
>>> but you can get to this point running:
>>>
>>>    $ make docker-image-debian-amd64 V=1 DEBUG=1
>>>
>>> This will build the docker image with netmap (so you don't have to mess
>>> with your workstation setup), then build QEMU within the image.
>> So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
>> build and install netmap software from sources.  Which pretty much
>> ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
>> The commit message points to <http://info.iet.unipi.it/~luigi/netmap/>,
>> which gives me "connection timed out" right now.
>>
>> On the other hand, it's covered in MAINTAINERS, and has seen
>> non-janitorial activity as late as Dec 2018 (commit c693fc748a).
>>
>> Luigi, Giuseppe, Vincenzo, what's the status of the netmap project?
>>
>> Why is the QEMU netmap backend worth keeping?
>>
>> Who is using the netmap backend?
> 
> 
> Netmap was supported by freebsd:
> https://www.freebsd.org/cgi/man.cgi?query=netmap&sektion=4. So I guess
> there should be real users.
> 
> 
>>
>> How do they obtain a netmap-enabled QEMU?  Compile it from sources
>> themselves?
> 
> 
> Yes.

Hmm at least on the FreeBSD setup by vmtest (12.0-RELEASE r341666) we
don't need to build it from source:

$ make vm-build-freebsd V=1 DEBUG=1
[...]
netmap support    yes
[...]

$ fgrep -r CONFIG_NETMAP .
./config-host.mak:CONFIG_NETMAP=y

Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Vincenzo Maffione 6 years, 6 months ago
Yes, indeed.
Netmap is actively maintained on FreeBSD, and QEMU is packaged on FreeBSD
with netmap support enabled.
Also keep in mind that, differently from Linux, the (current) tap driver on
FreeBSD does not support offloads (e.g. IFF_VNET_HDR, TUNSETVNETHDRSIZE and
so on). On the contrary, netmap (VALE) supports the same offloads as tap
does on Linux. This makes inter-VM networking with VALE more appealing on
FreeBSD.

Cheers,
  Vincenzo

Il giorno gio 8 ago 2019 alle ore 15:36 Philippe Mathieu-Daudé <
philmd@redhat.com> ha scritto:

> On 8/8/19 7:38 AM, Jason Wang wrote:
> >
> > On 2019/8/8 下午12:48, Markus Armbruster wrote:
> >> Please excuse the attention-grabbing subject.
> >>
> >> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> >>
> >>> On 8/7/19 10:16 PM, Markus Armbruster wrote:
> >> [...]
> >>>> Can you tell me offhand what I have to install so configure enables
> >>>> CONFIG_NETMAP?
> >>> The steps are listed in tests/docker/dockerfiles/debian-amd64.docker,
> >>> but you can get to this point running:
> >>>
> >>>    $ make docker-image-debian-amd64 V=1 DEBUG=1
> >>>
> >>> This will build the docker image with netmap (so you don't have to mess
> >>> with your workstation setup), then build QEMU within the image.
> >> So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
> >> build and install netmap software from sources.  Which pretty much
> >> ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
> >> The commit message points to <http://info.iet.unipi.it/~luigi/netmap/>,
> >> which gives me "connection timed out" right now.
> >>
> >> On the other hand, it's covered in MAINTAINERS, and has seen
> >> non-janitorial activity as late as Dec 2018 (commit c693fc748a).
> >>
> >> Luigi, Giuseppe, Vincenzo, what's the status of the netmap project?
> >>
> >> Why is the QEMU netmap backend worth keeping?
> >>
> >> Who is using the netmap backend?
> >
> >
> > Netmap was supported by freebsd:
> > https://www.freebsd.org/cgi/man.cgi?query=netmap&sektion=4. So I guess
> > there should be real users.
> >
> >
> >>
> >> How do they obtain a netmap-enabled QEMU?  Compile it from sources
> >> themselves?
> >
> >
> > Yes.
>
> Hmm at least on the FreeBSD setup by vmtest (12.0-RELEASE r341666) we
> don't need to build it from source:
>
> $ make vm-build-freebsd V=1 DEBUG=1
> [...]
> netmap support    yes
> [...]
>
> $ fgrep -r CONFIG_NETMAP .
> ./config-host.mak:CONFIG_NETMAP=y
>
Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Giuseppe Lettieri 6 years, 6 months ago
Dear Markus,

the netmap project is alive and well, if a bit understuffed. We have 
moved to github:

https://github.com/luigirizzo/netmap

We have users from FreeBSD, where it is part of the official kernel, and 
Linux, both from Academia and industry.

But you asked about the netmap backend in QEMU, in particular. When it 
was merged, the decision was made to disable it by default because it 
was not supported upstream in Linux. As Jason Wang says, this support is 
even more unlikely now than it was then.

The fact the the backend has to be explicitly enabled and built from the 
sources has obviously cut down the number of potential users. However, 
we still think it is useful and we have pending updates for it. If it's 
causing problems in the workflow, I am willing to help as much as I can.

Cheers,
Giuseppe

Il 08/08/19 06:48, Markus Armbruster ha scritto:
> Please excuse the attention-grabbing subject.
> 
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> On 8/7/19 10:16 PM, Markus Armbruster wrote:
> [...]
>>> Can you tell me offhand what I have to install so configure enables
>>> CONFIG_NETMAP?
>>
>> The steps are listed in tests/docker/dockerfiles/debian-amd64.docker,
>> but you can get to this point running:
>>
>>    $ make docker-image-debian-amd64 V=1 DEBUG=1
>>
>> This will build the docker image with netmap (so you don't have to mess
>> with your workstation setup), then build QEMU within the image.
> 
> So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
> build and install netmap software from sources.  Which pretty much
> ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
> The commit message points to <http://info.iet.unipi.it/~luigi/netmap/>,
> which gives me "connection timed out" right now.
> 
> On the other hand, it's covered in MAINTAINERS, and has seen
> non-janitorial activity as late as Dec 2018 (commit c693fc748a).
> 
> Luigi, Giuseppe, Vincenzo, what's the status of the netmap project?
> 
> Why is the QEMU netmap backend worth keeping?
> 
> Who is using the netmap backend?
> 
> How do they obtain a netmap-enabled QEMU?  Compile it from sources
> themselves?
> 
> Would it make sense to have netmap packaged in common Linux distros?
> 


-- 
Dr. Ing. Giuseppe Lettieri
Dipartimento di Ingegneria della Informazione
Universita' di Pisa
Largo Lucio Lazzarino 1, 56122 Pisa - Italy
Ph. : (+39) 050-2217.649 (direct) .599 (switch)
Fax : (+39) 050-2217.600
e-mail: g.lettieri@iet.unipi.it

Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Markus Armbruster 6 years, 6 months ago
Giuseppe Lettieri <giuseppe.lettieri@unipi.it> writes:

> Dear Markus,
>
> the netmap project is alive and well, if a bit understuffed. We have
> moved to github:
>
> https://github.com/luigirizzo/netmap
>
> We have users from FreeBSD, where it is part of the official kernel,
> and Linux, both from Academia and industry.
>
> But you asked about the netmap backend in QEMU, in particular. When it
> was merged, the decision was made to disable it by default because it
> was not supported upstream in Linux. As Jason Wang says, this support
> is even more unlikely now than it was then.
>
> The fact the the backend has to be explicitly enabled and built from
> the sources has obviously cut down the number of potential
> users. However, we still think it is useful and we have pending
> updates for it. If it's causing problems in the workflow, I am willing
> to help as much as I can.

Could we make it a submodule, simililar to slirp and capstone?

    --enable-netmap=system      use the system's netmap
    --enable-netmap=git         use the git submodule
    --enable-netmap             use system's, else git, else fail
    --disable-netmap            disable netmap
    default                     use system's, else git, else disable

A fresh clone of https://github.com/luigirizzo/netmap clocks in at
14MiB, which is between libslirp's 1.5MiB and capstone's 72MiB.

Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Philippe Mathieu-Daudé 6 years, 6 months ago
On 8/8/19 1:52 PM, Markus Armbruster wrote:
> Giuseppe Lettieri <giuseppe.lettieri@unipi.it> writes:
> 
>> Dear Markus,
>>
>> the netmap project is alive and well, if a bit understuffed. We have
>> moved to github:
>>
>> https://github.com/luigirizzo/netmap
>>
>> We have users from FreeBSD, where it is part of the official kernel,
>> and Linux, both from Academia and industry.
>>
>> But you asked about the netmap backend in QEMU, in particular. When it
>> was merged, the decision was made to disable it by default because it
>> was not supported upstream in Linux. As Jason Wang says, this support
>> is even more unlikely now than it was then.
>>
>> The fact the the backend has to be explicitly enabled and built from
>> the sources has obviously cut down the number of potential
>> users. However, we still think it is useful and we have pending
>> updates for it. If it's causing problems in the workflow, I am willing
>> to help as much as I can.
> 
> Could we make it a submodule, simililar to slirp and capstone?

Good idea, this would extend the coverage. Netmap users/developers are
probably best suited to do this.

> 
>     --enable-netmap=system      use the system's netmap
>     --enable-netmap=git         use the git submodule
>     --enable-netmap             use system's, else git, else fail
>     --disable-netmap            disable netmap
>     default                     use system's, else git, else disable
> 
> A fresh clone of https://github.com/luigirizzo/netmap clocks in at
> 14MiB, which is between libslirp's 1.5MiB and capstone's 72MiB.

In which directory should we clone it? As /netmap directly?

Should we start using a 3rd-party/ subdirectory?

Similarly, what about the virglrenderer component?

Its repository is: https://anongit.freedesktop.org/git/virglrenderer.git

Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Philippe Mathieu-Daudé 6 years, 6 months ago
I forgot to Cc Thomas, who did a lot of directory refactoring in the past.

On Mon, Aug 12, 2019 at 2:32 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
> On 8/8/19 1:52 PM, Markus Armbruster wrote:
> > Giuseppe Lettieri <giuseppe.lettieri@unipi.it> writes:
> >
> >> Dear Markus,
> >>
> >> the netmap project is alive and well, if a bit understuffed. We have
> >> moved to github:
> >>
> >> https://github.com/luigirizzo/netmap
> >>
> >> We have users from FreeBSD, where it is part of the official kernel,
> >> and Linux, both from Academia and industry.
> >>
> >> But you asked about the netmap backend in QEMU, in particular. When it
> >> was merged, the decision was made to disable it by default because it
> >> was not supported upstream in Linux. As Jason Wang says, this support
> >> is even more unlikely now than it was then.
> >>
> >> The fact the the backend has to be explicitly enabled and built from
> >> the sources has obviously cut down the number of potential
> >> users. However, we still think it is useful and we have pending
> >> updates for it. If it's causing problems in the workflow, I am willing
> >> to help as much as I can.
> >
> > Could we make it a submodule, simililar to slirp and capstone?
>
> Good idea, this would extend the coverage. Netmap users/developers are
> probably best suited to do this.
>
> >
> >     --enable-netmap=system      use the system's netmap
> >     --enable-netmap=git         use the git submodule
> >     --enable-netmap             use system's, else git, else fail
> >     --disable-netmap            disable netmap
> >     default                     use system's, else git, else disable
> >
> > A fresh clone of https://github.com/luigirizzo/netmap clocks in at
> > 14MiB, which is between libslirp's 1.5MiB and capstone's 72MiB.
>
> In which directory should we clone it? As /netmap directly?
>
> Should we start using a 3rd-party/ subdirectory?
>
> Similarly, what about the virglrenderer component?
>
> Its repository is: https://anongit.freedesktop.org/git/virglrenderer.git

Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Giuseppe Lettieri 6 years, 5 months ago
Hi all,

I have been thinking of the submodule suggestion and I have also 
prepared a patch for it (attached). However, I am not sure about what we 
want to achieve with it. In particular, I am not sure that the option is 
useful for end users. The problem is that netmap, unlike capstone and 
slirp, is a library plus a kernel module. If netmap is not installed in 
the system, the --enable-netmap=git option will successfully build qemu 
with netmap support, but then -netdev netmap will fail at runtime. On 
the other end, if netmap is installed in the system, using the =git 
option may cause qemu to be built against a different, possibly 
incompatible version.

If the option is only useful for developers to check that some qemu 
change does not break anything, then probably it should be enabled in 
some other, less visible way. What do you think?

Cheers,

Giuseppe

On 8/12/19 2:32 PM, Philippe Mathieu-Daudé wrote:

> On 8/8/19 1:52 PM, Markus Armbruster wrote:
>> Giuseppe Lettieri <giuseppe.lettieri@unipi.it> writes:
>>
>>> Dear Markus,
>>>
>>> the netmap project is alive and well, if a bit understuffed. We have
>>> moved to github:
>>>
>>> https://github.com/luigirizzo/netmap
>>>
>>> We have users from FreeBSD, where it is part of the official kernel,
>>> and Linux, both from Academia and industry.
>>>
>>> But you asked about the netmap backend in QEMU, in particular. When it
>>> was merged, the decision was made to disable it by default because it
>>> was not supported upstream in Linux. As Jason Wang says, this support
>>> is even more unlikely now than it was then.
>>>
>>> The fact the the backend has to be explicitly enabled and built from
>>> the sources has obviously cut down the number of potential
>>> users. However, we still think it is useful and we have pending
>>> updates for it. If it's causing problems in the workflow, I am willing
>>> to help as much as I can.
>> Could we make it a submodule, simililar to slirp and capstone?
> Good idea, this would extend the coverage. Netmap users/developers are
> probably best suited to do this.
>
>>      --enable-netmap=system      use the system's netmap
>>      --enable-netmap=git         use the git submodule
>>      --enable-netmap             use system's, else git, else fail
>>      --disable-netmap            disable netmap
>>      default                     use system's, else git, else disable
>>
>> A fresh clone of https://github.com/luigirizzo/netmap clocks in at
>> 14MiB, which is between libslirp's 1.5MiB and capstone's 72MiB.
> In which directory should we clone it? As /netmap directly?
>
> Should we start using a 3rd-party/ subdirectory?
>
> Similarly, what about the virglrenderer component?
>
> Its repository is: https://anongit.freedesktop.org/git/virglrenderer.git
Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Markus Armbruster 6 years, 4 months ago
Giuseppe Lettieri <giuseppe.lettieri@unipi.it> writes:

> Hi all,
>
> I have been thinking of the submodule suggestion and I have also
> prepared a patch for it (attached). However, I am not sure about what
> we want to achieve with it. In particular, I am not sure that the
> option is useful for end users. The problem is that netmap, unlike
> capstone and slirp, is a library plus a kernel module. If netmap is
> not installed in the system, the --enable-netmap=git option will
> successfully build qemu with netmap support, but then -netdev netmap
> will fail at runtime.

Yes.

What happens when I build with --enable-netmap=system on host A, then
run the resulting binary on some host B that doesn't have netmap
installed?

>                       On the other end, if netmap is installed in the
> system, using the =git option may cause qemu to be built against a
> different, possibly incompatible version.

Yes.  We default to netmap=system, though.  If you break things by
passing arcane arguments to configure, you get to keep the pieces :)

> If the option is only useful for developers to check that some qemu
> change does not break anything, then probably it should be enabled in
> some other, less visible way. What do you think?

I think an --enable-netmap patterned after --enable-capstone and
--enable-slirp has sufficiently low visibility as long as the default is
sane.

We clearly want configure to pick netmap=system when the system provides
netmap.

What shall configure pick when the system doesn't provide it?  If you
think netmap=git is too dangerous for general audience, consider
disabling netmap then.  Experts can still compile-test with
--enable-netmap=git.  Our CI certainly should.

Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Giuseppe Lettieri 6 years, 4 months ago
Il 13/09/19 15:04, Markus Armbruster ha scritto:
> 
> What happens when I build with --enable-netmap=system on host A, then
> run the resulting binary on some host B that doesn't have netmap
> installed?
> 

Qemu will fail at startup, complaining that /dev/netmap does not exists.


> 
> Yes.  We default to netmap=system, though.  If you break things by
> passing arcane arguments to configure, you get to keep the pieces :)
> 
>> If the option is only useful for developers to check that some qemu
>> change does not break anything, then probably it should be enabled in
>> some other, less visible way. What do you think?
> 
> I think an --enable-netmap patterned after --enable-capstone and
> --enable-slirp has sufficiently low visibility as long as the default is
> sane.
> 
> We clearly want configure to pick netmap=system when the system provides
> netmap.
> 
> What shall configure pick when the system doesn't provide it?  If you
> think netmap=git is too dangerous for general audience, consider
> disabling netmap then.  Experts can still compile-test with
> --enable-netmap=git.  Our CI certainly should.
> 

OK, sounds reasonable. The attached patch will select system if netmap 
is available, and git only if explicitly requested.

Cheers,
Giuseppe

-- 
Dr. Ing. Giuseppe Lettieri
Dipartimento di Ingegneria della Informazione
Universita' di Pisa
Largo Lucio Lazzarino 1, 56122 Pisa - Italy
Ph. : (+39) 050-2217.649 (direct) .599 (switch)
Fax : (+39) 050-2217.600
e-mail: g.lettieri@iet.unipi.it
Re: [Qemu-devel] Is network backend netmap worth keeping?
Posted by Markus Armbruster 6 years, 4 months ago
Giuseppe Lettieri <giuseppe.lettieri@unipi.it> writes:

> Il 13/09/19 15:04, Markus Armbruster ha scritto:
>>
>> What happens when I build with --enable-netmap=system on host A, then
>> run the resulting binary on some host B that doesn't have netmap
>> installed?
>>
>
> Qemu will fail at startup, complaining that /dev/netmap does not exists.
>
>
>>
>> Yes.  We default to netmap=system, though.  If you break things by
>> passing arcane arguments to configure, you get to keep the pieces :)
>>
>>> If the option is only useful for developers to check that some qemu
>>> change does not break anything, then probably it should be enabled in
>>> some other, less visible way. What do you think?
>>
>> I think an --enable-netmap patterned after --enable-capstone and
>> --enable-slirp has sufficiently low visibility as long as the default is
>> sane.
>>
>> We clearly want configure to pick netmap=system when the system provides
>> netmap.
>>
>> What shall configure pick when the system doesn't provide it?  If you
>> think netmap=git is too dangerous for general audience, consider
>> disabling netmap then.  Experts can still compile-test with
>> --enable-netmap=git.  Our CI certainly should.
>>
>
> OK, sounds reasonable. The attached patch will select system if netmap
> is available, and git only if explicitly requested.
>
> Cheers,
> Giuseppe
>
> -- 
> Dr. Ing. Giuseppe Lettieri
> Dipartimento di Ingegneria della Informazione
> Universita' di Pisa
> Largo Lucio Lazzarino 1, 56122 Pisa - Italy
> Ph. : (+39) 050-2217.649 (direct) .599 (switch)
> Fax : (+39) 050-2217.600
> e-mail: g.lettieri@iet.unipi.it
>
>>From 4e93b5cc3ad68d92bc3562df3745e1d10dc08fc0 Mon Sep 17 00:00:00 2001
> From: Giuseppe Lettieri <g.lettieri@iet.unipi.it>
> Date: Mon, 2 Sep 2019 17:35:33 +0200
> Subject: [PATCH] netmap: support git-submodule build otption
>
> With this patch, netmap support can be enabled with
> the following options to the configure script:
>
>   --enable-netmap[=system]
>
> 	Use the host system netmap installation.
> 	Fail if not found.
>
>   --enable-netmap=git
>
> 	clone the official netmap repository on
> 	github (mostly useful for CI)
>
> system will also be automatically used if no option is
> passed and netmap is available in the host system.
>
> Signed-off-by: Giuseppe Lettieri <giuseppe.lettieri@unipi.it>
> ---
>  .gitmodules |  3 +++
>  configure   | 68 ++++++++++++++++++++++++++++++++++++++++++++---------
>  netmap      |  1 +
>  3 files changed, 61 insertions(+), 11 deletions(-)
>  create mode 160000 netmap
>
> diff --git a/.gitmodules b/.gitmodules
> index c5c474169d..bf75dbc5e3 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -58,3 +58,6 @@
>  [submodule "roms/opensbi"]
>  	path = roms/opensbi
>  	url = 	https://git.qemu.org/git/opensbi.git
> +[submodule "netmap"]
> +	path = netmap
> +	url = https://github.com/luigirizzo/netmap.git
> diff --git a/configure b/configure
> index 30aad233d1..5cb924985c 100755
> --- a/configure
> +++ b/configure
> @@ -1133,6 +1133,10 @@ for opt do
>    ;;
>    --enable-netmap) netmap="yes"
>    ;;
> +  --enable-netmap=git) netmap="git"
> +  ;;
> +  --enable-netmap=system) netmap="system"
> +  ;;
>    --disable-xen) xen="no"
>    ;;
>    --enable-xen) xen="yes"
> @@ -3314,8 +3318,9 @@ fi
>  # a minor/major version number. Minor new features will be marked with values up
>  # to 15, and if something happens that requires a change to the backend we will
>  # move above 15, submit the backend fixes and modify this two bounds.
> -if test "$netmap" != "no" ; then
> -  cat > $TMPC << EOF
> +case "$netmap" in
> +    "" | yes | system)
> +      cat > $TMPC << EOF
>  #include <inttypes.h>
>  #include <net/if.h>
>  #include <net/netmap.h>
> @@ -3325,15 +3330,56 @@ if test "$netmap" != "no" ; then
>  #endif
>  int main(void) { return 0; }
>  EOF
> -  if compile_prog "" "" ; then
> -    netmap=yes
> -  else
> -    if test "$netmap" = "yes" ; then
> -      feature_not_found "netmap"
> +      if compile_prog "" "" ; then
> +        netmap_system=yes
> +      else
> +        netmap_system=no
> +      fi
> +      ;;
> +esac

Is the indentation change intentional?

> +
> +case "$netmap" in
> +  "" | yes)
> +    if test "$netmap_system" = "yes"; then
> +      netmap=system
> +    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> +      netmap=git
> +    elif test -e "${source_path}/netmap/configure" ; then
> +      netmap=internal
> +    elif test -z "$netmap" ; then
> +      netmap=no
> +    else
> +      feature_not_found "netmap" "Install netmap or git submodule"
>      fi
> -    netmap=no
> -  fi
> -fi
> +    ;;
> +
> +  system)
> +    if test "$netmap_system" = "no"; then
> +      feature_not_found "netmap" "Install netmap"
> +    fi
> +    ;;
> +esac
> +
> +case "$netmap" in
> +  git | internal)
> +    if test "$netmap" = git; then
> +      git_submodules="${git_submodules} netmap"
> +    fi
> +    mkdir -p netmap
> +    QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/netmap/sys"
> +    ;;
> +
> +  system)
> +    ;;
> +
> +  no)
> +    ;;
> +  *)
> +    error_exit "Unknown state for netmap: $netmap"
> +    ;;
> +esac
> +
> +##########################################
>  
>  ##########################################
>  # libcap-ng library probe
> @@ -6582,7 +6628,7 @@ if test "$vde" = "yes" ; then
>    echo "CONFIG_VDE=y" >> $config_host_mak
>    echo "VDE_LIBS=$vde_libs" >> $config_host_mak
>  fi
> -if test "$netmap" = "yes" ; then
> +if test "$netmap" != "no" ; then
>    echo "CONFIG_NETMAP=y" >> $config_host_mak
>  fi
>  if test "$l2tpv3" = "yes" ; then
> diff --git a/netmap b/netmap
> new file mode 160000
> index 0000000000..137f537eae
> --- /dev/null
> +++ b/netmap
> @@ -0,0 +1 @@
> +Subproject commit 137f537eae513f02d5d6871d1f91c049e6345803

Looks reasonable to me.  Please submit it as a patch.

[PATCH] netmap: support git-submodule build otption
Posted by Giuseppe Lettieri 6 years, 4 months ago
From: Giuseppe Lettieri <giuseppe.lettieri@unipi.it>

With this patch, netmap support can be enabled with
the following options to the configure script:

  --enable-netmap[=system]

	Use the host system netmap installation.
	Fail if not found.

  --enable-netmap=git

	clone the official netmap repository on
	github (mostly useful for CI)

Signed-off-by: Giuseppe Lettieri <giuseppe.lettieri@unipi.it>
---
 .gitmodules |  3 +++
 configure   | 64 +++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/.gitmodules b/.gitmodules
index c5c474169d..bf75dbc5e3 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -58,3 +58,6 @@
 [submodule "roms/opensbi"]
 	path = roms/opensbi
 	url = 	https://git.qemu.org/git/opensbi.git
+[submodule "netmap"]
+	path = netmap
+	url = https://github.com/luigirizzo/netmap.git
diff --git a/configure b/configure
index 8f8446f52b..cb2c6c70d6 100755
--- a/configure
+++ b/configure
@@ -1132,6 +1132,10 @@ for opt do
   ;;
   --enable-netmap) netmap="yes"
   ;;
+  --enable-netmap=git) netmap="git"
+  ;;
+  --enable-netmap=system) netmap="system"
+  ;;
   --disable-xen) xen="no"
   ;;
   --enable-xen) xen="yes"
@@ -3318,8 +3322,9 @@ fi
 # a minor/major version number. Minor new features will be marked with values up
 # to 15, and if something happens that requires a change to the backend we will
 # move above 15, submit the backend fixes and modify this two bounds.
-if test "$netmap" != "no" ; then
-  cat > $TMPC << EOF
+case "$netmap" in
+    "" | yes | system)
+      cat > $TMPC << EOF
 #include <inttypes.h>
 #include <net/if.h>
 #include <net/netmap.h>
@@ -3330,14 +3335,55 @@ if test "$netmap" != "no" ; then
 int main(void) { return 0; }
 EOF
   if compile_prog "" "" ; then
-    netmap=yes
+    netmap_system=yes
   else
-    if test "$netmap" = "yes" ; then
-      feature_not_found "netmap"
-    fi
-    netmap=no
+    netmap_system=no
   fi
-fi
+  ;;
+esac
+
+case "$netmap" in
+  "" | yes)
+    if test "$netmap_system" = "yes"; then
+      netmap=system
+    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
+      netmap=git
+    elif test -e "${source_path}/netmap/configure" ; then
+      netmap=internal
+    elif test -z "$netmap" ; then
+      netmap=no
+    else
+      feature_not_found "netmap" "Install netmap or git submodule"
+    fi
+    ;;
+
+  system)
+    if test "$netmap_system" = "no"; then
+      feature_not_found "netmap" "Install netmap"
+    fi
+    ;;
+esac
+
+case "$netmap" in
+  git | internal)
+    if test "$netmap" = git; then
+      git_submodules="${git_submodules} netmap"
+    fi
+    mkdir -p netmap
+    QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/netmap/sys"
+    ;;
+
+  system)
+    ;;
+
+  no)
+    ;;
+  *)
+    error_exit "Unknown state for netmap: $netmap"
+    ;;
+esac
+
+##########################################
 
 ##########################################
 # libcap-ng library probe
@@ -6585,7 +6631,7 @@ if test "$vde" = "yes" ; then
   echo "CONFIG_VDE=y" >> $config_host_mak
   echo "VDE_LIBS=$vde_libs" >> $config_host_mak
 fi
-if test "$netmap" = "yes" ; then
+if test "$netmap" != "no" ; then
   echo "CONFIG_NETMAP=y" >> $config_host_mak
 fi
 if test "$l2tpv3" = "yes" ; then
-- 
2.21.0


Re: [PATCH] netmap: support git-submodule build otption
Posted by Peter Maydell 6 years, 4 months ago
On Fri, 4 Oct 2019 at 14:03, Giuseppe Lettieri <g.lettieri@iet.unipi.it> wrote:
>
> From: Giuseppe Lettieri <giuseppe.lettieri@unipi.it>
>
> With this patch, netmap support can be enabled with
> the following options to the configure script:
>
>   --enable-netmap[=system]
>
>         Use the host system netmap installation.
>         Fail if not found.
>
>   --enable-netmap=git
>
>         clone the official netmap repository on
>         github (mostly useful for CI)
>
> Signed-off-by: Giuseppe Lettieri <giuseppe.lettieri@unipi.it>
> ---
>  .gitmodules |  3 +++
>  configure   | 64 +++++++++++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 58 insertions(+), 9 deletions(-)
>
> diff --git a/.gitmodules b/.gitmodules
> index c5c474169d..bf75dbc5e3 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -58,3 +58,6 @@
>  [submodule "roms/opensbi"]
>         path = roms/opensbi
>         url =   https://git.qemu.org/git/opensbi.git
> +[submodule "netmap"]
> +       path = netmap
> +       url = https://github.com/luigirizzo/netmap.git

Hi; this patch seems to be missing the rationale for why
we want to do this. New submodules:
 * should always be on git.qemu.org (we need to mirror them
in case the original upstream vanishes)
 * need a strong justification for why they're required
(ie why we can't just use whatever the system-provided
version of the library is, or fall back to not providing
the feature if the library isn't present)

Basically new submodules are a pain so we seek to minimize
the use of them.

thanks
-- PMM

Re: [PATCH] netmap: support git-submodule build otption
Posted by Markus Armbruster 6 years, 4 months ago
Peter Maydell <peter.maydell@linaro.org> writes:

D> On Fri, 4 Oct 2019 at 14:03, Giuseppe Lettieri <g.lettieri@iet.unipi.it> wrote:
>>
>> From: Giuseppe Lettieri <giuseppe.lettieri@unipi.it>
>>
>> With this patch, netmap support can be enabled with
>> the following options to the configure script:
>>
>>   --enable-netmap[=system]
>>
>>         Use the host system netmap installation.
>>         Fail if not found.
>>
>>   --enable-netmap=git
>>
>>         clone the official netmap repository on
>>         github (mostly useful for CI)
>>
>> Signed-off-by: Giuseppe Lettieri <giuseppe.lettieri@unipi.it>
>> ---
>>  .gitmodules |  3 +++
>>  configure   | 64 +++++++++++++++++++++++++++++++++++++++++++++--------
>>  2 files changed, 58 insertions(+), 9 deletions(-)
>>
>> diff --git a/.gitmodules b/.gitmodules
>> index c5c474169d..bf75dbc5e3 100644
>> --- a/.gitmodules
>> +++ b/.gitmodules
>> @@ -58,3 +58,6 @@
>>  [submodule "roms/opensbi"]
>>         path = roms/opensbi
>>         url =   https://git.qemu.org/git/opensbi.git
>> +[submodule "netmap"]
>> +       path = netmap
>> +       url = https://github.com/luigirizzo/netmap.git
>
> Hi; this patch seems to be missing the rationale for why
> we want to do this. New submodules:
>  * should always be on git.qemu.org (we need to mirror them
> in case the original upstream vanishes)
>  * need a strong justification for why they're required
> (ie why we can't just use whatever the system-provided
> version of the library is, or fall back to not providing
> the feature if the library isn't present)
>
> Basically new submodules are a pain so we seek to minimize
> the use of them.

I suggested making it a submodule upthread[*].  Let me try to distill
the conversation into a rationale.  Giuseppe, please correct mistakes.

To make use of QEMU's netmap backend (CONFIG_NETMAP), you have to build
and install netmap software from sources[**].  Which pretty much ensures
developers compile with CONFIG_NETMAP off, and the code rots.

For other dependencies that aren't readily available on common
development hosts (slirp, capstone), we use submodules to avoid such
rot.  If the system provides, we use that, and if it doesn't, we fall
back to the submodule.  This has served us well.

For netmap, falling back to the submodule when the host doesn't provide
tends not to be useful beyond compile-testing.  Because of that, we fall
back only when the user explicitly asks for it by passing
--enable-netmap=git to configure.  CI should do that.



[*] Message-ID: <87blwzho1y.fsf@dusky.pond.sub.org>

[**] FreeBSD hosts may be an exception; I'm not sure.

Re: [PATCH] netmap: support git-submodule build otption
Posted by Peter Maydell 6 years, 4 months ago
On Mon, 7 Oct 2019 at 11:50, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> > Basically new submodules are a pain so we seek to minimize
> > the use of them.
>
> I suggested making it a submodule upthread[*].  Let me try to distill
> the conversation into a rationale.  Giuseppe, please correct mistakes.
>
> To make use of QEMU's netmap backend (CONFIG_NETMAP), you have to build
> and install netmap software from sources[**].  Which pretty much ensures
> developers compile with CONFIG_NETMAP off, and the code rots.
>
> For other dependencies that aren't readily available on common
> development hosts (slirp, capstone), we use submodules to avoid such
> rot.  If the system provides, we use that, and if it doesn't, we fall
> back to the submodule.  This has served us well.

I would put this differently. We don't use submodules to avoid
code-rot. We use submodules where a dependency is needed for us
to provide QEMU features that are sufficiently important that we
want to provide them to users even if those users don't have the
dependency available to them as a system library.

There are lots of features of QEMU that only compile with sufficiently
recent versions of dependencies, and we don't try to submodule-ize
them because the features aren't really that important for the bulk
of our users. For instance, we provided pixman as a submodule for
a while because the features that require it (our graphics layer
code) are important to almost all users. But we didn't provide
spice as a module even when you pretty much needed to be
running bleeding-edge redhat to satisfy the version dependency
we had, because most users don't care about spice support.
Shipping our dependencies as submodules imposes real costs
on the project (for instance we then need to track the upstream
to see when we should be updating, including checking whether
we need to update to fix security issues). Submodules should be
the exception, not the rule.

> For netmap, falling back to the submodule when the host doesn't provide
> tends not to be useful beyond compile-testing.  Because of that, we fall
> back only when the user explicitly asks for it by passing
> --enable-netmap=git to configure.  CI should do that.

This sounds like netmap is in the same position as most of our
dependencies: OK to compile if the system provides the library,
but if the system doesn't then almost all users won't care
that the feature isn't present. If CI of the QEMU code is useful,
get the library supported by and shipped in distros. If you can't
get anybody in a distro (Linux or BSD) to care enough to ship the
library, this is a really niche feature, and up for consideration
for deprecate-and-drop from QEMU, I think.

thanks
-- PMM

Re: [PATCH] netmap: support git-submodule build otption
Posted by Markus Armbruster 6 years, 4 months ago
Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 7 Oct 2019 at 11:50, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > Basically new submodules are a pain so we seek to minimize
>> > the use of them.
>>
>> I suggested making it a submodule upthread[*].  Let me try to distill
>> the conversation into a rationale.  Giuseppe, please correct mistakes.
>>
>> To make use of QEMU's netmap backend (CONFIG_NETMAP), you have to build
>> and install netmap software from sources[**].  Which pretty much ensures
>> developers compile with CONFIG_NETMAP off, and the code rots.
>>
>> For other dependencies that aren't readily available on common
>> development hosts (slirp, capstone), we use submodules to avoid such
>> rot.  If the system provides, we use that, and if it doesn't, we fall
>> back to the submodule.  This has served us well.
>
> I would put this differently. We don't use submodules to avoid
> code-rot. We use submodules where a dependency is needed for us
> to provide QEMU features that are sufficiently important that we
> want to provide them to users even if those users don't have the
> dependency available to them as a system library.
>
> There are lots of features of QEMU that only compile with sufficiently
> recent versions of dependencies, and we don't try to submodule-ize
> them because the features aren't really that important for the bulk
> of our users. For instance, we provided pixman as a submodule for
> a while because the features that require it (our graphics layer
> code) are important to almost all users. But we didn't provide
> spice as a module even when you pretty much needed to be
> running bleeding-edge redhat to satisfy the version dependency
> we had, because most users don't care about spice support.

The ability to compile close to everything in a single build tree is
nevertheless convenient for developers.  Submodules aren't necessary for
that as long as "bleeding-edge redhat" can do the job.  And it pretty
much can: in my "everything" tree, config.status shows less than two
dozen "no", and most of them are uninteresting for compile-testing
(e.g. tcmalloc and jemalloc), or simply impossible to have (e.g. KVM is
for Linux, HAX is not for Linux, can't have both).  netmap's "no" is one
of the few that is due to missing dependencies.  Another one is vde.

> Shipping our dependencies as submodules imposes real costs
> on the project (for instance we then need to track the upstream
> to see when we should be updating, including checking whether
> we need to update to fix security issues). Submodules should be
> the exception, not the rule.

Point taken.

[...]

Re: [PATCH] netmap: support git-submodule build otption
Posted by Markus Armbruster 6 years, 4 months ago
Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 7 Oct 2019 at 11:50, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > Basically new submodules are a pain so we seek to minimize
>> > the use of them.
>>
>> I suggested making it a submodule upthread[*].  Let me try to distill
>> the conversation into a rationale.  Giuseppe, please correct mistakes.
>>
>> To make use of QEMU's netmap backend (CONFIG_NETMAP), you have to build
>> and install netmap software from sources[**].  Which pretty much ensures
>> developers compile with CONFIG_NETMAP off, and the code rots.
>>
>> For other dependencies that aren't readily available on common
>> development hosts (slirp, capstone), we use submodules to avoid such
>> rot.  If the system provides, we use that, and if it doesn't, we fall
>> back to the submodule.  This has served us well.
>
> I would put this differently. We don't use submodules to avoid
> code-rot. We use submodules where a dependency is needed for us
> to provide QEMU features that are sufficiently important that we
> want to provide them to users even if those users don't have the
> dependency available to them as a system library.
>
> There are lots of features of QEMU that only compile with sufficiently
> recent versions of dependencies, and we don't try to submodule-ize
> them because the features aren't really that important for the bulk
> of our users. For instance, we provided pixman as a submodule for
> a while because the features that require it (our graphics layer
> code) are important to almost all users. But we didn't provide
> spice as a module even when you pretty much needed to be
> running bleeding-edge redhat to satisfy the version dependency
> we had, because most users don't care about spice support.
> Shipping our dependencies as submodules imposes real costs
> on the project (for instance we then need to track the upstream
> to see when we should be updating, including checking whether
> we need to update to fix security issues). Submodules should be
> the exception, not the rule.
>
>> For netmap, falling back to the submodule when the host doesn't provide
>> tends not to be useful beyond compile-testing.  Because of that, we fall
>> back only when the user explicitly asks for it by passing
>> --enable-netmap=git to configure.  CI should do that.
>
> This sounds like netmap is in the same position as most of our
> dependencies: OK to compile if the system provides the library,
> but if the system doesn't then almost all users won't care
> that the feature isn't present. If CI of the QEMU code is useful,

If CI of QEMU code isn't useful, then I suspect the QEMU code isn't
useful, period.  Giuseppe assures us the netmap QEMU code *is* useful.
It followe we better make sure our CI covers it.

A submodule would make sure, but it looks like it won't fly.  So let's
try another tack:

> get the library supported by and shipped in distros. If you can't
> get anybody in a distro (Linux or BSD) to care enough to ship the
> library, this is a really niche feature, and up for consideration
> for deprecate-and-drop from QEMU, I think.

Giuseppe, you mentioned netmap is in FreeBSD, and getting it into Linux
is unlikely, so let's focus on FreeBSD.

We have a FreeBSD section in .patchew.yml, which makes me guess Patchew
CI tests FreeBSD.  Does it test with CONFIG_NETMAP out of the box?  If
not, how do we have to tweak its configuration to get CONFIG_NETMAP
enabled?  Who could help with this?

Re: [PATCH] netmap: support git-submodule build otption
Posted by Peter Maydell 6 years, 4 months ago
On Mon, 7 Oct 2019 at 13:36, Markus Armbruster <armbru@redhat.com> wrote:
> If CI of QEMU code isn't useful, then I suspect the QEMU code isn't
> useful, period.  Giuseppe assures us the netmap QEMU code *is* useful.
> It followe we better make sure our CI covers it.

It would be an interesting idea to have a requirement that
any new library dependency can't be introduced into QEMU
unless one of the systems we do builds on can be set up
so the new code is compiled...

thanks
-- PMM

Re: [PATCH] netmap: support git-submodule build otption
Posted by Daniel P. Berrangé 6 years, 4 months ago
On Mon, Oct 07, 2019 at 01:39:30PM +0100, Peter Maydell wrote:
> On Mon, 7 Oct 2019 at 13:36, Markus Armbruster <armbru@redhat.com> wrote:
> > If CI of QEMU code isn't useful, then I suspect the QEMU code isn't
> > useful, period.  Giuseppe assures us the netmap QEMU code *is* useful.
> > It followe we better make sure our CI covers it.
> 
> It would be an interesting idea to have a requirement that
> any new library dependency can't be introduced into QEMU
> unless one of the systems we do builds on can be set up
> so the new code is compiled...

Being able to at least compile code successfully is a pretty low bar
to cross in terms of CI, so I think that's reasonable in general.

I would not stop in terms of libraries though. We should document our
broader expectations for CI

Compilation

 - All new code must be compiled in one of[1] our CI systems.
 
   This implies

    - Libraries must be available in some OS we compile for

    - New host architectures must have cross compilers available

    - New OS distro targets must have VM test image support

This is not far off what we unofficially expect already, so
it shouldn't be too distruptive if we formally adopt it as a
mandatory rule.

Testing

 - All significant new features should have either unit or
   functional or integration test coverage

 ... something something something ...

We've not really set any expectations around CI beyond compile
testing thus far. We've got a mix of unit testing & functional
testing in the tests/ dir. We're increasingly getting stuff
added there when major features are added. Making this mandatory
is probably too large a step, but it is likely helpful if we
at least set some recommendations / guidelines to push people
in the direction we want to go longer term.

Regards,
Daniel

[1] Having to say "one of" is not especially great. It would be very nice
    to get to the point where we have either container images or VM images
    and no matter what CI harness(es) we use, they always run with either
    a container or VM image[2]. Even if we have bare metal machines available
    we can still execute actual builds inside containers or VM images so
    everyone uses a consistent environment for everything related to CI.

[2] macOS is a problem/exception here given that we can't legally distribute
    VM images, nor can we provide a cross compiler toolchain. For everything
    else we can make VM/container images though.
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] netmap: support git-submodule build otption
Posted by Markus Armbruster 6 years, 4 months ago
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, Oct 07, 2019 at 01:39:30PM +0100, Peter Maydell wrote:
>> On Mon, 7 Oct 2019 at 13:36, Markus Armbruster <armbru@redhat.com> wrote:
>> > If CI of QEMU code isn't useful, then I suspect the QEMU code isn't
>> > useful, period.  Giuseppe assures us the netmap QEMU code *is* useful.
>> > It followe we better make sure our CI covers it.
>> 
>> It would be an interesting idea to have a requirement that
>> any new library dependency can't be introduced into QEMU
>> unless one of the systems we do builds on can be set up
>> so the new code is compiled...
>
> Being able to at least compile code successfully is a pretty low bar
> to cross in terms of CI, so I think that's reasonable in general.
>
> I would not stop in terms of libraries though. We should document our
> broader expectations for CI
>
> Compilation
>
>  - All new code must be compiled in one of[1] our CI systems.

I think we should hold old code to the same standard.  Anything that's
not compiled now either gets fixed or deprecated.  If it's still unfixed
at the end of the deprecation grace period, it goes.

>    This implies
>
>     - Libraries must be available in some OS we compile for

How do we track compliance?  It's not obvious to (ignorant) me what
features exactly each CI compile enables.  Without that, it's not
obvious whether any CI run enables use of a certain library.

>     - New host architectures must have cross compilers available

Native tool chain in CI also satisfies "must be compiled in CI".

>     - New OS distro targets must have VM test image support

Non-virtual host in CI also satisfies "must be compiled in CI".

> This is not far off what we unofficially expect already, so
> it shouldn't be too distruptive if we formally adopt it as a
> mandatory rule.

Feels like a no-brainer, to be honest.

> Testing
>
>  - All significant new features should have either unit or
>    functional or integration test coverage
>
>  ... something something something ...

Spot the weasel words!  ;-P

> We've not really set any expectations around CI beyond compile
> testing thus far. We've got a mix of unit testing & functional
> testing in the tests/ dir. We're increasingly getting stuff
> added there when major features are added. Making this mandatory
> is probably too large a step, but it is likely helpful if we
> at least set some recommendations / guidelines to push people
> in the direction we want to go longer term.

We've been pushing, but not evenly.  It's basically up to maintainers,
and their expectations on testing vary.

> Regards,
> Daniel
>
> [1] Having to say "one of" is not especially great. It would be very nice
>     to get to the point where we have either container images or VM images
>     and no matter what CI harness(es) we use, they always run with either
>     a container or VM image[2]. Even if we have bare metal machines available
>     we can still execute actual builds inside containers or VM images so
>     everyone uses a consistent environment for everything related to CI.
>
> [2] macOS is a problem/exception here given that we can't legally distribute
>     VM images, nor can we provide a cross compiler toolchain. For everything
>     else we can make VM/container images though.

That makes Macs a secondary host at best.  If it breaks, it breaks.  If
somebody fixes it, nice, if not, *shrug*.  Don't expect git-bisect to
work.

Re: [PATCH] netmap: support git-submodule build otption
Posted by Thomas Huth 6 years, 4 months ago
On 07/10/2019 14.35, Markus Armbruster wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> On Mon, 7 Oct 2019 at 11:50, Markus Armbruster <armbru@redhat.com> wrote:
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>> Basically new submodules are a pain so we seek to minimize
>>>> the use of them.
>>>
>>> I suggested making it a submodule upthread[*].  Let me try to distill
>>> the conversation into a rationale.  Giuseppe, please correct mistakes.
>>>
>>> To make use of QEMU's netmap backend (CONFIG_NETMAP), you have to build
>>> and install netmap software from sources[**].  Which pretty much ensures
>>> developers compile with CONFIG_NETMAP off, and the code rots.
>>>
>>> For other dependencies that aren't readily available on common
>>> development hosts (slirp, capstone), we use submodules to avoid such
>>> rot.  If the system provides, we use that, and if it doesn't, we fall
>>> back to the submodule.  This has served us well.
>>
>> I would put this differently. We don't use submodules to avoid
>> code-rot. We use submodules where a dependency is needed for us
>> to provide QEMU features that are sufficiently important that we
>> want to provide them to users even if those users don't have the
>> dependency available to them as a system library.
>>
>> There are lots of features of QEMU that only compile with sufficiently
>> recent versions of dependencies, and we don't try to submodule-ize
>> them because the features aren't really that important for the bulk
>> of our users. For instance, we provided pixman as a submodule for
>> a while because the features that require it (our graphics layer
>> code) are important to almost all users. But we didn't provide
>> spice as a module even when you pretty much needed to be
>> running bleeding-edge redhat to satisfy the version dependency
>> we had, because most users don't care about spice support.
>> Shipping our dependencies as submodules imposes real costs
>> on the project (for instance we then need to track the upstream
>> to see when we should be updating, including checking whether
>> we need to update to fix security issues). Submodules should be
>> the exception, not the rule.
>>
>>> For netmap, falling back to the submodule when the host doesn't provide
>>> tends not to be useful beyond compile-testing.  Because of that, we fall
>>> back only when the user explicitly asks for it by passing
>>> --enable-netmap=git to configure.  CI should do that.
>>
>> This sounds like netmap is in the same position as most of our
>> dependencies: OK to compile if the system provides the library,
>> but if the system doesn't then almost all users won't care
>> that the feature isn't present. If CI of the QEMU code is useful,
> 
> If CI of QEMU code isn't useful, then I suspect the QEMU code isn't
> useful, period.  Giuseppe assures us the netmap QEMU code *is* useful.
> It followe we better make sure our CI covers it.
> 
> A submodule would make sure, but it looks like it won't fly.  So let's
> try another tack:
> 
>> get the library supported by and shipped in distros. If you can't
>> get anybody in a distro (Linux or BSD) to care enough to ship the
>> library, this is a really niche feature, and up for consideration
>> for deprecate-and-drop from QEMU, I think.
> 
> Giuseppe, you mentioned netmap is in FreeBSD, and getting it into Linux
> is unlikely, so let's focus on FreeBSD.
> 
> We have a FreeBSD section in .patchew.yml, which makes me guess Patchew
> CI tests FreeBSD.  Does it test with CONFIG_NETMAP out of the box?  If
> not, how do we have to tweak its configuration to get CONFIG_NETMAP
> enabled?  Who could help with this?

I just tried this patch here:

diff --git a/.cirrus.yml b/.cirrus.yml
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -8,7 +8,7 @@ freebsd_12_task:
     memory: 8G
   install_script: pkg install -y
     bash bison curl cyrus-sasl git glib gmake gnutls gsed
-    nettle perl5 pixman pkgconf png usbredir
+    nettle perl5 pixman pkgconf png usbredir netmap
   script:
     - mkdir build
     - cd build

... and looks like net/netmap.c now gets successfully compiled on
FreeBSD in the Cirrus-CI:

 https://api.cirrus-ci.com/v1/task/5669479475838976/logs/main.log

We can also add it to the vm-freebsd test:

diff --git a/tests/vm/freebsd b/tests/vm/freebsd
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -54,6 +54,9 @@ class FreeBSDVM(basevm.BaseVM):
         # libs: opengl
         "libepoxy",
         "mesa-libs",
+
+        # libs: network
+        "netmap",
     ]

     BUILD_SCRIPT = """

... then it gets compiled succesfully during "make vm-build-freebsd".

So does that sound like a good way to keep netmap.c from bitrotting? If
so, I can send the above two diffs as a proper patch, if you like.

 Thomas



Re: [PATCH] netmap: support git-submodule build otption
Posted by Markus Armbruster 6 years, 4 months ago
Thomas Huth <thuth@redhat.com> writes:

> On 07/10/2019 14.35, Markus Armbruster wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> 
>>> On Mon, 7 Oct 2019 at 11:50, Markus Armbruster <armbru@redhat.com> wrote:
>>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>>> Basically new submodules are a pain so we seek to minimize
>>>>> the use of them.
>>>>
>>>> I suggested making it a submodule upthread[*].  Let me try to distill
>>>> the conversation into a rationale.  Giuseppe, please correct mistakes.
>>>>
>>>> To make use of QEMU's netmap backend (CONFIG_NETMAP), you have to build
>>>> and install netmap software from sources[**].  Which pretty much ensures
>>>> developers compile with CONFIG_NETMAP off, and the code rots.
>>>>
>>>> For other dependencies that aren't readily available on common
>>>> development hosts (slirp, capstone), we use submodules to avoid such
>>>> rot.  If the system provides, we use that, and if it doesn't, we fall
>>>> back to the submodule.  This has served us well.
>>>
>>> I would put this differently. We don't use submodules to avoid
>>> code-rot. We use submodules where a dependency is needed for us
>>> to provide QEMU features that are sufficiently important that we
>>> want to provide them to users even if those users don't have the
>>> dependency available to them as a system library.
>>>
>>> There are lots of features of QEMU that only compile with sufficiently
>>> recent versions of dependencies, and we don't try to submodule-ize
>>> them because the features aren't really that important for the bulk
>>> of our users. For instance, we provided pixman as a submodule for
>>> a while because the features that require it (our graphics layer
>>> code) are important to almost all users. But we didn't provide
>>> spice as a module even when you pretty much needed to be
>>> running bleeding-edge redhat to satisfy the version dependency
>>> we had, because most users don't care about spice support.
>>> Shipping our dependencies as submodules imposes real costs
>>> on the project (for instance we then need to track the upstream
>>> to see when we should be updating, including checking whether
>>> we need to update to fix security issues). Submodules should be
>>> the exception, not the rule.
>>>
>>>> For netmap, falling back to the submodule when the host doesn't provide
>>>> tends not to be useful beyond compile-testing.  Because of that, we fall
>>>> back only when the user explicitly asks for it by passing
>>>> --enable-netmap=git to configure.  CI should do that.
>>>
>>> This sounds like netmap is in the same position as most of our
>>> dependencies: OK to compile if the system provides the library,
>>> but if the system doesn't then almost all users won't care
>>> that the feature isn't present. If CI of the QEMU code is useful,
>> 
>> If CI of QEMU code isn't useful, then I suspect the QEMU code isn't
>> useful, period.  Giuseppe assures us the netmap QEMU code *is* useful.
>> It followe we better make sure our CI covers it.
>> 
>> A submodule would make sure, but it looks like it won't fly.  So let's
>> try another tack:
>> 
>>> get the library supported by and shipped in distros. If you can't
>>> get anybody in a distro (Linux or BSD) to care enough to ship the
>>> library, this is a really niche feature, and up for consideration
>>> for deprecate-and-drop from QEMU, I think.
>> 
>> Giuseppe, you mentioned netmap is in FreeBSD, and getting it into Linux
>> is unlikely, so let's focus on FreeBSD.
>> 
>> We have a FreeBSD section in .patchew.yml, which makes me guess Patchew
>> CI tests FreeBSD.  Does it test with CONFIG_NETMAP out of the box?  If
>> not, how do we have to tweak its configuration to get CONFIG_NETMAP
>> enabled?  Who could help with this?
>
> I just tried this patch here:
>
> diff --git a/.cirrus.yml b/.cirrus.yml
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -8,7 +8,7 @@ freebsd_12_task:
>      memory: 8G
>    install_script: pkg install -y
>      bash bison curl cyrus-sasl git glib gmake gnutls gsed
> -    nettle perl5 pixman pkgconf png usbredir
> +    nettle perl5 pixman pkgconf png usbredir netmap
>    script:
>      - mkdir build
>      - cd build
>
> ... and looks like net/netmap.c now gets successfully compiled on
> FreeBSD in the Cirrus-CI:
>
>  https://api.cirrus-ci.com/v1/task/5669479475838976/logs/main.log

Awesome :)

> We can also add it to the vm-freebsd test:
>
> diff --git a/tests/vm/freebsd b/tests/vm/freebsd
> --- a/tests/vm/freebsd
> +++ b/tests/vm/freebsd
> @@ -54,6 +54,9 @@ class FreeBSDVM(basevm.BaseVM):
>          # libs: opengl
>          "libepoxy",
>          "mesa-libs",
> +
> +        # libs: network
> +        "netmap",
>      ]
>
>      BUILD_SCRIPT = """
>
> ... then it gets compiled succesfully during "make vm-build-freebsd".
>
> So does that sound like a good way to keep netmap.c from bitrotting? If
> so, I can send the above two diffs as a proper patch, if you like.

Yes, please!

Re: [Qemu-devel] Is network backend netmap worth keeping? (was: [PATCH v2 27/29] Include sysemu/sysemu.h a lot less)
Posted by Stefano Garzarella 6 years, 6 months ago
On Thu, Aug 08, 2019 at 06:48:25AM +0200, Markus Armbruster wrote:
> Please excuse the attention-grabbing subject.
> 
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
> > On 8/7/19 10:16 PM, Markus Armbruster wrote:
> [...]
> >> Can you tell me offhand what I have to install so configure enables
> >> CONFIG_NETMAP?
> >
> > The steps are listed in tests/docker/dockerfiles/debian-amd64.docker,
> > but you can get to this point running:
> >
> >   $ make docker-image-debian-amd64 V=1 DEBUG=1
> >
> > This will build the docker image with netmap (so you don't have to mess
> > with your workstation setup), then build QEMU within the image.
> 
> So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
> build and install netmap software from sources.  Which pretty much
> ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
> The commit message points to <http://info.iet.unipi.it/~luigi/netmap/>,
> which gives me "connection timed out" right now.
> 
> On the other hand, it's covered in MAINTAINERS, and has seen
> non-janitorial activity as late as Dec 2018 (commit c693fc748a).
> 
> Luigi, Giuseppe, Vincenzo, what's the status of the netmap project?

I think Giuseppe and Vincenzo are currently maintain netmap.
I worked with them on my master's thesis. :) I can give you some information,
but I'm sure they can be more specific.

More info here: https://github.com/luigirizzo/netmap

> 
> Why is the QEMU netmap backend worth keeping?

Netmap provides a virtual switch (VALE) and netmap pipes that can be
useful for VMs and the netmap backend allows us to use them.

> 
> Who is using the netmap backend?
> 
> How do they obtain a netmap-enabled QEMU?  Compile it from sources
> themselves?

Yes, I think so.

> 
> Would it make sense to have netmap packaged in common Linux distros?
> 

Maybe yes, for the virtual switch (VALE) and netmap pipes there shouldn't
be a problem. To use the network cards, however, you would need the modified
drivers.

Cheers,
Stefano

Is network backend vde worth keeping? (was: Is network backend netmap worth keeping?)
Posted by Markus Armbruster 6 years, 4 months ago
Markus Armbruster <armbru@redhat.com> writes:

> Please excuse the attention-grabbing subject.

Again.

[...]
> So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
> build and install netmap software from sources.  Which pretty much

CONFIG_VDE seems to be similarly cumbersome to build-test.

> ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
[...]

The vde backend was added in commit 8a16d273887 (Jul 2008).  The commit
message blames it on Luca Bigliardi.  Julia (cc'ed) fixed a bug in 2018.
Can't see any other VDE-specific activity since we split net/vde.c off
net.c in 2009.

I found a github repository virtualsquare/vde-2, which seems to be
pertinent.  Recent commits have been merged by
danielinux@users.noreply.github.com, which looks anti-social enough to
me not to bother with a cc.  Further digging coughed up Renzo Davoli
(cc'ed).

[...]
> Why is the QEMU netmap backend worth keeping?
>
> Who is using the netmap backend?
>
> How do they obtain a netmap-enabled QEMU?  Compile it from sources
> themselves?
>
> Would it make sense to have netmap packaged in common Linux distros?

Same questions for the QEMU vde backend.

Re: Is network backend vde worth keeping? (was: Is network backend netmap worth keeping?)
Posted by Thomas Huth 6 years, 4 months ago
On 07/10/2019 20.21, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Please excuse the attention-grabbing subject.
> 
> Again.
> 
> [...]
>> So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
>> build and install netmap software from sources.  Which pretty much
> 
> CONFIG_VDE seems to be similarly cumbersome to build-test.

There seems to be a libvdeplug-dev package on Debian / Ubuntu which
should provide the necessary headers if I've got that right...?

==> Try to add libvdeplug-dev to .travis.yml and .gitlab-ci.yml to get
at least the compile test coverage?

 Thomas

Re: Is network backend vde worth keeping? (was: Is network backend netmap worth keeping?)
Posted by Thomas Huth 6 years, 4 months ago
On 09/10/2019 11.13, Thomas Huth wrote:
> On 07/10/2019 20.21, Markus Armbruster wrote:
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> Please excuse the attention-grabbing subject.
>>
>> Again.
>>
>> [...]
>>> So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
>>> build and install netmap software from sources.  Which pretty much
>>
>> CONFIG_VDE seems to be similarly cumbersome to build-test.
> 
> There seems to be a libvdeplug-dev package on Debian / Ubuntu which
> should provide the necessary headers if I've got that right...?

FWIW:
https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01983.html

That seems to enable "vde support" according to the output of configure:

https://travis-ci.com/huth/qemu/jobs/243803521#L1019
https://travis-ci.com/huth/qemu/jobs/243803521#L2522

 Thomas

Re: Is network backend vde worth keeping? (was: Is network backend netmap worth keeping?)
Posted by Julia Suvorova 6 years, 4 months ago
On Mon, Oct 7, 2019 at 8:23 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Markus Armbruster <armbru@redhat.com> writes:
>
> > Please excuse the attention-grabbing subject.
>
> Again.
>
> [...]
> > So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
> > build and install netmap software from sources.  Which pretty much
>
> CONFIG_VDE seems to be similarly cumbersome to build-test.
>
> > ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
> [...]
>
> The vde backend was added in commit 8a16d273887 (Jul 2008).  The commit
> message blames it on Luca Bigliardi.  Julia (cc'ed) fixed a bug in 2018.

This was a bugzilla entry dated 2010 from the BiteSizedTasks list. You
can definitely ignore this commit when deciding whether to keep vde
backend.

Best regards, Julia Suvorova.

Re: Is network backend vde worth keeping? (was: Is network backend netmap worth keeping?)
Posted by Daniel P. Berrangé 6 years, 4 months ago
On Mon, Oct 07, 2019 at 08:21:14PM +0200, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
> > Please excuse the attention-grabbing subject.
> 
> Again.
> 
> [...]
> > So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
> > build and install netmap software from sources.  Which pretty much
> 
> CONFIG_VDE seems to be similarly cumbersome to build-test.

VDE2 package is available in Ubuntu at least, so we could possibly
build in our CI that way I believe:

  https://packages.ubuntu.com/disco/vde2

> > ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
> [...]
> 
> The vde backend was added in commit 8a16d273887 (Jul 2008).  The commit
> message blames it on Luca Bigliardi.  Julia (cc'ed) fixed a bug in 2018.
> Can't see any other VDE-specific activity since we split net/vde.c off
> net.c in 2009.
> 
> I found a github repository virtualsquare/vde-2, which seems to be
> pertinent.  Recent commits have been merged by
> danielinux@users.noreply.github.com, which looks anti-social enough to
> me not to bother with a cc.  Further digging coughed up Renzo Davoli
> (cc'ed).

The website(s) are certainly not very clear as to what's the right
thing to be using.

The vde-2 code appears to be deprecated from what I can see though

http://wiki.virtualsquare.org/#!repos.md

"The vde-2 vde_switch is fully compatible with vdeplug4. Although
 it has not been rewritten yet, it includes several tools that are
 actual like vde\switch and wirefilter."

The vdeplug4 repo docs do refer to QEMU's  'vde' network config

   https://github.com/rd235/vdeplug4

but the repository doesn't show especially high level of activity
and I don't see this in any distros, only the old vde-2.

> > Why is the QEMU netmap backend worth keeping?
> >
> > Who is using the netmap backend?
> >
> > How do they obtain a netmap-enabled QEMU?  Compile it from sources
> > themselves?
> >
> > Would it make sense to have netmap packaged in common Linux distros?
> 
> Same questions for the QEMU vde backend.

It is in Ubuntu.  If it isn't widely available in distributions though,
that is surely a reflection the level of usage amongst users. 

The popular high performance userspace networking project(s) (DPDK) these
days all seem to be based around QEMU's vhostuser or TAP backends, which
is good as it means QEMU doesn't need to support an application specific
backend.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 27/29] Include sysemu/sysemu.h a lot less
Posted by Philippe Mathieu-Daudé 6 years, 6 months ago
On 8/6/19 5:14 PM, Markus Armbruster wrote:
> In my "build everything" tree, changing sysemu/sysemu.h triggers a
> recompile of some 5400 out of 6600 objects (not counting tests and
> objects that don't depend on qemu/osdep.h).
> 
> hw/qdev-core.h includes sysemu/sysemu.h since recent commit e965ffa70a
> "qdev: add qdev_add_vm_change_state_handler()".  This is a bad idea:
> hw/qdev-core.h is widely included.
> 
> Move the declaration of qdev_add_vm_change_state_handler() to
> sysemu/sysemu.h, and drop the problematic include from hw/qdev-core.h.
> 
> Touching sysemu/sysemu.h now recompiles some 1800 objects.
> qemu/uuid.h also drops from 5400 to 1800.  A few more headers show
> smaller improvement: qemu/notify.h drops from 5600 to 5200,
> qemu/timer.h from 5600 to 4500, and qapi/qapi-types-run-state.h from
> 5500 to 5000.
> 
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  accel/kvm/kvm-all.c               | 1 +
>  backends/hostmem.c                | 1 +
>  cpus.c                            | 1 +
>  hw/arm/allwinner-a10.c            | 1 +
>  hw/arm/aspeed_soc.c               | 1 +
>  hw/arm/kzm.c                      | 1 +
>  hw/arm/msf2-soc.c                 | 1 +
>  hw/arm/stm32f205_soc.c            | 1 +
>  hw/char/serial-isa.c              | 1 +
>  hw/char/xen_console.c             | 1 +
>  hw/core/numa.c                    | 1 +
>  hw/core/vm-change-state-handler.c | 1 +
>  hw/display/qxl-render.c           | 1 +
>  hw/i386/xen/xen-hvm.c             | 1 +
>  hw/i386/xen/xen-mapcache.c        | 1 +
>  hw/intc/ioapic.c                  | 1 +
>  hw/pci/pci.c                      | 1 +
>  hw/riscv/sifive_e.c               | 1 +
>  hw/riscv/sifive_u.c               | 1 +
>  hw/riscv/spike.c                  | 1 +
>  hw/riscv/virt.c                   | 1 +
>  hw/sparc64/niagara.c              | 2 +-
>  hw/usb/hcd-ehci.h                 | 1 +
>  hw/xen/xen-common.c               | 1 +
>  hw/xen/xen_devconfig.c            | 1 +
>  hw/xenpv/xen_machine_pv.c         | 1 +
>  include/hw/qdev-core.h            | 5 -----
>  include/sysemu/sysemu.h           | 3 +++
>  migration/global_state.c          | 1 +
>  migration/migration.c             | 1 +
>  migration/savevm.c                | 1 +
>  31 files changed, 32 insertions(+), 6 deletions(-)
> 
[...]
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index e5b62dd2fc..de70b7a19a 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -5,7 +5,6 @@
>  #include "qemu/bitmap.h"
>  #include "qom/object.h"
>  #include "hw/hotplug.h"
> -#include "sysemu/sysemu.h"

Another build errors on OSX:

ui/cocoa.m:445:10: error: use of undeclared identifier 'cursor_hide'
    if (!cursor_hide) {
         ^
ui/cocoa.m:453:10: error: use of undeclared identifier 'cursor_hide'
    if (!cursor_hide) {
         ^
ui/cocoa.m:596:13: error: use of undeclared identifier 'qemu_name'
        if (qemu_name)
            ^
warning: format specifies type 'char *' but the argument has type
'<dependent type>' [-Wformat]
ui/cocoa.m:597:75: error: use of undeclared identifier 'qemu_name'
            [normalWindow setTitle:[NSString stringWithFormat:@"QEMU
%s", qemu_name]];
                                                                         ^
ui/cocoa.m:995:13: error: use of undeclared identifier 'qemu_name'
        if (qemu_name)
            ^
warning: format specifies type 'char *' but the argument has type
'<dependent type>' [-Wformat]
ui/cocoa.m:996:117: error: use of undeclared identifier 'qemu_name'
            [normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s
- (Press ctrl + alt + g to release Mouse)", qemu_name]];

                                            ^
ui/cocoa.m:1013:13: error: use of undeclared identifier 'qemu_name'
        if (qemu_name)
            ^
warning: format specifies type 'char *' but the argument has type
'<dependent type>' [-Wformat]
ui/cocoa.m:1014:75: error: use of undeclared identifier 'qemu_name'
            [normalWindow setTitle:[NSString stringWithFormat:@"QEMU
%s", qemu_name]];
                                                                          ^
ui/cocoa.m:1164:5: warning: implicit declaration of function
'qemu_system_shutdown_request' is invalid in C99
[-Wimplicit-function-declaration]
    qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
    ^
ui/cocoa.m:1164:5: warning: this function declaration is not a prototype
[-Wstrict-prototypes]
make: *** [ui/cocoa.o] Error 1

>  
>  enum {
>      DEV_NVECTORS_UNSPECIFIED = -1,
> @@ -451,8 +450,4 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
>  void device_listener_register(DeviceListener *listener);
>  void device_listener_unregister(DeviceListener *listener);
>  
> -VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
> -                                                     VMChangeStateHandler *cb,
> -                                                     void *opaque);
> -
>  #endif
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 227202999d..908f158677 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -29,6 +29,9 @@ VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
>                                                       void *opaque);
>  VMChangeStateEntry *qemu_add_vm_change_state_handler_prio(
>          VMChangeStateHandler *cb, void *opaque, int priority);
> +VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
> +                                                     VMChangeStateHandler *cb,
> +                                                     void *opaque);
>  void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
>  void vm_state_notify(int running, RunState state);

Re: [Qemu-devel] [PATCH v2 27/29] Include sysemu/sysemu.h a lot less
Posted by Markus Armbruster 6 years, 6 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 8/6/19 5:14 PM, Markus Armbruster wrote:
>> In my "build everything" tree, changing sysemu/sysemu.h triggers a
>> recompile of some 5400 out of 6600 objects (not counting tests and
>> objects that don't depend on qemu/osdep.h).
>> 
>> hw/qdev-core.h includes sysemu/sysemu.h since recent commit e965ffa70a
>> "qdev: add qdev_add_vm_change_state_handler()".  This is a bad idea:
>> hw/qdev-core.h is widely included.
>> 
>> Move the declaration of qdev_add_vm_change_state_handler() to
>> sysemu/sysemu.h, and drop the problematic include from hw/qdev-core.h.
>> 
>> Touching sysemu/sysemu.h now recompiles some 1800 objects.
>> qemu/uuid.h also drops from 5400 to 1800.  A few more headers show
>> smaller improvement: qemu/notify.h drops from 5600 to 5200,
>> qemu/timer.h from 5600 to 4500, and qapi/qapi-types-run-state.h from
>> 5500 to 5000.
>> 
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
[...]
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index e5b62dd2fc..de70b7a19a 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -5,7 +5,6 @@
>>  #include "qemu/bitmap.h"
>>  #include "qom/object.h"
>>  #include "hw/hotplug.h"
>> -#include "sysemu/sysemu.h"
>
> Another build errors on OSX:
>
> ui/cocoa.m:445:10: error: use of undeclared identifier 'cursor_hide'
>     if (!cursor_hide) {
>          ^
> ui/cocoa.m:453:10: error: use of undeclared identifier 'cursor_hide'
>     if (!cursor_hide) {
>          ^
> ui/cocoa.m:596:13: error: use of undeclared identifier 'qemu_name'
>         if (qemu_name)
>             ^
> warning: format specifies type 'char *' but the argument has type
> '<dependent type>' [-Wformat]
> ui/cocoa.m:597:75: error: use of undeclared identifier 'qemu_name'
>             [normalWindow setTitle:[NSString stringWithFormat:@"QEMU
> %s", qemu_name]];
>                                                                          ^
> ui/cocoa.m:995:13: error: use of undeclared identifier 'qemu_name'
>         if (qemu_name)
>             ^
> warning: format specifies type 'char *' but the argument has type
> '<dependent type>' [-Wformat]
> ui/cocoa.m:996:117: error: use of undeclared identifier 'qemu_name'
>             [normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s
> - (Press ctrl + alt + g to release Mouse)", qemu_name]];
>
>                                             ^
> ui/cocoa.m:1013:13: error: use of undeclared identifier 'qemu_name'
>         if (qemu_name)
>             ^
> warning: format specifies type 'char *' but the argument has type
> '<dependent type>' [-Wformat]
> ui/cocoa.m:1014:75: error: use of undeclared identifier 'qemu_name'
>             [normalWindow setTitle:[NSString stringWithFormat:@"QEMU
> %s", qemu_name]];
>                                                                           ^
> ui/cocoa.m:1164:5: warning: implicit declaration of function
> 'qemu_system_shutdown_request' is invalid in C99
> [-Wimplicit-function-declaration]
>     qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
>     ^
> ui/cocoa.m:1164:5: warning: this function declaration is not a prototype
> [-Wstrict-prototypes]
> make: *** [ui/cocoa.o] Error 1

Will fix, thanks!