Skip to contentSkip to conversation

rdma: fix up include directives

PATCH In review Michael S. Tsirkin 5 participants 10 messages Mar 21, 2018 – Mar 22, 2018 Kbuild

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 rdma: fix up include directives

Our rule right now is to use <> for external headers only. RDMA code violates that, fix it up.

---
 hw/rdma/rdma_backend.h        |  2 +-
 hw/rdma/rdma_backend_defs.h   |  2 +-
 hw/rdma/rdma_rm.h             |  2 +-
 hw/rdma/rdma_utils.h          |  6 +++---
 hw/rdma/vmw/pvrdma.h          |  8 ++++----
 hw/rdma/vmw/pvrdma_dev_ring.h |  2 +-
 hw/rdma/rdma_backend.c        |  6 +++---
 hw/rdma/rdma_rm.c             |  6 +++---
 hw/rdma/vmw/pvrdma_cmd.c      |  8 ++++----
 hw/rdma/vmw/pvrdma_dev_ring.c |  8 ++++----
 hw/rdma/vmw/pvrdma_main.c     | 24 ++++++++++++------------
 hw/rdma/vmw/pvrdma_qp_ops.c   |  4 ++--
 12 files changed, 39 insertions(+), 39 deletions(-)
hw/rdma/rdma_backend.h +1 -1
index 68f2b05..abae262 100644
@@ -16,7 +16,7 @@
 #ifndef RDMA_BACKEND_H
 #define RDMA_BACKEND_H
 
-#include <qapi/error.h>
+#include "qapi/error.h"
 #include "rdma_rm_defs.h"
 #include "rdma_backend_defs.h"
 
hw/rdma/rdma_backend_defs.h +1 -1
index 837e324..ff5cfc2 100644
@@ -17,7 +17,7 @@
 #define RDMA_BACKEND_DEFS_H
 
 #include <infiniband/verbs.h>
-#include <qemu/thread.h>
+#include "qemu/thread.h"
 
 typedef struct RdmaDeviceResources RdmaDeviceResources;
 
hw/rdma/rdma_rm.h +1 -1
index be95c1b..82c1ddd 100644
@@ -16,7 +16,7 @@
 #ifndef RDMA_RM_H
 #define RDMA_RM_H
 
-#include <qapi/error.h>
+#include "qapi/error.h"
 #include "rdma_backend_defs.h"
 #include "rdma_rm_defs.h"
 
hw/rdma/rdma_utils.h +3 -3
index cdac910..3dc0789 100644
@@ -17,9 +17,9 @@
 #ifndef RDMA_UTILS_H
 #define RDMA_UTILS_H
 
