* [PATCH v3 0/1] Clean up includes
@ 2023-01-12 11:50 Markus Armbruster
2023-01-12 11:50 ` [PATCH v3 1/1] include: Don't include qemu/osdep.h Markus Armbruster
0 siblings, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2023-01-12 11:50 UTC (permalink / raw)
To: qemu-devel
Cc: imp, kevans, berrange, ben.widawsky, jonathan.cameron, kbastian,
jasowang, michael.roth, kkostiuk, tsimpson, palmer,
alistair.francis, bin.meng, qemu-riscv, philmd, mst
Back in 2016, we discussed[1] rules for headers, and these were
generally liked:
1. Have a carefully curated header that's included everywhere first. We
got that already thanks to Peter: osdep.h.
2. Headers should normally include everything they need beyond osdep.h.
If exceptions are needed for some reason, they must be documented in
the header. If all that's needed from a header is typedefs, put
those into qemu/typedefs.h instead of including the header.
3. Cyclic inclusion is forbidden.
This series fixes a number of rule violations.
Together with
[PATCH v2 0/4] hw/ppc: Clean up includes
[PATCH v2 0/7] include/hw/pci include/hw/cxl: Clean up includes
in master as commit 9d94c21363..881e019770
[PATCH v2 0/3] block: Clean up includes
[PATCH v3 0/5] coroutine: Clean up includes
just three inclusion loops remain reachable from include/:
target/microblaze/cpu.h target/microblaze/mmu.h
target/nios2/cpu.h target/nios2/mmu.h
target/riscv/cpu.h target/riscv/pmp.h
Breaking them would be nice, but I'm out of steam.
v3:
* Rebased, old PATCH 1+2+4 are in master as commit
881e019770..f07ceffdf5
* PATCH 1: Fix bsd-user
v2:
* Rebased
* PATCH 3: v1 posted separately
* PATCH 4: New
[1] Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
Markus Armbruster (1):
include: Don't include qemu/osdep.h
bsd-user/qemu.h | 1 -
crypto/block-luks-priv.h | 1 -
include/hw/cxl/cxl_host.h | 1 -
include/hw/input/pl050.h | 1 -
include/hw/tricore/triboard.h | 1 -
include/qemu/userfaultfd.h | 1 -
net/vmnet_int.h | 1 -
qga/cutils.h | 1 -
target/hexagon/hex_arch_types.h | 1 -
target/hexagon/mmvec/macros.h | 1 -
target/riscv/pmu.h | 1 -
bsd-user/arm/signal.c | 1 +
bsd-user/arm/target_arch_cpu.c | 2 ++
bsd-user/freebsd/os-sys.c | 1 +
bsd-user/i386/signal.c | 1 +
bsd-user/x86_64/signal.c | 1 +
qga/cutils.c | 3 ++-
17 files changed, 8 insertions(+), 12 deletions(-)
--
2.39.0
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 11:50 [PATCH v3 0/1] Clean up includes Markus Armbruster @ 2023-01-12 11:50 ` Markus Armbruster 2023-01-12 13:51 ` Michael S. Tsirkin ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Markus Armbruster @ 2023-01-12 11:50 UTC (permalink / raw) To: qemu-devel Cc: imp, kevans, berrange, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, mst, Bin Meng docs/devel/style.rst mandates: The "qemu/osdep.h" header contains preprocessor macros that affect the behavior of core system headers like <stdint.h>. It must be the first include so that core system headers included by external libraries get the preprocessor macros that QEMU depends on. Do not include "qemu/osdep.h" from header files since the .c file will have already included it. A few violations have crept in. Fix them. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> --- bsd-user/qemu.h | 1 - crypto/block-luks-priv.h | 1 - include/hw/cxl/cxl_host.h | 1 - include/hw/input/pl050.h | 1 - include/hw/tricore/triboard.h | 1 - include/qemu/userfaultfd.h | 1 - net/vmnet_int.h | 1 - qga/cutils.h | 1 - target/hexagon/hex_arch_types.h | 1 - target/hexagon/mmvec/macros.h | 1 - target/riscv/pmu.h | 1 - bsd-user/arm/signal.c | 1 + bsd-user/arm/target_arch_cpu.c | 2 ++ bsd-user/freebsd/os-sys.c | 1 + bsd-user/i386/signal.c | 1 + bsd-user/x86_64/signal.c | 1 + qga/cutils.c | 3 ++- 17 files changed, 8 insertions(+), 12 deletions(-) diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h index be6105385e..0ceecfb6df 100644 --- a/bsd-user/qemu.h +++ b/bsd-user/qemu.h @@ -17,7 +17,6 @@ #ifndef QEMU_H #define QEMU_H -#include "qemu/osdep.h" #include "cpu.h" #include "qemu/units.h" #include "exec/cpu_ldst.h" diff --git a/crypto/block-luks-priv.h b/crypto/block-luks-priv.h index 90a20d432b..1066df0307 100644 --- a/crypto/block-luks-priv.h +++ b/crypto/block-luks-priv.h @@ -18,7 +18,6 @@ * */ -#include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/bswap.h" diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h index a1b662ce40..c9bc9c7c50 100644 --- a/include/hw/cxl/cxl_host.h +++ b/include/hw/cxl/cxl_host.h @@ -7,7 +7,6 @@ * COPYING file in the top-level directory. */ -#include "qemu/osdep.h" #include "hw/cxl/cxl.h" #include "hw/boards.h" diff --git a/include/hw/input/pl050.h b/include/hw/input/pl050.h index 89ec4fafc9..4cb8985f31 100644 --- a/include/hw/input/pl050.h +++ b/include/hw/input/pl050.h @@ -10,7 +10,6 @@ #ifndef HW_PL050_H #define HW_PL050_H -#include "qemu/osdep.h" #include "hw/sysbus.h" #include "migration/vmstate.h" #include "hw/input/ps2.h" diff --git a/include/hw/tricore/triboard.h b/include/hw/tricore/triboard.h index 094c8bd563..4fdd2d7d97 100644 --- a/include/hw/tricore/triboard.h +++ b/include/hw/tricore/triboard.h @@ -18,7 +18,6 @@ * License along with this library; if not, see <http://www.gnu.org/licenses/>. */ -#include "qemu/osdep.h" #include "qapi/error.h" #include "hw/boards.h" #include "sysemu/sysemu.h" diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h index 6b74f92792..55c95998e8 100644 --- a/include/qemu/userfaultfd.h +++ b/include/qemu/userfaultfd.h @@ -13,7 +13,6 @@ #ifndef USERFAULTFD_H #define USERFAULTFD_H -#include "qemu/osdep.h" #include "exec/hwaddr.h" #include <linux/userfaultfd.h> diff --git a/net/vmnet_int.h b/net/vmnet_int.h index adf6e8c20d..d0b90594f2 100644 --- a/net/vmnet_int.h +++ b/net/vmnet_int.h @@ -10,7 +10,6 @@ #ifndef VMNET_INT_H #define VMNET_INT_H -#include "qemu/osdep.h" #include "vmnet_int.h" #include "clients.h" diff --git a/qga/cutils.h b/qga/cutils.h index f0f30a7d28..2bfaf554a8 100644 --- a/qga/cutils.h +++ b/qga/cutils.h @@ -1,7 +1,6 @@ #ifndef CUTILS_H_ #define CUTILS_H_ -#include "qemu/osdep.h" int qga_open_cloexec(const char *name, int flags, mode_t mode); diff --git a/target/hexagon/hex_arch_types.h b/target/hexagon/hex_arch_types.h index 885f68f760..52a7f2b2f3 100644 --- a/target/hexagon/hex_arch_types.h +++ b/target/hexagon/hex_arch_types.h @@ -18,7 +18,6 @@ #ifndef HEXAGON_HEX_ARCH_TYPES_H #define HEXAGON_HEX_ARCH_TYPES_H -#include "qemu/osdep.h" #include "mmvec/mmvec.h" #include "qemu/int128.h" diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h index 8c864e8c68..1201d778d0 100644 --- a/target/hexagon/mmvec/macros.h +++ b/target/hexagon/mmvec/macros.h @@ -18,7 +18,6 @@ #ifndef HEXAGON_MMVEC_MACROS_H #define HEXAGON_MMVEC_MACROS_H -#include "qemu/osdep.h" #include "qemu/host-utils.h" #include "arch.h" #include "mmvec/system_ext_mmvec.h" diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h index 3004ce37b6..0c819ca983 100644 --- a/target/riscv/pmu.h +++ b/target/riscv/pmu.h @@ -16,7 +16,6 @@ * this program. If not, see <http://www.gnu.org/licenses/>. */ -#include "qemu/osdep.h" #include "qemu/log.h" #include "cpu.h" #include "qemu/main-loop.h" diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c index 2b1dd745d1..9734407543 100644 --- a/bsd-user/arm/signal.c +++ b/bsd-user/arm/signal.c @@ -17,6 +17,7 @@ * along with this program; if not, see <http://www.gnu.org/licenses/>. */ +#include "qemu/osdep.h" #include "qemu.h" /* diff --git a/bsd-user/arm/target_arch_cpu.c b/bsd-user/arm/target_arch_cpu.c index 02bf9149d5..fe38ae2210 100644 --- a/bsd-user/arm/target_arch_cpu.c +++ b/bsd-user/arm/target_arch_cpu.c @@ -16,6 +16,8 @@ * You should have received a copy of the GNU General Public License * along with this program; if not, see <http://www.gnu.org/licenses/>. */ + +#include "qemu/osdep.h" #include "target_arch.h" void target_cpu_set_tls(CPUARMState *env, target_ulong newtls) diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c index 309e27b9d6..1676ec10f8 100644 --- a/bsd-user/freebsd/os-sys.c +++ b/bsd-user/freebsd/os-sys.c @@ -17,6 +17,7 @@ * along with this program; if not, see <http://www.gnu.org/licenses/>. */ +#include "qemu/osdep.h" #include "qemu.h" #include "target_arch_sysarch.h" diff --git a/bsd-user/i386/signal.c b/bsd-user/i386/signal.c index 5dd975ce56..a3131047b8 100644 --- a/bsd-user/i386/signal.c +++ b/bsd-user/i386/signal.c @@ -17,6 +17,7 @@ * along with this program; if not, see <http://www.gnu.org/licenses/>. */ +#include "qemu/osdep.h" #include "qemu.h" /* diff --git a/bsd-user/x86_64/signal.c b/bsd-user/x86_64/signal.c index c3875bc4c6..46cb865180 100644 --- a/bsd-user/x86_64/signal.c +++ b/bsd-user/x86_64/signal.c @@ -16,6 +16,7 @@ * along with this program; if not, see <http://www.gnu.org/licenses/>. */ +#include "qemu/osdep.h" #include "qemu.h" /* diff --git a/qga/cutils.c b/qga/cutils.c index b8e142ef64..b21bcf3683 100644 --- a/qga/cutils.c +++ b/qga/cutils.c @@ -2,8 +2,9 @@ * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. */ -#include "cutils.h" +#include "qemu/osdep.h" +#include "cutils.h" #include "qapi/error.h" /** -- 2.39.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 11:50 ` [PATCH v3 1/1] include: Don't include qemu/osdep.h Markus Armbruster @ 2023-01-12 13:51 ` Michael S. Tsirkin 2023-01-12 13:56 ` Michael S. Tsirkin 2023-01-12 14:52 ` Daniel P. Berrangé 2023-01-12 17:30 ` Jonathan Cameron via 2023-01-12 17:41 ` Michael S. Tsirkin 2 siblings, 2 replies; 17+ messages in thread From: Michael S. Tsirkin @ 2023-01-12 13:51 UTC (permalink / raw) To: Markus Armbruster Cc: qemu-devel, imp, kevans, berrange, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: > docs/devel/style.rst mandates: > > The "qemu/osdep.h" header contains preprocessor macros that affect > the behavior of core system headers like <stdint.h>. It must be > the first include so that core system headers included by external > libraries get the preprocessor macros that QEMU depends on. > > Do not include "qemu/osdep.h" from header files since the .c file > will have already included it. > > A few violations have crept in. Fix them. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> With my awesome grep skillz I found one more: $ grep -r --include='*.h' qemu/osdep.h include/block/graph-lock.h:#include "qemu/osdep.h" Looks like all C files must include qemu/osdep.h, no? How about 1- add -include qemu/osdep.h on compile command line drop #include "qemu/osdep.h" from C files 2- drop double include guards, replace with a warning. following patch implements part 2: qemu/osdep: don't include it from headers doing so will lead to trouble eventually - instead of working around such cases make it more likely it will fail. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 7d059ad526..e4a60f911c 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -24,7 +24,12 @@ * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. */ -#ifndef QEMU_OSDEP_H +#ifdef QEMU_OSDEP_H +#warning "Never include qemu/osdep.h from a header!" +#endif + +static inline void qemu_osdep_never_include_from_header(void) {} + #define QEMU_OSDEP_H #include "config-host.h" @@ -714,5 +719,3 @@ static inline int platform_does_not_support_system(const char *command) #ifdef __cplusplus } #endif - -#endif ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 13:51 ` Michael S. Tsirkin @ 2023-01-12 13:56 ` Michael S. Tsirkin 2023-01-12 14:47 ` Markus Armbruster 2023-01-12 14:52 ` Daniel P. Berrangé 1 sibling, 1 reply; 17+ messages in thread From: Michael S. Tsirkin @ 2023-01-12 13:56 UTC (permalink / raw) To: Markus Armbruster Cc: qemu-devel, imp, kevans, berrange, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, Jan 12, 2023 at 08:51:32AM -0500, Michael S. Tsirkin wrote: > On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: > > docs/devel/style.rst mandates: > > > > The "qemu/osdep.h" header contains preprocessor macros that affect > > the behavior of core system headers like <stdint.h>. It must be > > the first include so that core system headers included by external > > libraries get the preprocessor macros that QEMU depends on. > > > > Do not include "qemu/osdep.h" from header files since the .c file > > will have already included it. > > > > A few violations have crept in. Fix them. > > > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > With my awesome grep skillz I found one more: > $ grep -r --include='*.h' qemu/osdep.h > include/block/graph-lock.h:#include "qemu/osdep.h" Also: $ grep -r --include='*.inc' qemu/osdep.h ui/vnc-enc-zrle.c.inc:#include "qemu/osdep.h" crypto/akcipher-nettle.c.inc:#include "qemu/osdep.h" crypto/akcipher-gcrypt.c.inc:#include "qemu/osdep.h" crypto/rsakey-nettle.c.inc:#include "qemu/osdep.h" crypto/cipher-gnutls.c.inc:#include "qemu/osdep.h" target/xtensa/core-dc233c/xtensa-modules.c.inc:#include "qemu/osdep.h" target/xtensa/core-sample_controller/xtensa-modules.c.inc:#include "qemu/osdep.h" target/xtensa/core-de212/xtensa-modules.c.inc:#include "qemu/osdep.h" target/xtensa/core-dc232b/xtensa-modules.c.inc:#include "qemu/osdep.h" target/xtensa/core-fsf/xtensa-modules.c.inc:#include "qemu/osdep.h" target/cris/translate_v10.c.inc:#include "qemu/osdep.h" > Looks like all C files must include qemu/osdep.h, no? > How about > > 1- add -include qemu/osdep.h on compile command line > drop #include "qemu/osdep.h" from C files > 2- drop double include guards, replace with a warning. > > following patch implements part 2: > > > qemu/osdep: don't include it from headers > > doing so will lead to trouble eventually - instead of > working around such cases make it more likely it will fail. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 7d059ad526..e4a60f911c 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -24,7 +24,12 @@ > * This work is licensed under the terms of the GNU GPL, version 2 or later. > * See the COPYING file in the top-level directory. > */ > -#ifndef QEMU_OSDEP_H > +#ifdef QEMU_OSDEP_H > +#warning "Never include qemu/osdep.h from a header!" > +#endif > + > +static inline void qemu_osdep_never_include_from_header(void) {} > + > #define QEMU_OSDEP_H > > #include "config-host.h" > @@ -714,5 +719,3 @@ static inline int platform_does_not_support_system(const char *command) > #ifdef __cplusplus > } > #endif > - > -#endif ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 13:56 ` Michael S. Tsirkin @ 2023-01-12 14:47 ` Markus Armbruster 2023-01-12 17:37 ` Michael S. Tsirkin 0 siblings, 1 reply; 17+ messages in thread From: Markus Armbruster @ 2023-01-12 14:47 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Markus Armbruster, qemu-devel, imp, kevans, berrange, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng "Michael S. Tsirkin" <mst@redhat.com> writes: > On Thu, Jan 12, 2023 at 08:51:32AM -0500, Michael S. Tsirkin wrote: >> On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: >> > docs/devel/style.rst mandates: >> > >> > The "qemu/osdep.h" header contains preprocessor macros that affect >> > the behavior of core system headers like <stdint.h>. It must be >> > the first include so that core system headers included by external >> > libraries get the preprocessor macros that QEMU depends on. >> > >> > Do not include "qemu/osdep.h" from header files since the .c file >> > will have already included it. >> > >> > A few violations have crept in. Fix them. >> > >> > Signed-off-by: Markus Armbruster <armbru@redhat.com> >> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> >> > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> >> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> >> >> With my awesome grep skillz I found one more: >> $ grep -r --include='*.h' qemu/osdep.h >> include/block/graph-lock.h:#include "qemu/osdep.h" Crept in after I prepared my v1. I neglected to re-check. > Also: > $ grep -r --include='*.inc' qemu/osdep.h > ui/vnc-enc-zrle.c.inc:#include "qemu/osdep.h" > crypto/akcipher-nettle.c.inc:#include "qemu/osdep.h" > crypto/akcipher-gcrypt.c.inc:#include "qemu/osdep.h" > crypto/rsakey-nettle.c.inc:#include "qemu/osdep.h" > crypto/cipher-gnutls.c.inc:#include "qemu/osdep.h" > target/xtensa/core-dc233c/xtensa-modules.c.inc:#include "qemu/osdep.h" > target/xtensa/core-sample_controller/xtensa-modules.c.inc:#include "qemu/osdep.h" > target/xtensa/core-de212/xtensa-modules.c.inc:#include "qemu/osdep.h" > target/xtensa/core-dc232b/xtensa-modules.c.inc:#include "qemu/osdep.h" > target/xtensa/core-fsf/xtensa-modules.c.inc:#include "qemu/osdep.h" > target/cris/translate_v10.c.inc:#include "qemu/osdep.h" Good point. Looks like I successfully supressed all memory of .inc. >> Looks like all C files must include qemu/osdep.h, no? I remember there are a few exceptions, but I don't remember which .c they are. Hmm... see commit 4bd802b209cff612d1a99674a91895b735be8630. >> How about >> >> 1- add -include qemu/osdep.h on compile command line >> drop #include "qemu/osdep.h" from C files Then you need to encode the exceptions in the build system. Which might not be a bad thing. >> 2- drop double include guards, replace with a warning. >> >> following patch implements part 2: >> >> >> qemu/osdep: don't include it from headers >> >> doing so will lead to trouble eventually - instead of >> working around such cases make it more likely it will fail. >> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >> >> --- >> >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h >> index 7d059ad526..e4a60f911c 100644 >> --- a/include/qemu/osdep.h >> +++ b/include/qemu/osdep.h >> @@ -24,7 +24,12 @@ >> * This work is licensed under the terms of the GNU GPL, version 2 or later. >> * See the COPYING file in the top-level directory. >> */ >> -#ifndef QEMU_OSDEP_H >> +#ifdef QEMU_OSDEP_H >> +#warning "Never include qemu/osdep.h from a header!" >> +#endif >> + >> +static inline void qemu_osdep_never_include_from_header(void) {} >> + Why do you need the function, too? >> #define QEMU_OSDEP_H >> >> #include "config-host.h" >> @@ -714,5 +719,3 @@ static inline int platform_does_not_support_system(const char *command) >> #ifdef __cplusplus >> } >> #endif >> - >> -#endif ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 14:47 ` Markus Armbruster @ 2023-01-12 17:37 ` Michael S. Tsirkin 2023-01-12 17:44 ` Daniel P. Berrangé 0 siblings, 1 reply; 17+ messages in thread From: Michael S. Tsirkin @ 2023-01-12 17:37 UTC (permalink / raw) To: Markus Armbruster Cc: qemu-devel, imp, kevans, berrange, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, Jan 12, 2023 at 03:47:19PM +0100, Markus Armbruster wrote: > "Michael S. Tsirkin" <mst@redhat.com> writes: > > > On Thu, Jan 12, 2023 at 08:51:32AM -0500, Michael S. Tsirkin wrote: > >> On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: > >> > docs/devel/style.rst mandates: > >> > > >> > The "qemu/osdep.h" header contains preprocessor macros that affect > >> > the behavior of core system headers like <stdint.h>. It must be > >> > the first include so that core system headers included by external > >> > libraries get the preprocessor macros that QEMU depends on. > >> > > >> > Do not include "qemu/osdep.h" from header files since the .c file > >> > will have already included it. > >> > > >> > A few violations have crept in. Fix them. > >> > > >> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > >> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > >> > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > >> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > >> > >> With my awesome grep skillz I found one more: > >> $ grep -r --include='*.h' qemu/osdep.h > >> include/block/graph-lock.h:#include "qemu/osdep.h" > > Crept in after I prepared my v1. I neglected to re-check. > > > Also: > > $ grep -r --include='*.inc' qemu/osdep.h > > ui/vnc-enc-zrle.c.inc:#include "qemu/osdep.h" > > crypto/akcipher-nettle.c.inc:#include "qemu/osdep.h" > > crypto/akcipher-gcrypt.c.inc:#include "qemu/osdep.h" > > crypto/rsakey-nettle.c.inc:#include "qemu/osdep.h" > > crypto/cipher-gnutls.c.inc:#include "qemu/osdep.h" > > target/xtensa/core-dc233c/xtensa-modules.c.inc:#include "qemu/osdep.h" > > target/xtensa/core-sample_controller/xtensa-modules.c.inc:#include "qemu/osdep.h" > > target/xtensa/core-de212/xtensa-modules.c.inc:#include "qemu/osdep.h" > > target/xtensa/core-dc232b/xtensa-modules.c.inc:#include "qemu/osdep.h" > > target/xtensa/core-fsf/xtensa-modules.c.inc:#include "qemu/osdep.h" > > target/cris/translate_v10.c.inc:#include "qemu/osdep.h" > > Good point. Looks like I successfully supressed all memory of .inc. > > >> Looks like all C files must include qemu/osdep.h, no? > > I remember there are a few exceptions, but I don't remember which .c > they are. Hmm... see commit 4bd802b209cff612d1a99674a91895b735be8630. > > >> How about > >> > >> 1- add -include qemu/osdep.h on compile command line > >> drop #include "qemu/osdep.h" from C files > > Then you need to encode the exceptions in the build system. Which might > not be a bad thing. > > >> 2- drop double include guards, replace with a warning. > >> > >> following patch implements part 2: > >> > >> > >> qemu/osdep: don't include it from headers > >> > >> doing so will lead to trouble eventually - instead of > >> working around such cases make it more likely it will fail. > >> > >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > >> > >> --- > >> > >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > >> index 7d059ad526..e4a60f911c 100644 > >> --- a/include/qemu/osdep.h > >> +++ b/include/qemu/osdep.h > >> @@ -24,7 +24,12 @@ > >> * This work is licensed under the terms of the GNU GPL, version 2 or later. > >> * See the COPYING file in the top-level directory. > >> */ > >> -#ifndef QEMU_OSDEP_H > >> +#ifdef QEMU_OSDEP_H > >> +#warning "Never include qemu/osdep.h from a header!" > >> +#endif > >> + > >> +static inline void qemu_osdep_never_include_from_header(void) {} > >> + > > Why do you need the function, too? This seems to give a bit more info if header does get included twice: instead of just a warning on the second include compiler says definition is duplicated and then shows where the first definition was. OTOH first one was almost for sure from the proper first include so maybe we don't care. Let me drop this. > >> #define QEMU_OSDEP_H > >> > >> #include "config-host.h" > >> @@ -714,5 +719,3 @@ static inline int platform_does_not_support_system(const char *command) > >> #ifdef __cplusplus > >> } > >> #endif > >> - > >> -#endif ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 17:37 ` Michael S. Tsirkin @ 2023-01-12 17:44 ` Daniel P. Berrangé 2023-01-12 17:47 ` Michael S. Tsirkin 0 siblings, 1 reply; 17+ messages in thread From: Daniel P. Berrangé @ 2023-01-12 17:44 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Markus Armbruster, qemu-devel, imp, kevans, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, Jan 12, 2023 at 12:37:46PM -0500, Michael S. Tsirkin wrote: > On Thu, Jan 12, 2023 at 03:47:19PM +0100, Markus Armbruster wrote: > > "Michael S. Tsirkin" <mst@redhat.com> writes: > > > > > On Thu, Jan 12, 2023 at 08:51:32AM -0500, Michael S. Tsirkin wrote: > > >> On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: > > >> > docs/devel/style.rst mandates: > > >> > > > >> > The "qemu/osdep.h" header contains preprocessor macros that affect > > >> > the behavior of core system headers like <stdint.h>. It must be > > >> > the first include so that core system headers included by external > > >> > libraries get the preprocessor macros that QEMU depends on. > > >> > > > >> > Do not include "qemu/osdep.h" from header files since the .c file > > >> > will have already included it. > > >> > > > >> > A few violations have crept in. Fix them. > > >> > > > >> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > > >> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > >> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > >> > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > > >> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > >> > > >> With my awesome grep skillz I found one more: > > >> $ grep -r --include='*.h' qemu/osdep.h > > >> include/block/graph-lock.h:#include "qemu/osdep.h" > > > > Crept in after I prepared my v1. I neglected to re-check. > > > > > Also: > > > $ grep -r --include='*.inc' qemu/osdep.h > > > ui/vnc-enc-zrle.c.inc:#include "qemu/osdep.h" > > > crypto/akcipher-nettle.c.inc:#include "qemu/osdep.h" > > > crypto/akcipher-gcrypt.c.inc:#include "qemu/osdep.h" > > > crypto/rsakey-nettle.c.inc:#include "qemu/osdep.h" > > > crypto/cipher-gnutls.c.inc:#include "qemu/osdep.h" > > > target/xtensa/core-dc233c/xtensa-modules.c.inc:#include "qemu/osdep.h" > > > target/xtensa/core-sample_controller/xtensa-modules.c.inc:#include "qemu/osdep.h" > > > target/xtensa/core-de212/xtensa-modules.c.inc:#include "qemu/osdep.h" > > > target/xtensa/core-dc232b/xtensa-modules.c.inc:#include "qemu/osdep.h" > > > target/xtensa/core-fsf/xtensa-modules.c.inc:#include "qemu/osdep.h" > > > target/cris/translate_v10.c.inc:#include "qemu/osdep.h" > > > > Good point. Looks like I successfully supressed all memory of .inc. > > > > >> Looks like all C files must include qemu/osdep.h, no? > > > > I remember there are a few exceptions, but I don't remember which .c > > they are. Hmm... see commit 4bd802b209cff612d1a99674a91895b735be8630. > > > > >> How about > > >> > > >> 1- add -include qemu/osdep.h on compile command line > > >> drop #include "qemu/osdep.h" from C files > > > > Then you need to encode the exceptions in the build system. Which might > > not be a bad thing. > > > > >> 2- drop double include guards, replace with a warning. > > >> > > >> following patch implements part 2: > > >> > > >> > > >> qemu/osdep: don't include it from headers > > >> > > >> doing so will lead to trouble eventually - instead of > > >> working around such cases make it more likely it will fail. > > >> > > >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > >> > > >> --- > > >> > > >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > > >> index 7d059ad526..e4a60f911c 100644 > > >> --- a/include/qemu/osdep.h > > >> +++ b/include/qemu/osdep.h > > >> @@ -24,7 +24,12 @@ > > >> * This work is licensed under the terms of the GNU GPL, version 2 or later. > > >> * See the COPYING file in the top-level directory. > > >> */ > > >> -#ifndef QEMU_OSDEP_H > > >> +#ifdef QEMU_OSDEP_H > > >> +#warning "Never include qemu/osdep.h from a header!" > > >> +#endif > > >> + > > >> +static inline void qemu_osdep_never_include_from_header(void) {} > > >> + > > > > Why do you need the function, too? > > This seems to give a bit more info if header does get included > twice: instead of just a warning on the second include compiler says > definition is duplicated and then shows where the first definition was. > OTOH first one was almost for sure from the proper first include so > maybe we don't care. Let me drop this. FWIW, if we want to simplify our header guards, we could replace the #ifndef FOO_H #define FOO_H .... #endif /* FOO_H */ with merely #pragma once at the top of each header. With 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 :| ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 17:44 ` Daniel P. Berrangé @ 2023-01-12 17:47 ` Michael S. Tsirkin 0 siblings, 0 replies; 17+ messages in thread From: Michael S. Tsirkin @ 2023-01-12 17:47 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Markus Armbruster, qemu-devel, imp, kevans, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, Jan 12, 2023 at 05:44:31PM +0000, Daniel P. Berrangé wrote: > On Thu, Jan 12, 2023 at 12:37:46PM -0500, Michael S. Tsirkin wrote: > > On Thu, Jan 12, 2023 at 03:47:19PM +0100, Markus Armbruster wrote: > > > "Michael S. Tsirkin" <mst@redhat.com> writes: > > > > > > > On Thu, Jan 12, 2023 at 08:51:32AM -0500, Michael S. Tsirkin wrote: > > > >> On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: > > > >> > docs/devel/style.rst mandates: > > > >> > > > > >> > The "qemu/osdep.h" header contains preprocessor macros that affect > > > >> > the behavior of core system headers like <stdint.h>. It must be > > > >> > the first include so that core system headers included by external > > > >> > libraries get the preprocessor macros that QEMU depends on. > > > >> > > > > >> > Do not include "qemu/osdep.h" from header files since the .c file > > > >> > will have already included it. > > > >> > > > > >> > A few violations have crept in. Fix them. > > > >> > > > > >> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > > > >> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > > >> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > > >> > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > > > >> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > >> > > > >> With my awesome grep skillz I found one more: > > > >> $ grep -r --include='*.h' qemu/osdep.h > > > >> include/block/graph-lock.h:#include "qemu/osdep.h" > > > > > > Crept in after I prepared my v1. I neglected to re-check. > > > > > > > Also: > > > > $ grep -r --include='*.inc' qemu/osdep.h > > > > ui/vnc-enc-zrle.c.inc:#include "qemu/osdep.h" > > > > crypto/akcipher-nettle.c.inc:#include "qemu/osdep.h" > > > > crypto/akcipher-gcrypt.c.inc:#include "qemu/osdep.h" > > > > crypto/rsakey-nettle.c.inc:#include "qemu/osdep.h" > > > > crypto/cipher-gnutls.c.inc:#include "qemu/osdep.h" > > > > target/xtensa/core-dc233c/xtensa-modules.c.inc:#include "qemu/osdep.h" > > > > target/xtensa/core-sample_controller/xtensa-modules.c.inc:#include "qemu/osdep.h" > > > > target/xtensa/core-de212/xtensa-modules.c.inc:#include "qemu/osdep.h" > > > > target/xtensa/core-dc232b/xtensa-modules.c.inc:#include "qemu/osdep.h" > > > > target/xtensa/core-fsf/xtensa-modules.c.inc:#include "qemu/osdep.h" > > > > target/cris/translate_v10.c.inc:#include "qemu/osdep.h" > > > > > > Good point. Looks like I successfully supressed all memory of .inc. > > > > > > >> Looks like all C files must include qemu/osdep.h, no? > > > > > > I remember there are a few exceptions, but I don't remember which .c > > > they are. Hmm... see commit 4bd802b209cff612d1a99674a91895b735be8630. > > > > > > >> How about > > > >> > > > >> 1- add -include qemu/osdep.h on compile command line > > > >> drop #include "qemu/osdep.h" from C files > > > > > > Then you need to encode the exceptions in the build system. Which might > > > not be a bad thing. > > > > > > >> 2- drop double include guards, replace with a warning. > > > >> > > > >> following patch implements part 2: > > > >> > > > >> > > > >> qemu/osdep: don't include it from headers > > > >> > > > >> doing so will lead to trouble eventually - instead of > > > >> working around such cases make it more likely it will fail. > > > >> > > > >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > >> > > > >> --- > > > >> > > > >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > > > >> index 7d059ad526..e4a60f911c 100644 > > > >> --- a/include/qemu/osdep.h > > > >> +++ b/include/qemu/osdep.h > > > >> @@ -24,7 +24,12 @@ > > > >> * This work is licensed under the terms of the GNU GPL, version 2 or later. > > > >> * See the COPYING file in the top-level directory. > > > >> */ > > > >> -#ifndef QEMU_OSDEP_H > > > >> +#ifdef QEMU_OSDEP_H > > > >> +#warning "Never include qemu/osdep.h from a header!" > > > >> +#endif > > > >> + > > > >> +static inline void qemu_osdep_never_include_from_header(void) {} > > > >> + > > > > > > Why do you need the function, too? > > > > This seems to give a bit more info if header does get included > > twice: instead of just a warning on the second include compiler says > > definition is duplicated and then shows where the first definition was. > > OTOH first one was almost for sure from the proper first include so > > maybe we don't care. Let me drop this. > > FWIW, if we want to simplify our header guards, we could replace the > > #ifndef FOO_H > #define FOO_H > > .... > > #endif /* FOO_H */ > > with merely > > #pragma once > > at the top of each header. > > With regards, > Daniel Will break this trick, won't it? -- MST ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 13:51 ` Michael S. Tsirkin 2023-01-12 13:56 ` Michael S. Tsirkin @ 2023-01-12 14:52 ` Daniel P. Berrangé 2023-01-12 15:58 ` Peter Maydell 1 sibling, 1 reply; 17+ messages in thread From: Daniel P. Berrangé @ 2023-01-12 14:52 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Markus Armbruster, qemu-devel, imp, kevans, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, Jan 12, 2023 at 08:51:26AM -0500, Michael S. Tsirkin wrote: > On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: > > docs/devel/style.rst mandates: > > > > The "qemu/osdep.h" header contains preprocessor macros that affect > > the behavior of core system headers like <stdint.h>. It must be > > the first include so that core system headers included by external > > libraries get the preprocessor macros that QEMU depends on. > > > > Do not include "qemu/osdep.h" from header files since the .c file > > will have already included it. > > > > A few violations have crept in. Fix them. > > > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > With my awesome grep skillz I found one more: > $ grep -r --include='*.h' qemu/osdep.h > include/block/graph-lock.h:#include "qemu/osdep.h" > > Looks like all C files must include qemu/osdep.h, no? Yes, and IMHO that is/was a mistake, as it means our other header files are not self-contained, which prevents developer tools from reporting useful bugs when you're editting. For example, if you have clangd integrated into your editor, it will warn you as you're editting if you've referenced a function / type that doesn't exist in the file, or anything it includes. This is made completely useless for QEMU .h files though, as they're all incomplete, only the .c files have the full headers. With 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 :| ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 14:52 ` Daniel P. Berrangé @ 2023-01-12 15:58 ` Peter Maydell 2023-01-12 16:07 ` Daniel P. Berrangé 2023-01-12 17:43 ` Michael S. Tsirkin 0 siblings, 2 replies; 17+ messages in thread From: Peter Maydell @ 2023-01-12 15:58 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Michael S. Tsirkin, Markus Armbruster, qemu-devel, imp, kevans, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, 12 Jan 2023 at 15:14, Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Thu, Jan 12, 2023 at 08:51:26AM -0500, Michael S. Tsirkin wrote: > > On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: > > > docs/devel/style.rst mandates: > > > > > > The "qemu/osdep.h" header contains preprocessor macros that affect > > > the behavior of core system headers like <stdint.h>. It must be > > > the first include so that core system headers included by external > > > libraries get the preprocessor macros that QEMU depends on. > > > > > > Do not include "qemu/osdep.h" from header files since the .c file > > > will have already included it. > > > > > > A few violations have crept in. Fix them. > > > > > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > > With my awesome grep skillz I found one more: > > $ grep -r --include='*.h' qemu/osdep.h > > include/block/graph-lock.h:#include "qemu/osdep.h" > > > > Looks like all C files must include qemu/osdep.h, no? > > Yes, and IMHO that is/was a mistake, as it means our other header > files are not self-contained, which prevents developer tools from > reporting useful bugs when you're editting. The underlying requirement is "osdep.h must be included before any system header file". "Always first in every .c file" is an easy way to achieve that, and "never in any .h file" is then not mandatory but falls out from the fact that any such include is pointless and only serves to increase the compilation time (and to increase the chances that you don't notice that you forgot osdep.h in your .c file). If there's a better way to do this (e.g. one which meant that it was a compile error to put osdep includes in the wrong place or to omit them) then that would certainly save us periodically having to do this kind of fixup commit. thanks -- PMM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 15:58 ` Peter Maydell @ 2023-01-12 16:07 ` Daniel P. Berrangé 2023-01-12 16:20 ` Peter Maydell 2023-01-12 17:43 ` Michael S. Tsirkin 1 sibling, 1 reply; 17+ messages in thread From: Daniel P. Berrangé @ 2023-01-12 16:07 UTC (permalink / raw) To: Peter Maydell Cc: Michael S. Tsirkin, Markus Armbruster, qemu-devel, imp, kevans, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, Jan 12, 2023 at 03:58:56PM +0000, Peter Maydell wrote: > On Thu, 12 Jan 2023 at 15:14, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > On Thu, Jan 12, 2023 at 08:51:26AM -0500, Michael S. Tsirkin wrote: > > > On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: > > > > docs/devel/style.rst mandates: > > > > > > > > The "qemu/osdep.h" header contains preprocessor macros that affect > > > > the behavior of core system headers like <stdint.h>. It must be > > > > the first include so that core system headers included by external > > > > libraries get the preprocessor macros that QEMU depends on. > > > > > > > > Do not include "qemu/osdep.h" from header files since the .c file > > > > will have already included it. > > > > > > > > A few violations have crept in. Fix them. > > > > > > > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > > > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > > > > With my awesome grep skillz I found one more: > > > $ grep -r --include='*.h' qemu/osdep.h > > > include/block/graph-lock.h:#include "qemu/osdep.h" > > > > > > Looks like all C files must include qemu/osdep.h, no? > > > > Yes, and IMHO that is/was a mistake, as it means our other header > > files are not self-contained, which prevents developer tools from > > reporting useful bugs when you're editting. > > The underlying requirement is "osdep.h must be included > before any system header file". "Always first in every .c file" > is an easy way to achieve that, and "never in any .h file" is > then not mandatory but falls out from the fact that any > such include is pointless and only serves to increase > the compilation time (and to increase the chances that > you don't notice that you forgot osdep.h in your .c file). > > If there's a better way to do this (e.g. one which meant > that it was a compile error to put osdep includes in the > wrong place or to omit them) then that would certainly > save us periodically having to do this kind of fixup commit. I think the challenge is that osdep.h is too big as it exists today. The stuff the needs to come before system headers is actually little more than config-host.h and a few #defines most of which are specific to windows. If those critical #defines went into config-host.h, then we could have a rule 'config-host.h' must be included in all .c files as the first thing. All the header files could just reference the specific system headers they care about instead of making everything from osdep.h visible in their namespace. Still this would be quite a lot of work to adapt to at this point. With 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 :| ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 16:07 ` Daniel P. Berrangé @ 2023-01-12 16:20 ` Peter Maydell 2023-01-12 16:30 ` Daniel P. Berrangé 0 siblings, 1 reply; 17+ messages in thread From: Peter Maydell @ 2023-01-12 16:20 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Michael S. Tsirkin, Markus Armbruster, qemu-devel, imp, kevans, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, 12 Jan 2023 at 16:08, Daniel P. Berrangé <berrange@redhat.com> wrote: > I think the challenge is that osdep.h is too big as it exists > today. The stuff the needs to come before system headers is > actually little more than config-host.h and a few #defines > most of which are specific to windows. If those critical > #defines went into config-host.h, then we could have a rule > 'config-host.h' must be included in all .c files as the first > thing. This doesn't seem much different to the rules we have today, except you've renamed osdep.h to config-host.h... > All the header files could just reference the specific > system headers they care about instead of making everything > from osdep.h visible in their namespace. Still this would be > quite a lot of work to adapt to at this point. It certainly does have more in it than strictly necessary, though we have thinned it out quite a bit from when we first put in the convention. A lot of the functions at the tail end of the file could be moved out into their own headers, for instance -- patches welcome ;-) > All the header files could just reference the specific > system headers they care about instead of making everything > from osdep.h visible in their namespace. There are some complicated things in there, not always limited to Windows. Also where there is some header that needs a platform-specific workaround I prefer that that header is pulled in by osdep.h. This avoids the failure mode of "developer working on Linux directly includes some-system-header.h; works fine on their machine, but doesn't work on oddball-platform where the header needs a workaround". (For instance, handling "sys/mman.h on this system doesn't define MAP_ANONYMOUS", or the backcompat stuff in glib-compat.h.) thanks -- PMM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 16:20 ` Peter Maydell @ 2023-01-12 16:30 ` Daniel P. Berrangé 2023-01-12 16:38 ` Peter Maydell 0 siblings, 1 reply; 17+ messages in thread From: Daniel P. Berrangé @ 2023-01-12 16:30 UTC (permalink / raw) To: Peter Maydell Cc: Michael S. Tsirkin, Markus Armbruster, qemu-devel, imp, kevans, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, Jan 12, 2023 at 04:20:36PM +0000, Peter Maydell wrote: > On Thu, 12 Jan 2023 at 16:08, Daniel P. Berrangé <berrange@redhat.com> wrote: > > I think the challenge is that osdep.h is too big as it exists > > today. The stuff the needs to come before system headers is > > actually little more than config-host.h and a few #defines > > most of which are specific to windows. If those critical > > #defines went into config-host.h, then we could have a rule > > 'config-host.h' must be included in all .c files as the first > > thing. > > This doesn't seem much different to the rules we have today, > except you've renamed osdep.h to config-host.h... If the QEMU header files all contain #includes for the system headers they rely on, then when tools are validating code in the header, they can stand a better chance of being able to resolve all the types. Though it'll still fail if some of the system header pieces only get exposed as a result of config-host.h macros, but that's relatively few, compared to today where amost nothing resolves if yuo validate the headers files in isolation. > > All the header files could just reference the specific > > system headers they care about instead of making everything > > from osdep.h visible in their namespace. > > There are some complicated things in there, not always > limited to Windows. Also where there is some header > that needs a platform-specific workaround I prefer that > that header is pulled in by osdep.h. This avoids the > failure mode of "developer working on Linux directly > includes some-system-header.h; works fine on their machine, > but doesn't work on oddball-platform where the header > needs a workaround". (For instance, handling "sys/mman.h > on this system doesn't define MAP_ANONYMOUS", or the > backcompat stuff in glib-compat.h.) Yeah, its not entirely straightforward, though our CI will catch that on our most important target platforms. With 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 :| ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 16:30 ` Daniel P. Berrangé @ 2023-01-12 16:38 ` Peter Maydell 0 siblings, 0 replies; 17+ messages in thread From: Peter Maydell @ 2023-01-12 16:38 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Michael S. Tsirkin, Markus Armbruster, qemu-devel, imp, kevans, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, 12 Jan 2023 at 16:30, Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Thu, Jan 12, 2023 at 04:20:36PM +0000, Peter Maydell wrote: > > On Thu, 12 Jan 2023 at 16:08, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > I think the challenge is that osdep.h is too big as it exists > > > today. The stuff the needs to come before system headers is > > > actually little more than config-host.h and a few #defines > > > most of which are specific to windows. If those critical > > > #defines went into config-host.h, then we could have a rule > > > 'config-host.h' must be included in all .c files as the first > > > thing. > > > > This doesn't seem much different to the rules we have today, > > except you've renamed osdep.h to config-host.h... > > If the QEMU header files all contain #includes for the > system headers they rely on, then when tools are validating > code in the header, they can stand a better chance of being > able to resolve all the types. Though it'll still fail if > some of the system header pieces only get exposed as a result > of config-host.h macros, but that's relatively few, compared > to today where amost nothing resolves if yuo validate the > headers files in isolation. Yeah, but I don't want QEMU header files to contain lots of includes for system header files, because of... > > There are some complicated things in there, not always > > limited to Windows. Also where there is some header > > that needs a platform-specific workaround I prefer that > > that header is pulled in by osdep.h. This avoids the > > failure mode of "developer working on Linux directly > > includes some-system-header.h; works fine on their machine, > > but doesn't work on oddball-platform where the header > > needs a workaround". (For instance, handling "sys/mman.h > > on this system doesn't define MAP_ANONYMOUS", or the > > backcompat stuff in glib-compat.h.) ...this. So we'd have to have config-host.h include all the system headers we're working around anyway. -- PMM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 15:58 ` Peter Maydell 2023-01-12 16:07 ` Daniel P. Berrangé @ 2023-01-12 17:43 ` Michael S. Tsirkin 1 sibling, 0 replies; 17+ messages in thread From: Michael S. Tsirkin @ 2023-01-12 17:43 UTC (permalink / raw) To: Peter Maydell Cc: Daniel P. Berrangé, Markus Armbruster, qemu-devel, imp, kevans, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, Jan 12, 2023 at 03:58:56PM +0000, Peter Maydell wrote: > On Thu, 12 Jan 2023 at 15:14, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > On Thu, Jan 12, 2023 at 08:51:26AM -0500, Michael S. Tsirkin wrote: > > > On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: > > > > docs/devel/style.rst mandates: > > > > > > > > The "qemu/osdep.h" header contains preprocessor macros that affect > > > > the behavior of core system headers like <stdint.h>. It must be > > > > the first include so that core system headers included by external > > > > libraries get the preprocessor macros that QEMU depends on. > > > > > > > > Do not include "qemu/osdep.h" from header files since the .c file > > > > will have already included it. > > > > > > > > A few violations have crept in. Fix them. > > > > > > > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > > > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > > > > With my awesome grep skillz I found one more: > > > $ grep -r --include='*.h' qemu/osdep.h > > > include/block/graph-lock.h:#include "qemu/osdep.h" > > > > > > Looks like all C files must include qemu/osdep.h, no? > > > > Yes, and IMHO that is/was a mistake, as it means our other header > > files are not self-contained, which prevents developer tools from > > reporting useful bugs when you're editting. > > The underlying requirement is "osdep.h must be included > before any system header file". "Always first in every .c file" > is an easy way to achieve that, and "never in any .h file" is > then not mandatory but falls out from the fact that any > such include is pointless and only serves to increase > the compilation time (and to increase the chances that > you don't notice that you forgot osdep.h in your .c file). > > If there's a better way to do this (e.g. one which meant > that it was a compile error to put osdep includes in the > wrong place or to omit them) then that would certainly > save us periodically having to do this kind of fixup commit. > > thanks > -- PMM yes I just posted a patch that will catch most (though not all) such cases. if we switch to -include it will catch all of them but there seems to be some resistance to this idea. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 11:50 ` [PATCH v3 1/1] include: Don't include qemu/osdep.h Markus Armbruster 2023-01-12 13:51 ` Michael S. Tsirkin @ 2023-01-12 17:30 ` Jonathan Cameron via 2023-01-12 17:41 ` Michael S. Tsirkin 2 siblings, 0 replies; 17+ messages in thread From: Jonathan Cameron via @ 2023-01-12 17:30 UTC (permalink / raw) To: Markus Armbruster Cc: qemu-devel, imp, kevans, berrange, ben.widawsky, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, mst, Bin Meng On Thu, 12 Jan 2023 12:50:05 +0100 Markus Armbruster <armbru@redhat.com> wrote: > docs/devel/style.rst mandates: > > The "qemu/osdep.h" header contains preprocessor macros that affect > the behavior of core system headers like <stdint.h>. It must be > the first include so that core system headers included by external > libraries get the preprocessor macros that QEMU depends on. > > Do not include "qemu/osdep.h" from header files since the .c file > will have already included it. > > A few violations have crept in. Fix them. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> For the CXL one. Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > bsd-user/qemu.h | 1 - > crypto/block-luks-priv.h | 1 - > include/hw/cxl/cxl_host.h | 1 - > include/hw/input/pl050.h | 1 - > include/hw/tricore/triboard.h | 1 - > include/qemu/userfaultfd.h | 1 - > net/vmnet_int.h | 1 - > qga/cutils.h | 1 - > target/hexagon/hex_arch_types.h | 1 - > target/hexagon/mmvec/macros.h | 1 - > target/riscv/pmu.h | 1 - > bsd-user/arm/signal.c | 1 + > bsd-user/arm/target_arch_cpu.c | 2 ++ > bsd-user/freebsd/os-sys.c | 1 + > bsd-user/i386/signal.c | 1 + > bsd-user/x86_64/signal.c | 1 + > qga/cutils.c | 3 ++- > 17 files changed, 8 insertions(+), 12 deletions(-) > > diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h > index be6105385e..0ceecfb6df 100644 > --- a/bsd-user/qemu.h > +++ b/bsd-user/qemu.h > @@ -17,7 +17,6 @@ > #ifndef QEMU_H > #define QEMU_H > > -#include "qemu/osdep.h" > #include "cpu.h" > #include "qemu/units.h" > #include "exec/cpu_ldst.h" > diff --git a/crypto/block-luks-priv.h b/crypto/block-luks-priv.h > index 90a20d432b..1066df0307 100644 > --- a/crypto/block-luks-priv.h > +++ b/crypto/block-luks-priv.h > @@ -18,7 +18,6 @@ > * > */ > > -#include "qemu/osdep.h" > #include "qapi/error.h" > #include "qemu/bswap.h" > > diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h > index a1b662ce40..c9bc9c7c50 100644 > --- a/include/hw/cxl/cxl_host.h > +++ b/include/hw/cxl/cxl_host.h > @@ -7,7 +7,6 @@ > * COPYING file in the top-level directory. > */ > > -#include "qemu/osdep.h" > #include "hw/cxl/cxl.h" > #include "hw/boards.h" > > diff --git a/include/hw/input/pl050.h b/include/hw/input/pl050.h > index 89ec4fafc9..4cb8985f31 100644 > --- a/include/hw/input/pl050.h > +++ b/include/hw/input/pl050.h > @@ -10,7 +10,6 @@ > #ifndef HW_PL050_H > #define HW_PL050_H > > -#include "qemu/osdep.h" > #include "hw/sysbus.h" > #include "migration/vmstate.h" > #include "hw/input/ps2.h" > diff --git a/include/hw/tricore/triboard.h b/include/hw/tricore/triboard.h > index 094c8bd563..4fdd2d7d97 100644 > --- a/include/hw/tricore/triboard.h > +++ b/include/hw/tricore/triboard.h > @@ -18,7 +18,6 @@ > * License along with this library; if not, see <http://www.gnu.org/licenses/>. > */ > > -#include "qemu/osdep.h" > #include "qapi/error.h" > #include "hw/boards.h" > #include "sysemu/sysemu.h" > diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h > index 6b74f92792..55c95998e8 100644 > --- a/include/qemu/userfaultfd.h > +++ b/include/qemu/userfaultfd.h > @@ -13,7 +13,6 @@ > #ifndef USERFAULTFD_H > #define USERFAULTFD_H > > -#include "qemu/osdep.h" > #include "exec/hwaddr.h" > #include <linux/userfaultfd.h> > > diff --git a/net/vmnet_int.h b/net/vmnet_int.h > index adf6e8c20d..d0b90594f2 100644 > --- a/net/vmnet_int.h > +++ b/net/vmnet_int.h > @@ -10,7 +10,6 @@ > #ifndef VMNET_INT_H > #define VMNET_INT_H > > -#include "qemu/osdep.h" > #include "vmnet_int.h" > #include "clients.h" > > diff --git a/qga/cutils.h b/qga/cutils.h > index f0f30a7d28..2bfaf554a8 100644 > --- a/qga/cutils.h > +++ b/qga/cutils.h > @@ -1,7 +1,6 @@ > #ifndef CUTILS_H_ > #define CUTILS_H_ > > -#include "qemu/osdep.h" > > int qga_open_cloexec(const char *name, int flags, mode_t mode); > > diff --git a/target/hexagon/hex_arch_types.h b/target/hexagon/hex_arch_types.h > index 885f68f760..52a7f2b2f3 100644 > --- a/target/hexagon/hex_arch_types.h > +++ b/target/hexagon/hex_arch_types.h > @@ -18,7 +18,6 @@ > #ifndef HEXAGON_HEX_ARCH_TYPES_H > #define HEXAGON_HEX_ARCH_TYPES_H > > -#include "qemu/osdep.h" > #include "mmvec/mmvec.h" > #include "qemu/int128.h" > > diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h > index 8c864e8c68..1201d778d0 100644 > --- a/target/hexagon/mmvec/macros.h > +++ b/target/hexagon/mmvec/macros.h > @@ -18,7 +18,6 @@ > #ifndef HEXAGON_MMVEC_MACROS_H > #define HEXAGON_MMVEC_MACROS_H > > -#include "qemu/osdep.h" > #include "qemu/host-utils.h" > #include "arch.h" > #include "mmvec/system_ext_mmvec.h" > diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h > index 3004ce37b6..0c819ca983 100644 > --- a/target/riscv/pmu.h > +++ b/target/riscv/pmu.h > @@ -16,7 +16,6 @@ > * this program. If not, see <http://www.gnu.org/licenses/>. > */ > > -#include "qemu/osdep.h" > #include "qemu/log.h" > #include "cpu.h" > #include "qemu/main-loop.h" > diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c > index 2b1dd745d1..9734407543 100644 > --- a/bsd-user/arm/signal.c > +++ b/bsd-user/arm/signal.c > @@ -17,6 +17,7 @@ > * along with this program; if not, see <http://www.gnu.org/licenses/>. > */ > > +#include "qemu/osdep.h" > #include "qemu.h" > > /* > diff --git a/bsd-user/arm/target_arch_cpu.c b/bsd-user/arm/target_arch_cpu.c > index 02bf9149d5..fe38ae2210 100644 > --- a/bsd-user/arm/target_arch_cpu.c > +++ b/bsd-user/arm/target_arch_cpu.c > @@ -16,6 +16,8 @@ > * You should have received a copy of the GNU General Public License > * along with this program; if not, see <http://www.gnu.org/licenses/>. > */ > + > +#include "qemu/osdep.h" > #include "target_arch.h" > > void target_cpu_set_tls(CPUARMState *env, target_ulong newtls) > diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c > index 309e27b9d6..1676ec10f8 100644 > --- a/bsd-user/freebsd/os-sys.c > +++ b/bsd-user/freebsd/os-sys.c > @@ -17,6 +17,7 @@ > * along with this program; if not, see <http://www.gnu.org/licenses/>. > */ > > +#include "qemu/osdep.h" > #include "qemu.h" > #include "target_arch_sysarch.h" > > diff --git a/bsd-user/i386/signal.c b/bsd-user/i386/signal.c > index 5dd975ce56..a3131047b8 100644 > --- a/bsd-user/i386/signal.c > +++ b/bsd-user/i386/signal.c > @@ -17,6 +17,7 @@ > * along with this program; if not, see <http://www.gnu.org/licenses/>. > */ > > +#include "qemu/osdep.h" > #include "qemu.h" > > /* > diff --git a/bsd-user/x86_64/signal.c b/bsd-user/x86_64/signal.c > index c3875bc4c6..46cb865180 100644 > --- a/bsd-user/x86_64/signal.c > +++ b/bsd-user/x86_64/signal.c > @@ -16,6 +16,7 @@ > * along with this program; if not, see <http://www.gnu.org/licenses/>. > */ > > +#include "qemu/osdep.h" > #include "qemu.h" > > /* > diff --git a/qga/cutils.c b/qga/cutils.c > index b8e142ef64..b21bcf3683 100644 > --- a/qga/cutils.c > +++ b/qga/cutils.c > @@ -2,8 +2,9 @@ > * This work is licensed under the terms of the GNU GPL, version 2 or later. > * See the COPYING file in the top-level directory. > */ > -#include "cutils.h" > > +#include "qemu/osdep.h" > +#include "cutils.h" > #include "qapi/error.h" > > /** ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h 2023-01-12 11:50 ` [PATCH v3 1/1] include: Don't include qemu/osdep.h Markus Armbruster 2023-01-12 13:51 ` Michael S. Tsirkin 2023-01-12 17:30 ` Jonathan Cameron via @ 2023-01-12 17:41 ` Michael S. Tsirkin 2 siblings, 0 replies; 17+ messages in thread From: Michael S. Tsirkin @ 2023-01-12 17:41 UTC (permalink / raw) To: Markus Armbruster Cc: qemu-devel, imp, kevans, berrange, ben.widawsky, jonathan.cameron, kbastian, jasowang, michael.roth, kkostiuk, tsimpson, palmer, alistair.francis, bin.meng, qemu-riscv, philmd, Bin Meng On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote: > docs/devel/style.rst mandates: > > The "qemu/osdep.h" header contains preprocessor macros that affect > the behavior of core system headers like <stdint.h>. It must be > the first include so that core system headers included by external > libraries get the preprocessor macros that QEMU depends on. > > Do not include "qemu/osdep.h" from header files since the .c file > will have already included it. > > A few violations have crept in. Fix them. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> here's v2 - can be applied after you fix all instances of this. Feel free to use. ---> qemu/osdep: we don't include it from headers doing so will lead to trouble eventually - instead of silently working around such cases make it more likely it will fail. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> -- diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 7d059ad526..3ddeb7fd41 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -24,7 +24,9 @@ * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. */ -#ifndef QEMU_OSDEP_H +#ifdef QEMU_OSDEP_H +#warning "Never include qemu/osdep.h from a header!" +#else #define QEMU_OSDEP_H #include "config-host.h" ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-01-12 17:58 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-12 11:50 [PATCH v3 0/1] Clean up includes Markus Armbruster 2023-01-12 11:50 ` [PATCH v3 1/1] include: Don't include qemu/osdep.h Markus Armbruster 2023-01-12 13:51 ` Michael S. Tsirkin 2023-01-12 13:56 ` Michael S. Tsirkin 2023-01-12 14:47 ` Markus Armbruster 2023-01-12 17:37 ` Michael S. Tsirkin 2023-01-12 17:44 ` Daniel P. Berrangé 2023-01-12 17:47 ` Michael S. Tsirkin 2023-01-12 14:52 ` Daniel P. Berrangé 2023-01-12 15:58 ` Peter Maydell 2023-01-12 16:07 ` Daniel P. Berrangé 2023-01-12 16:20 ` Peter Maydell 2023-01-12 16:30 ` Daniel P. Berrangé 2023-01-12 16:38 ` Peter Maydell 2023-01-12 17:43 ` Michael S. Tsirkin 2023-01-12 17:30 ` Jonathan Cameron via 2023-01-12 17:41 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).