-#include <qemu/osdep.h>
-#include <include/hw/pci/pci.h>
-#include <include/sysemu/dma.h>
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "sysemu/dma.h"
 
 #define pr_info(fmt, ...) \
     fprintf(stdout, "%s: %-20s (%3d): " fmt, "pvrdma",  __func__, __LINE__,\
hw/rdma/vmw/pvrdma.h +4 -4
index b05f94a..8c173cb 100644
@@ -16,14 +16,14 @@
 #ifndef PVRDMA_PVRDMA_H
 #define PVRDMA_PVRDMA_H
 
-#include <hw/pci/pci.h>
-#include <hw/pci/msix.h>
+#include "hw/pci/pci.h"
+#include "hw/pci/msix.h"
 
 #include "../rdma_backend_defs.h"
 #include "../rdma_rm_defs.h"
 
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
 #include "pvrdma_dev_ring.h"
 
 /* BARs */
hw/rdma/vmw/pvrdma_dev_ring.h +1 -1
index 02a590b..2d0461f 100644
@@ -16,7 +16,7 @@
 #ifndef PVRDMA_DEV_RING_H
 #define PVRDMA_DEV_RING_H
 
-#include <qemu/typedefs.h>
+#include "qemu/typedefs.h"
 
 #define MAX_RING_NAME_SZ 32
 
hw/rdma/rdma_backend.c +3 -3
index e306fba..f85c7b2 100644
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qemu/error-report.h>
-#include <qapi/error.h>
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
 
 #include <infiniband/verbs.h>
 
hw/rdma/rdma_rm.c +3 -3
index b5fc45d..80ffc94 100644
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qapi/error.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "cpu.h"
 
 #include "rdma_utils.h"
 #include "rdma_backend.h"
hw/rdma/vmw/pvrdma_cmd.c +4 -4
index 293dfed..a9fa1ef 100644
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qemu/error-report.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "cpu.h"
 #include <linux/types.h>
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
@@ -26,7 +26,7 @@
 #include "../rdma_utils.h"
 
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
 
 static void *pvrdma_map_to_pdir(PCIDevice *pdev, uint64_t pdir_dma,
                                 uint32_t nchunks, size_t length)
hw/rdma/vmw/pvrdma_dev_ring.c +4 -4
index ec309da..ff19a9e 100644
@@ -13,12 +13,12 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <hw/pci/pci.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "cpu.h"
 
 #include "../rdma_utils.h"
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
 #include "pvrdma_dev_ring.h"
 
 int pvrdma_ring_init(PvrdmaRing *ring, const char *name, PCIDevice *dev,
hw/rdma/vmw/pvrdma_main.c +12 -12
index 9978781..25ea02a 100644
@@ -13,16 +13,16 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qapi/error.h>
-#include <hw/hw.h>
-#include <hw/pci/pci.h>
-#include <hw/pci/pci_ids.h>
-#include <hw/pci/msi.h>
-#include <hw/pci/msix.h>
-#include <hw/qdev-core.h>
-#include <hw/qdev-properties.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/hw.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_ids.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/msix.h"
+#include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
+#include "cpu.h"
 #include "trace.h"
 
 #include "../rdma_rm.h"
@@ -31,8 +31,8 @@
 
 #include <infiniband/verbs.h>
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
 #include "pvrdma_qp_ops.h"
 
 static Property pvrdma_dev_properties[] = {
hw/rdma/vmw/pvrdma_qp_ops.c +2 -2
index f0a1f9e..a693c06 100644
@@ -13,14 +13,14 @@
  *
  */
 
-#include <qemu/osdep.h>
+#include "qemu/osdep.h"
 
 #include "../rdma_utils.h"
 #include "../rdma_rm.h"
 #include "../rdma_backend.h"
 
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
 #include "pvrdma_qp_ops.h"
 
 typedef struct CompHandlerCtx {
Signature · 1 line
MST
#1 lore raw

PATCH 2/2 make: switch from -I to -iquote

Our rule right now is to use <> for external headers, "" for internal ones. The idea was to avoid conflicts between e.g. a system file named <trace.h> and an internal one by the same name.

Unfortunately we use -I compiler flag so it does not
help: a system file doing #include <trace.h> will still pick up ours first.

To fix, switch to -iquote which is supported by both gcc and clang and only affects #include "" directives.

As a side effect, this catches any future uses of
#include <> for internal headers.

---

I still think we want to switch to a more formal rule such as qemu/ prefix for all includes down the road, but this will at least catch any scheme violations from creeping in meanwhile.

 configure       | 16 ++++++++--------
 rules.mak       |  2 +-
 Makefile.target |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)
configure +8 -8
index af72fc8..23a4f3b 100755
@@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
-QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
+QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg -iquote \$(SRC_PATH)/include"
 if test "$debug_info" = "yes"; then
     CFLAGS="-g $CFLAGS"
     LDFLAGS="-g $LDFLAGS"
@@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then
 fi
 
 if test "$tcg_interpreter" = "yes"; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
 elif test "$ARCH" = "sparc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
 elif test "$ARCH" = "s390x" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
 elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
 elif test "$ARCH" = "ppc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
 else
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
 fi
-QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
+QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES"
 
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
rules.mak +1 -1
index 6e94333..93a0702 100644
@@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 # dir, one absolute and the other relative to the compiler working
 # directory. These are the same for target-independent files, but
 # different for target-dependent ones.
-QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D)
+QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D)
 
 WL_U := -Wl,-u,
 find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
Makefile.target +2 -2
index 6549481..d0ec77a 100644
@@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
 ifdef CONFIG_LINUX
 QEMU_CFLAGS += -I../linux-headers
 endif
-QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
+QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
 
-QEMU_CFLAGS+=-I$(SRC_PATH)/include
+QEMU_CFLAGS+=-iquote $(SRC_PATH)/include
 
 ifdef CONFIG_USER_ONLY
 # user emulator name
Signature · 1 line
MST
Reviewed-by on v1 2/2
#2 lore raw

Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote

Quoted 8 lines · Michael S. Tsirkin

Our rule right now is to use <> for external headers, "" for internal ones. The idea was to avoid conflicts between e.g. a system file named <trace.h> and an internal one by the same name.

Unfortunately we use -I compiler flag so it does not
help: a system file doing #include <trace.h> will still pick up ours first.

Has that actually hit you practice ? I'm wondering if this is just theoretical in which case it could wait till 2.13 since this scenario has existed in QEMU along time, or a real problem right now requiring fix in 2.12.

Quoted 2 lines

To fix, switch to -iquote which is supported by both gcc and clang and only affects #include "" directives.

Fine since we don't support anything other than gcc and clang. I'm assuming -iquote has been supported by these two compilers for a long time though ? The GCC docs annoyingly don't ever mention what release features appear in :-(

Quoted 7 lines

As a side effect, this catches any future uses of
#include <> for internal headers.

---

Quoted 83 lines

I still think we want to switch to a more formal rule such as qemu/ prefix for all includes down the road, but this will at least catch any scheme violations from creeping in meanwhile.

 configure       | 16 ++++++++--------
 rules.mak       |  2 +-
 Makefile.target |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)
configure +8 -8
index af72fc8..23a4f3b 100755
@@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
-QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
+QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg -iquote \$(SRC_PATH)/include"
 if test "$debug_info" = "yes"; then
     CFLAGS="-g $CFLAGS"
     LDFLAGS="-g $LDFLAGS"
@@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then
 fi
 
 if test "$tcg_interpreter" = "yes"; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
 elif test "$ARCH" = "sparc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
 elif test "$ARCH" = "s390x" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
 elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
 elif test "$ARCH" = "ppc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
 else
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
 fi
-QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
+QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES"
 
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
rules.mak +1 -1
index 6e94333..93a0702 100644
@@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 # dir, one absolute and the other relative to the compiler working
 # directory. These are the same for target-independent files, but
 # different for target-dependent ones.
-QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D)
+QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D)
 
 WL_U := -Wl,-u,
 find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
Makefile.target +2 -2
index 6549481..d0ec77a 100644
@@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
 ifdef CONFIG_LINUX
 QEMU_CFLAGS += -I../linux-headers
 endif
-QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
+QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
 
-QEMU_CFLAGS+=-I$(SRC_PATH)/include
+QEMU_CFLAGS+=-iquote $(SRC_PATH)/include
 
 ifdef CONFIG_USER_ONLY
 # user emulator name
Signature · 1 line
MST

Regards,
Daniel

Signature · 3 lines
|: 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 :|
#3 lore raw

Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote

Quoted 6 lines · Daniel P. Berrangé
Quoted 5 lines · Michael S. Tsirkin

Our rule right now is to use <> for external headers, "" for internal ones. The idea was to avoid conflicts between e.g. a system file named <trace.h> and an internal one by the same name.

Quoted 8 lines
Quoted 3 lines

Unfortunately we use -I compiler flag so it does not
help: a system file doing #include <trace.h> will still pick up ours first.

Has that actually hit you practice ? I'm wondering if this is just theoretical in which case it could wait till 2.13 since this scenario has existed in QEMU along time, or a real problem right now requiring fix in 2.12.

It's borderline.

In particular there is at least one instance where we already have a conflict (yes, there are probably other ways to address this, but I consider them less robust):

util/getauxval.c:#include <sys/auxv.h>
/usr/include/sys/auxv.h:#include <elf.h>

So it really does build by luck.

Quoted 8 lines
Quoted 2 lines

To fix, switch to -iquote which is supported by both gcc and clang and only affects #include "" directives.

Fine since we don't support anything other than gcc and clang. I'm assuming -iquote has been supported by these two compilers for a long time though ? The GCC docs annoyingly don't ever mention what release features appear in :-(

Judging by git log output:

For clang it appears to be there since 2007 that's pretty close to day 1.

gcc's log is better documented, it's been there since 2004-05-03

Quoted 9 lines
Quoted 7 lines

As a side effect, this catches any future uses of
#include <> for internal headers.

---

Thanks!

Quoted 91 lines
Quoted 84 lines

I still think we want to switch to a more formal rule such as qemu/ prefix for all includes down the road, but this will at least catch any scheme violations from creeping in meanwhile.

 configure       | 16 ++++++++--------
 rules.mak       |  2 +-
 Makefile.target |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)
configure +8 -8
index af72fc8..23a4f3b 100755
@@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
-QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
+QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg -iquote \$(SRC_PATH)/include"
 if test "$debug_info" = "yes"; then
     CFLAGS="-g $CFLAGS"
     LDFLAGS="-g $LDFLAGS"
@@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then
 fi
 
 if test "$tcg_interpreter" = "yes"; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
 elif test "$ARCH" = "sparc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
 elif test "$ARCH" = "s390x" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
 elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
 elif test "$ARCH" = "ppc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
 else
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
 fi
-QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
+QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES"
 
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
rules.mak +1 -1
index 6e94333..93a0702 100644
@@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 # dir, one absolute and the other relative to the compiler working
 # directory. These are the same for target-independent files, but
 # different for target-dependent ones.
-QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D)
+QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D)
 
 WL_U := -Wl,-u,
 find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
Makefile.target +2 -2
index 6549481..d0ec77a 100644
@@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
 ifdef CONFIG_LINUX
 QEMU_CFLAGS += -I../linux-headers
 endif
-QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
+QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
 
-QEMU_CFLAGS+=-I$(SRC_PATH)/include
+QEMU_CFLAGS+=-iquote $(SRC_PATH)/include
 
 ifdef CONFIG_USER_ONLY
 # user emulator name
Signature · 1 line
MST

Regards,
Daniel

Signature · 3 lines
|: 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 :|
#4 lore raw

Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote

Quoted 48 lines · Michael S. Tsirkin
Quoted 6 lines · Daniel P. Berrangé
Quoted 5 lines · Michael S. Tsirkin

Our rule right now is to use <> for external headers, "" for internal ones. The idea was to avoid conflicts between e.g. a system file named <trace.h> and an internal one by the same name.

Quoted 8 lines
Quoted 3 lines

Unfortunately we use -I compiler flag so it does not
help: a system file doing #include <trace.h> will still pick up ours first.

Has that actually hit you practice ? I'm wondering if this is just theoretical in which case it could wait till 2.13 since this scenario has existed in QEMU along time, or a real problem right now requiring fix in 2.12.

It's borderline.

In particular there is at least one instance where we already have a conflict (yes, there are probably other ways to address this, but I consider them less robust):

util/getauxval.c:#include <sys/auxv.h>
/usr/include/sys/auxv.h:#include <elf.h>

So it really does build by luck.

Quoted 8 lines
Quoted 2 lines

To fix, switch to -iquote which is supported by both gcc and clang and only affects #include "" directives.

Fine since we don't support anything other than gcc and clang. I'm assuming -iquote has been supported by these two compilers for a long time though ? The GCC docs annoyingly don't ever mention what release features appear in :-(

Judging by git log output:

For clang it appears to be there since 2007 that's pretty close to day 1.

gcc's log is better documented, it's been there since 2004-05-03

Quoted 4 lines
Quoted 4 lines

As a side effect, this catches any future uses of
#include <> for internal headers.

Quoted 9 lines
Quoted 5 lines
Quoted 3 lines

---

Thanks!

Hi Michael,
Should I add this patch to the RDMA pull request?

Thanks,
Marcel

Quoted 91 lines
Quoted 91 lines
Quoted 84 lines

I still think we want to switch to a more formal rule such as qemu/ prefix for all includes down the road, but this will at least catch any scheme violations from creeping in meanwhile.

 configure       | 16 ++++++++--------
 rules.mak       |  2 +-
 Makefile.target |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)
configure +8 -8
index af72fc8..23a4f3b 100755
@@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
-QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
+QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg -iquote \$(SRC_PATH)/include"
 if test "$debug_info" = "yes"; then
     CFLAGS="-g $CFLAGS"
     LDFLAGS="-g $LDFLAGS"
@@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then
 fi
 
 if test "$tcg_interpreter" = "yes"; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
 elif test "$ARCH" = "sparc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
 elif test "$ARCH" = "s390x" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
 elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
 elif test "$ARCH" = "ppc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
 else
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
 fi
-QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
+QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES"
 
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
rules.mak +1 -1
index 6e94333..93a0702 100644
@@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 # dir, one absolute and the other relative to the compiler working
 # directory. These are the same for target-independent files, but
 # different for target-dependent ones.
-QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D)
+QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D)
 
 WL_U := -Wl,-u,
 find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
Makefile.target +2 -2
index 6549481..d0ec77a 100644
@@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
 ifdef CONFIG_LINUX
 QEMU_CFLAGS += -I../linux-headers
 endif
-QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
+QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
 
-QEMU_CFLAGS+=-I$(SRC_PATH)/include
+QEMU_CFLAGS+=-iquote $(SRC_PATH)/include
 
 ifdef CONFIG_USER_ONLY
 # user emulator name
Signature · 1 line
MST

Regards,
Daniel

Signature · 3 lines
|: 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 :|
#5 lore raw

Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote

Quoted 56 lines · Marcel Apfelbaum
Quoted 55 lines · Michael S. Tsirkin
Quoted 6 lines · Daniel P. Berrangé
Quoted 5 lines · Michael S. Tsirkin

Our rule right now is to use <> for external headers, "" for internal ones. The idea was to avoid conflicts between e.g. a system file named <trace.h> and an internal one by the same name.

Quoted 8 lines
Quoted 3 lines

Unfortunately we use -I compiler flag so it does not
help: a system file doing #include <trace.h> will still pick up ours first.

Has that actually hit you practice ? I'm wondering if this is just theoretical in which case it could wait till 2.13 since this scenario has existed in QEMU along time, or a real problem right now requiring fix in 2.12.

It's borderline.

In particular there is at least one instance where we already have a conflict (yes, there are probably other ways to address this, but I consider them less robust):

util/getauxval.c:#include <sys/auxv.h>
/usr/include/sys/auxv.h:#include <elf.h>

So it really does build by luck.

Quoted 8 lines
Quoted 2 lines

To fix, switch to -iquote which is supported by both gcc and clang and only affects #include "" directives.

Fine since we don't support anything other than gcc and clang. I'm assuming -iquote has been supported by these two compilers for a long time though ? The GCC docs annoyingly don't ever mention what release features appear in :-(

Judging by git log output:

For clang it appears to be there since 2007 that's pretty close to day 1.

gcc's log is better documented, it's been there since 2004-05-03

Quoted 9 lines
Quoted 7 lines

As a side effect, this catches any future uses of
#include <> for internal headers.

---

Quoted 8 lines
Quoted 2 lines

Thanks!

Hi Michael,
Should I add this patch to the RDMA pull request?

Thanks,
Marcel

Go ahead and add both of them pls.

Quoted 92 lines
Quoted 92 lines
Quoted 92 lines
Quoted 85 lines

I still think we want to switch to a more formal rule such as qemu/ prefix for all includes down the road, but this will at least catch any scheme violations from creeping in meanwhile.

 configure       | 16 ++++++++--------
 rules.mak       |  2 +-
 Makefile.target |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)
configure +8 -8
index af72fc8..23a4f3b 100755
@@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
-QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
+QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg -iquote \$(SRC_PATH)/include"
 if test "$debug_info" = "yes"; then
     CFLAGS="-g $CFLAGS"
     LDFLAGS="-g $LDFLAGS"
@@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then
 fi
 
 if test "$tcg_interpreter" = "yes"; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
 elif test "$ARCH" = "sparc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
 elif test "$ARCH" = "s390x" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
 elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
 elif test "$ARCH" = "ppc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
 else
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
 fi
-QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
+QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES"
 
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
rules.mak +1 -1
index 6e94333..93a0702 100644
@@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 # dir, one absolute and the other relative to the compiler working
 # directory. These are the same for target-independent files, but
 # different for target-dependent ones.
-QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D)
+QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D)
 
 WL_U := -Wl,-u,
 find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
Makefile.target +2 -2
index 6549481..d0ec77a 100644
@@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
 ifdef CONFIG_LINUX
 QEMU_CFLAGS += -I../linux-headers
 endif
-QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
+QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
 
-QEMU_CFLAGS+=-I$(SRC_PATH)/include
+QEMU_CFLAGS+=-iquote $(SRC_PATH)/include
 
 ifdef CONFIG_USER_ONLY
 # user emulator name
Signature · 1 line
MST

Regards,
Daniel

Signature · 3 lines
|: 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 :|
Reviewed-by on v1 2/2
#6 lore raw

Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote

In reply to Michael S. Tsirkin
Quoted 24 lines · Michael S. Tsirkin

Our rule right now is to use <> for external headers, "" for internal ones. The idea was to avoid conflicts between e.g. a system file named <trace.h> and an internal one by the same name.

Unfortunately we use -I compiler flag so it does not
help: a system file doing #include <trace.h> will still pick up ours first.

To fix, switch to -iquote which is supported by both gcc and clang and only affects #include "" directives.

As a side effect, this catches any future uses of
#include <> for internal headers.

---

I still think we want to switch to a more formal rule such as qemu/ prefix for all includes down the road, but this will at least catch any scheme violations from creeping in meanwhile.

Quoted 4 lines
 configure       | 16 ++++++++--------
 rules.mak       |  2 +-
 Makefile.target |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)
Reviewed-by on v1 1/2
#7 lore raw
In reply to Michael S. Tsirkin
Quoted 4 lines · Michael S. Tsirkin

Our rule right now is to use <> for external headers only. RDMA code violates that, fix it up.

Quoted 14 lines
---
 hw/rdma/rdma_backend.h        |  2 +-
 hw/rdma/rdma_backend_defs.h   |  2 +-
 hw/rdma/rdma_rm.h             |  2 +-
 hw/rdma/rdma_utils.h          |  6 +++---
 hw/rdma/vmw/pvrdma.h          |  8 ++++----
 hw/rdma/vmw/pvrdma_dev_ring.h |  2 +-
 hw/rdma/rdma_backend.c        |  6 +++---
 hw/rdma/rdma_rm.c             |  6 +++---
 hw/rdma/vmw/pvrdma_cmd.c      |  8 ++++----
 hw/rdma/vmw/pvrdma_dev_ring.c |  8 ++++----
 hw/rdma/vmw/pvrdma_main.c     | 24 ++++++++++++------------
 hw/rdma/vmw/pvrdma_qp_ops.c   |  4 ++--
 12 files changed, 39 insertions(+), 39 deletions(-)

I'm assuming these were detected by build failures caused by the 2nd patch in this series, so other areas of the codebase are all correct already.

Regards,
Daniel

Signature · 3 lines
|: 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 :|
Reviewed-by on v1 1/2
#8 lore raw
In reply to Michael S. Tsirkin
Quoted 4 lines · Michael S. Tsirkin

Our rule right now is to use <> for external headers only. RDMA code violates that, fix it up.

Thanks,
Marcel

Quoted 252 lines
---
 hw/rdma/rdma_backend.h        |  2 +-
 hw/rdma/rdma_backend_defs.h   |  2 +-
 hw/rdma/rdma_rm.h             |  2 +-
 hw/rdma/rdma_utils.h          |  6 +++---
 hw/rdma/vmw/pvrdma.h          |  8 ++++----
 hw/rdma/vmw/pvrdma_dev_ring.h |  2 +-
 hw/rdma/rdma_backend.c        |  6 +++---
 hw/rdma/rdma_rm.c             |  6 +++---
 hw/rdma/vmw/pvrdma_cmd.c      |  8 ++++----
 hw/rdma/vmw/pvrdma_dev_ring.c |  8 ++++----
 hw/rdma/vmw/pvrdma_main.c     | 24 ++++++++++++------------
 hw/rdma/vmw/pvrdma_qp_ops.c   |  4 ++--
 12 files changed, 39 insertions(+), 39 deletions(-)
hw/rdma/rdma_backend.h +1 -1
index 68f2b05..abae262 100644
@@ -16,7 +16,7 @@
 #ifndef RDMA_BACKEND_H
 #define RDMA_BACKEND_H
 
-#include <qapi/error.h>
+#include "qapi/error.h"
 #include "rdma_rm_defs.h"
 #include "rdma_backend_defs.h"
 
hw/rdma/rdma_backend_defs.h +1 -1
index 837e324..ff5cfc2 100644
@@ -17,7 +17,7 @@
 #define RDMA_BACKEND_DEFS_H
 
 #include <infiniband/verbs.h>
-#include <qemu/thread.h>
+#include "qemu/thread.h"
 
 typedef struct RdmaDeviceResources RdmaDeviceResources;
 
hw/rdma/rdma_rm.h +1 -1
index be95c1b..82c1ddd 100644
@@ -16,7 +16,7 @@
 #ifndef RDMA_RM_H
 #define RDMA_RM_H
 
-#include <qapi/error.h>
+#include "qapi/error.h"
 #include "rdma_backend_defs.h"
 #include "rdma_rm_defs.h"
 
hw/rdma/rdma_utils.h +3 -3
index cdac910..3dc0789 100644
@@ -17,9 +17,9 @@
 #ifndef RDMA_UTILS_H
 #define RDMA_UTILS_H
 
-#include <qemu/osdep.h>
-#include <include/hw/pci/pci.h>
-#include <include/sysemu/dma.h>
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "sysemu/dma.h"
 
 #define pr_info(fmt, ...) \
     fprintf(stdout, "%s: %-20s (%3d): " fmt, "pvrdma",  __func__, __LINE__,\
hw/rdma/vmw/pvrdma.h +4 -4
index b05f94a..8c173cb 100644
@@ -16,14 +16,14 @@
 #ifndef PVRDMA_PVRDMA_H
 #define PVRDMA_PVRDMA_H
 
-#include <hw/pci/pci.h>
-#include <hw/pci/msix.h>
+#include "hw/pci/pci.h"
+#include "hw/pci/msix.h"
 
 #include "../rdma_backend_defs.h"
 #include "../rdma_rm_defs.h"
 
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
 #include "pvrdma_dev_ring.h"
 
 /* BARs */
hw/rdma/vmw/pvrdma_dev_ring.h +1 -1
index 02a590b..2d0461f 100644
@@ -16,7 +16,7 @@
 #ifndef PVRDMA_DEV_RING_H
 #define PVRDMA_DEV_RING_H
 
-#include <qemu/typedefs.h>
+#include "qemu/typedefs.h"
 
 #define MAX_RING_NAME_SZ 32
 
hw/rdma/rdma_backend.c +3 -3
index e306fba..f85c7b2 100644
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qemu/error-report.h>
-#include <qapi/error.h>
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
 
 #include <infiniband/verbs.h>
 
hw/rdma/rdma_rm.c +3 -3
index b5fc45d..80ffc94 100644
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qapi/error.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "cpu.h"
 
 #include "rdma_utils.h"
 #include "rdma_backend.h"
hw/rdma/vmw/pvrdma_cmd.c +4 -4
index 293dfed..a9fa1ef 100644
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qemu/error-report.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "cpu.h"
 #include <linux/types.h>
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
@@ -26,7 +26,7 @@
 #include "../rdma_utils.h"
 
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
 
 static void *pvrdma_map_to_pdir(PCIDevice *pdev, uint64_t pdir_dma,
                                 uint32_t nchunks, size_t length)
hw/rdma/vmw/pvrdma_dev_ring.c +4 -4
index ec309da..ff19a9e 100644
@@ -13,12 +13,12 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <hw/pci/pci.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "cpu.h"
 
 #include "../rdma_utils.h"
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
 #include "pvrdma_dev_ring.h"
 
 int pvrdma_ring_init(PvrdmaRing *ring, const char *name, PCIDevice *dev,
hw/rdma/vmw/pvrdma_main.c +12 -12
index 9978781..25ea02a 100644
@@ -13,16 +13,16 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qapi/error.h>
-#include <hw/hw.h>
-#include <hw/pci/pci.h>
-#include <hw/pci/pci_ids.h>
-#include <hw/pci/msi.h>
-#include <hw/pci/msix.h>
-#include <hw/qdev-core.h>
-#include <hw/qdev-properties.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/hw.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_ids.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/msix.h"
+#include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
+#include "cpu.h"
 #include "trace.h"
 
 #include "../rdma_rm.h"
@@ -31,8 +31,8 @@
 
 #include <infiniband/verbs.h>
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
 #include "pvrdma_qp_ops.h"
 
 static Property pvrdma_dev_properties[] = {
hw/rdma/vmw/pvrdma_qp_ops.c +2 -2
index f0a1f9e..a693c06 100644
@@ -13,14 +13,14 @@
  *
  */
 
-#include <qemu/osdep.h>
+#include "qemu/osdep.h"
 
 #include "../rdma_utils.h"
 #include "../rdma_rm.h"
 #include "../rdma_backend.h"
 
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
 #include "pvrdma_qp_ops.h"
 
 typedef struct CompHandlerCtx {
Reviewed-by on v1 1/2
#9 lore raw
In reply to Michael S. Tsirkin
Quoted 4 lines · Michael S. Tsirkin

Our rule right now is to use <> for external headers only. RDMA code violates that, fix it up.

Thanks.

Quoted 254 lines
---
 hw/rdma/rdma_backend.h        |  2 +-
 hw/rdma/rdma_backend_defs.h   |  2 +-
 hw/rdma/rdma_rm.h             |  2 +-
 hw/rdma/rdma_utils.h          |  6 +++---
 hw/rdma/vmw/pvrdma.h          |  8 ++++----
 hw/rdma/vmw/pvrdma_dev_ring.h |  2 +-
 hw/rdma/rdma_backend.c        |  6 +++---
 hw/rdma/rdma_rm.c             |  6 +++---
 hw/rdma/vmw/pvrdma_cmd.c      |  8 ++++----
 hw/rdma/vmw/pvrdma_dev_ring.c |  8 ++++----
 hw/rdma/vmw/pvrdma_main.c     | 24 ++++++++++++------------
 hw/rdma/vmw/pvrdma_qp_ops.c   |  4 ++--
 12 files changed, 39 insertions(+), 39 deletions(-)
hw/rdma/rdma_backend.h +1 -1
index 68f2b05..abae262 100644
@@ -16,7 +16,7 @@
 #ifndef RDMA_BACKEND_H
 #define RDMA_BACKEND_H
 
-#include <qapi/error.h>
+#include "qapi/error.h"
 #include "rdma_rm_defs.h"
 #include "rdma_backend_defs.h"
 
hw/rdma/rdma_backend_defs.h +1 -1
index 837e324..ff5cfc2 100644
@@ -17,7 +17,7 @@
 #define RDMA_BACKEND_DEFS_H
 
 #include <infiniband/verbs.h>
-#include <qemu/thread.h>
+#include "qemu/thread.h"
 
 typedef struct RdmaDeviceResources RdmaDeviceResources;
 
hw/rdma/rdma_rm.h +1 -1
index be95c1b..82c1ddd 100644
@@ -16,7 +16,7 @@
 #ifndef RDMA_RM_H
 #define RDMA_RM_H
 
-#include <qapi/error.h>
+#include "qapi/error.h"
 #include "rdma_backend_defs.h"
 #include "rdma_rm_defs.h"
 
hw/rdma/rdma_utils.h +3 -3
index cdac910..3dc0789 100644
@@ -17,9 +17,9 @@
 #ifndef RDMA_UTILS_H
 #define RDMA_UTILS_H
 
-#include <qemu/osdep.h>
-#include <include/hw/pci/pci.h>
-#include <include/sysemu/dma.h>
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "sysemu/dma.h"
 
 #define pr_info(fmt, ...) \
     fprintf(stdout, "%s: %-20s (%3d): " fmt, "pvrdma",  __func__, __LINE__,\
hw/rdma/vmw/pvrdma.h +4 -4
index b05f94a..8c173cb 100644
@@ -16,14 +16,14 @@
 #ifndef PVRDMA_PVRDMA_H
 #define PVRDMA_PVRDMA_H
 
-#include <hw/pci/pci.h>
-#include <hw/pci/msix.h>
+#include "hw/pci/pci.h"
+#include "hw/pci/msix.h"
 
 #include "../rdma_backend_defs.h"
 #include "../rdma_rm_defs.h"
 
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
 #include "pvrdma_dev_ring.h"
 
 /* BARs */
hw/rdma/vmw/pvrdma_dev_ring.h +1 -1
index 02a590b..2d0461f 100644
@@ -16,7 +16,7 @@
 #ifndef PVRDMA_DEV_RING_H
 #define PVRDMA_DEV_RING_H
 
-#include <qemu/typedefs.h>
+#include "qemu/typedefs.h"
 
 #define MAX_RING_NAME_SZ 32
 
hw/rdma/rdma_backend.c +3 -3
index e306fba..f85c7b2 100644
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qemu/error-report.h>
-#include <qapi/error.h>
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
 
 #include <infiniband/verbs.h>
 
hw/rdma/rdma_rm.c +3 -3
index b5fc45d..80ffc94 100644
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qapi/error.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "cpu.h"
 
 #include "rdma_utils.h"
 #include "rdma_backend.h"
hw/rdma/vmw/pvrdma_cmd.c +4 -4
index 293dfed..a9fa1ef 100644
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qemu/error-report.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "cpu.h"
 #include <linux/types.h>
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
@@ -26,7 +26,7 @@
 #include "../rdma_utils.h"
 
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
 
 static void *pvrdma_map_to_pdir(PCIDevice *pdev, uint64_t pdir_dma,
                                 uint32_t nchunks, size_t length)
hw/rdma/vmw/pvrdma_dev_ring.c +4 -4
index ec309da..ff19a9e 100644
@@ -13,12 +13,12 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <hw/pci/pci.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "cpu.h"
 
 #include "../rdma_utils.h"
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
 #include "pvrdma_dev_ring.h"
 
 int pvrdma_ring_init(PvrdmaRing *ring, const char *name, PCIDevice *dev,
hw/rdma/vmw/pvrdma_main.c +12 -12
index 9978781..25ea02a 100644
@@ -13,16 +13,16 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qapi/error.h>
-#include <hw/hw.h>
-#include <hw/pci/pci.h>
-#include <hw/pci/pci_ids.h>
-#include <hw/pci/msi.h>
-#include <hw/pci/msix.h>
-#include <hw/qdev-core.h>
-#include <hw/qdev-properties.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/hw.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_ids.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/msix.h"
+#include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
+#include "cpu.h"
 #include "trace.h"
 
 #include "../rdma_rm.h"
@@ -31,8 +31,8 @@
 
 #include <infiniband/verbs.h>
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
 #include "pvrdma_qp_ops.h"
 
 static Property pvrdma_dev_properties[] = {
hw/rdma/vmw/pvrdma_qp_ops.c +2 -2
index f0a1f9e..a693c06 100644
@@ -13,14 +13,14 @@
  *
  */
 
-#include <qemu/osdep.h>
+#include "qemu/osdep.h"
 
 #include "../rdma_utils.h"
 #include "../rdma_rm.h"
 #include "../rdma_backend.h"
 
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
 #include "pvrdma_qp_ops.h"
 
 typedef struct CompHandlerCtx {
Signature · 1 line
MST