[Qemu-devel] [PATCH 00/12] qemu-common.h/osdep.h refactoring

[Qemu-devel] [PATCH 00/12] qemu-common.h/osdep.h refactoring

[Peter Maydell]

This series makes a start at cleaning up some of our headers
to avoid the common problem of header files including qemu-common.h
(which then in turn can lead to awkward circular includes).

One common cause of this is that we don't have any header
which will include the basic things most header files require,
except for qemu-common.h. This series fixes that by making
'osdep.h' that "common basic stuff" header. The idea is that:
 * osdep.h can be included from anywhere, since it doesn't
   include any other QEMU headers itself except a few very
   restricted special purpose ones (config-host.h, compiler.h, etc)
 * osdep.h provides:
   + things everybody needs, like NULL, int32_t, container_of,
     the CONFIG_* defines, etc
   + things that will cause subtle problems if they're not
     present everywhere (eg directly using system headers and
     not getting the portability fixups will result in something
     that builds on most but not all hosts)

So most places can just include osdep.h, not the full qemu-common.h.

(I initially thought about defining a new header for this
purpose, "qemu/basics.h" or some such, but in fact osdep.h
was already very close to what I wanted so it didn't really
seem like it was very useful to switch everything over.)

The series has some minor cleanups, some shuffling around of
things between qemu-common.h, osdep.h and compiler.h, and
a couple of patches at the end that use osdep.h to allow
dropping a qemu-common.h include from various header files,
as a demonstration.

The real question here I guess is whether people like the
direction I'm trying to go with this. If so, we can further
reduce the number of qemu-common.h includes without too much
difficulty with further patches.

A cleanup deferred for the future is that osdep.h (in addition
to the stuff listed above) also has a pile of prototypes for
various functions that could reasonably be split out to their
own header files, since they're not critically important to
provide everywhere.

host-utils.h is rather misnamed these days, incidentally, but
a header rename doesn't seem really worth the effort (and I
couldn't think of a better name anyway).


Compile-tested on Linux, OSX, Windows and FreeBSD.

Peter Maydell (12):
  qapi/qmp-event.c: Don't manually include os-win32.h/os-posix.h
  osdep.h: Remove qemu_printf
  osdep.h: Move some compiler-specific things to compiler.h
  compiler.h: Use glue() in QEMU_BUILD_BUG_ON define
  qemu-common.h: Move Win32 fixups into os-win32.h
  osdep.h: Move some OS header includes and fixups from qemu-common.h
  osdep.h: Add header comment
  qemu-common.h: Move muldiv64() to host-utils.h
  apic_internal.h: Include cpu.h directly
  timer.h: Don't include qemu-common.h
  event_notifier.h: Don't include qemu-common.h
  throttle.h: Don't include qemu-common.h

 include/hw/i386/apic_internal.h |   1 +
 include/qemu-common.h           | 103 +------------------------------------
 include/qemu/compiler.h         |  51 ++++++++++++++++--
 include/qemu/event_notifier.h   |   7 +--
 include/qemu/host-utils.h       |  29 +++++++++++
 include/qemu/osdep.h            | 111 ++++++++++++++++++++++++----------------
 include/qemu/throttle.h         |   4 +-
 include/qemu/timer.h            |   4 +-
 include/sysemu/os-win32.h       |  18 +++++++
 monitor.c                       |   4 +-
 qapi/qmp-event.c                |   8 ---
 user-exec.c                     |   4 +-
 12 files changed, 175 insertions(+), 169 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 01/12] qapi/qmp-event.c: Don't manually include os-win32.h/os-posix.h

[Peter Maydell]

qmp-event.c already includes qemu-common.h, so manually including
os-win32.h/os-posix.h is unnecessary (and potentially fragile,
since it's duplicating the #ifdef logic that chooses which of the
two we need). Remove the unnecessary include logic.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 qapi/qmp-event.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
index 0d1ce0b..c0e435f 100644
--- a/qapi/qmp-event.c
+++ b/qapi/qmp-event.c
@@ -18,14 +18,6 @@
 #include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qjson.h"
 
-#ifdef _WIN32
-#include "sysemu/os-win32.h"
-#endif
-
-#ifdef CONFIG_POSIX
-#include "sysemu/os-posix.h"
-#endif
-
 static QMPEventFuncEmit qmp_emit;
 
 void qmp_event_set_func_emit(QMPEventFuncEmit emit)
-- 
1.9.1

[Qemu-devel] [PATCH 02/12] osdep.h: Remove qemu_printf

[Peter Maydell]

qemu_printf is an ancient remnant which has been a simple #define to
printf for over a decade, and is used in only a few places. Expand
it out in those places and remove the #define.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/osdep.h | 2 --
 monitor.c            | 4 ++--
 user-exec.c          | 4 ++--
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 3247364..e9b8b92 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -99,8 +99,6 @@ typedef signed int              int_fast16_t;
 #define inline always_inline
 #endif
 
-#define qemu_printf printf
-
 int qemu_daemon(int nochdir, int noclose);
 void *qemu_try_memalign(size_t alignment, size_t size);
 void *qemu_memalign(size_t alignment, size_t size);
diff --git a/monitor.c b/monitor.c
index aeea2b5..fc32f12 100644
--- a/monitor.c
+++ b/monitor.c
@@ -678,7 +678,7 @@ static int get_str(char *buf, int buf_size, const char **pp)
                 case '\"':
                     break;
                 default:
-                    qemu_printf("unsupported escape code: '\\%c'\n", c);
+                    printf("unsupported escape code: '\\%c'\n", c);
                     goto fail;
                 }
                 if ((q - buf) < buf_size - 1) {
@@ -692,7 +692,7 @@ static int get_str(char *buf, int buf_size, const char **pp)
             }
         }
         if (*p != '\"') {
-            qemu_printf("unterminated string\n");
+            printf("unterminated string\n");
             goto fail;
         }
         p++;
diff --git a/user-exec.c b/user-exec.c
index ed9a07f..8ad89a4 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -92,8 +92,8 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
     int ret;
 
 #if defined(DEBUG_SIGNAL)
-    qemu_printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n",
-                pc, address, is_write, *(unsigned long *)old_set);
+    printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n",
+           pc, address, is_write, *(unsigned long *)old_set);
 #endif
     /* XXX: locking issue */
     if (is_write && h2g_valid(address)
-- 
1.9.1

[Qemu-devel] [PATCH 03/12] osdep.h: Move some compiler-specific things to compiler.h

[Peter Maydell]

osdep.h has a few things which are really compiler specific;
move them to compiler.h, and include compiler.h from osdep.h.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/compiler.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/osdep.h    | 48 +-----------------------------------------------
 2 files changed, 48 insertions(+), 47 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index df9dd51..bdc9a60 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -42,6 +42,53 @@
 # define QEMU_PACKED __attribute__((packed))
 #endif
 
+#ifndef glue
+#define xglue(x, y) x ## y
+#define glue(x, y) xglue(x, y)
+#define stringify(s)	tostring(s)
+#define tostring(s)	#s
+#endif
+
+#ifndef likely
+#if __GNUC__ < 3
+#define __builtin_expect(x, n) (x)
+#endif
+
+#define likely(x)   __builtin_expect(!!(x), 1)
+#define unlikely(x)   __builtin_expect(!!(x), 0)
+#endif
+
+#ifndef container_of
+#define container_of(ptr, type, member) ({                      \
+        const typeof(((type *) 0)->member) *__mptr = (ptr);     \
+        (type *) ((char *) __mptr - offsetof(type, member));})
+#endif
+
+/* Convert from a base type to a parent type, with compile time checking.  */
+#ifdef __GNUC__
+#define DO_UPCAST(type, field, dev) ( __extension__ ( { \
+    char __attribute__((unused)) offset_must_be_zero[ \
+        -offsetof(type, field)]; \
+    container_of(dev, type, field);}))
+#else
+#define DO_UPCAST(type, field, dev) container_of(dev, type, field)
+#endif
+
+#define typeof_field(type, field) typeof(((type *)0)->field)
+#define type_check(t1,t2) ((t1*)0 - (t2*)0)
+
+#ifndef always_inline
+#if !((__GNUC__ < 3) || defined(__APPLE__))
+#ifdef __OPTIMIZE__
+#undef inline
+#define inline __attribute__ (( always_inline )) __inline__
+#endif
+#endif
+#else
+#undef inline
+#define inline always_inline
+#endif
+
 #define cat(x,y) x ## y
 #define cat2(x,y) cat(x,y)
 #define QEMU_BUILD_BUG_ON(x) \
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index e9b8b92..4ae2c64 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -2,6 +2,7 @@
 #define QEMU_OSDEP_H
 
 #include "config-host.h"
+#include "qemu/compiler.h"
 #include <stdarg.h>
 #include <stddef.h>
 #include <stdbool.h>
@@ -27,41 +28,6 @@ typedef unsigned int            uint_fast16_t;
 typedef signed int              int_fast16_t;
 #endif
 
-#ifndef glue
-#define xglue(x, y) x ## y
-#define glue(x, y) xglue(x, y)
-#define stringify(s)	tostring(s)
-#define tostring(s)	#s
-#endif
-
-#ifndef likely
-#if __GNUC__ < 3
-#define __builtin_expect(x, n) (x)
-#endif
-
-#define likely(x)   __builtin_expect(!!(x), 1)
-#define unlikely(x)   __builtin_expect(!!(x), 0)
-#endif
-
-#ifndef container_of
-#define container_of(ptr, type, member) ({                      \
-        const typeof(((type *) 0)->member) *__mptr = (ptr);     \
-        (type *) ((char *) __mptr - offsetof(type, member));})
-#endif
-
-/* Convert from a base type to a parent type, with compile time checking.  */
-#ifdef __GNUC__
-#define DO_UPCAST(type, field, dev) ( __extension__ ( { \
-    char __attribute__((unused)) offset_must_be_zero[ \
-        -offsetof(type, field)]; \
-    container_of(dev, type, field);}))
-#else
-#define DO_UPCAST(type, field, dev) container_of(dev, type, field)
-#endif
-
-#define typeof_field(type, field) typeof(((type *)0)->field)
-#define type_check(t1,t2) ((t1*)0 - (t2*)0)
-
 #ifndef MIN
 #define MIN(a, b) (((a) < (b)) ? (a) : (b))
 #endif
@@ -87,18 +53,6 @@ typedef signed int              int_fast16_t;
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 #endif
 
-#ifndef always_inline
-#if !((__GNUC__ < 3) || defined(__APPLE__))
-#ifdef __OPTIMIZE__
-#undef inline
-#define inline __attribute__ (( always_inline )) __inline__
-#endif
-#endif
-#else
-#undef inline
-#define inline always_inline
-#endif
-
 int qemu_daemon(int nochdir, int noclose);
 void *qemu_try_memalign(size_t alignment, size_t size);
 void *qemu_memalign(size_t alignment, size_t size);
-- 
1.9.1

[Qemu-devel] [PATCH 04/12] compiler.h: Use glue() in QEMU_BUILD_BUG_ON define

[Peter Maydell]

Rather than rolling custom concatenate-strings macros for the
QEMU_BUILD_BUG_ON macro to use, use the glue() macro we already
have (since it's now available to us in this header).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/compiler.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index bdc9a60..d22eb01 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -89,10 +89,8 @@
 #define inline always_inline
 #endif
 
-#define cat(x,y) x ## y
-#define cat2(x,y) cat(x,y)
 #define QEMU_BUILD_BUG_ON(x) \
-    typedef char cat2(qemu_build_bug_on__,__LINE__)[(x)?-1:1] __attribute__((unused));
+    typedef char glue(qemu_build_bug_on__,__LINE__)[(x)?-1:1] __attribute__((unused));
 
 #if defined __GNUC__
 # if !QEMU_GNUC_PREREQ(4, 4)
-- 
1.9.1

[Qemu-devel] [PATCH 05/12] qemu-common.h: Move Win32 fixups into os-win32.h

[Peter Maydell]

qemu-common.h includes some fixups for things the Win32
headers don't define or define weirdly. These really
belong in os-win32.h, so move them there.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu-common.h     | 17 -----------------
 include/sysemu/os-win32.h | 18 ++++++++++++++++++
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 237d654..9e622a5 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -86,23 +86,6 @@
 # error Unknown pointer size
 #endif
 
-#ifdef _WIN32
-#define fsync _commit
-#if !defined(lseek)
-# define lseek _lseeki64
-#endif
-int qemu_ftruncate64(int, int64_t);
-#if !defined(ftruncate)
-# define ftruncate qemu_ftruncate64
-#endif
-
-static inline char *realpath(const char *path, char *resolved_path)
-{
-    _fullpath(resolved_path, path, _MAX_PATH);
-    return resolved_path;
-}
-#endif
-
 void cpu_ticks_init(void);
 
 /* icount */
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 4035c4f..706d85a 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -109,4 +109,22 @@ static inline int os_mlock(void)
     return -ENOSYS;
 }
 
+#define fsync _commit
+
+#if !defined(lseek)
+# define lseek _lseeki64
+#endif
+
+int qemu_ftruncate64(int, int64_t);
+
+#if !defined(ftruncate)
+# define ftruncate qemu_ftruncate64
+#endif
+
+static inline char *realpath(const char *path, char *resolved_path)
+{
+    _fullpath(resolved_path, path, _MAX_PATH);
+    return resolved_path;
+}
+
 #endif
-- 
1.9.1

[Qemu-devel] [PATCH 06/12] osdep.h: Move some OS header includes and fixups from qemu-common.h

[Peter Maydell]

qemu-common.h has some system header includes and fixups for
things that might be missing. This is really an OS dependency
and belongs in osdep.h, so move it across.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu-common.h | 55 +--------------------------------------------------
 include/qemu/osdep.h  | 49 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 55 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 9e622a5..de4ae53 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -12,8 +12,7 @@
 #ifndef QEMU_COMMON_H
 #define QEMU_COMMON_H
 
-#include "qemu/compiler.h"
-#include "config-host.h"
+#include "qemu/osdep.h"
 #include "qemu/typedefs.h"
 #include "qemu/fprintf-fn.h"
 
@@ -23,60 +22,9 @@
 
 #define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
 
-/* we put basic includes here to avoid repeating them in device drivers */
-#include <stdlib.h>
-#include <stdio.h>
-#include <stdarg.h>
-#include <stdbool.h>
-#include <string.h>
-#include <strings.h>
-#include <inttypes.h>
-#include <limits.h>
-#include <time.h>
-#include <ctype.h>
-#include <errno.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <sys/stat.h>
-#include <sys/time.h>
-#include <assert.h>
-#include <signal.h>
 #include "glib-compat.h"
 #include "qemu/option.h"
 
-#ifdef _WIN32
-#include "sysemu/os-win32.h"
-#endif
-
-#ifdef CONFIG_POSIX
-#include "sysemu/os-posix.h"
-#endif
-
-#ifndef O_LARGEFILE
-#define O_LARGEFILE 0
-#endif
-#ifndef O_BINARY
-#define O_BINARY 0
-#endif
-#ifndef MAP_ANONYMOUS
-#define MAP_ANONYMOUS MAP_ANON
-#endif
-#ifndef ENOMEDIUM
-#define ENOMEDIUM ENODEV
-#endif
-#if !defined(ENOTSUP)
-#define ENOTSUP 4096
-#endif
-#if !defined(ECANCELED)
-#define ECANCELED 4097
-#endif
-#if !defined(EMEDIUMTYPE)
-#define EMEDIUMTYPE 4098
-#endif
-#ifndef TIME_MAX
-#define TIME_MAX LONG_MAX
-#endif
-
 /* HOST_LONG_BITS is the size of a native pointer in bits. */
 #if UINTPTR_MAX == UINT32_MAX
 # define HOST_LONG_BITS 32
@@ -97,7 +45,6 @@ extern int64_t max_delay;
 extern int64_t max_advance;
 void dump_drift_info(FILE *f, fprintf_function cpu_fprintf);
 
-#include "qemu/osdep.h"
 #include "qemu/bswap.h"
 
 /* FIXME: Remove NEED_CPU_H.  */
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 4ae2c64..60ac27d 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -8,6 +8,22 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <sys/types.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <strings.h>
+#include <inttypes.h>
+#include <limits.h>
+#include <time.h>
+#include <ctype.h>
+#include <errno.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/time.h>
+#include <assert.h>
+#include <signal.h>
+
 #ifdef __OpenBSD__
 #include <sys/signal.h>
 #endif
@@ -19,7 +35,13 @@
 #define WEXITSTATUS(x) (x)
 #endif
 
-#include <sys/time.h>
+#ifdef _WIN32
+#include "sysemu/os-win32.h"
+#endif
+
+#ifdef CONFIG_POSIX
+#include "sysemu/os-posix.h"
+#endif
 
 #if defined(CONFIG_SOLARIS) && CONFIG_SOLARIS_VERSION < 10
 /* [u]int_fast*_t not in <sys/int_types.h> */
@@ -28,6 +50,31 @@ typedef unsigned int            uint_fast16_t;
 typedef signed int              int_fast16_t;
 #endif
 
+#ifndef O_LARGEFILE
+#define O_LARGEFILE 0
+#endif
+#ifndef O_BINARY
+#define O_BINARY 0
+#endif
+#ifndef MAP_ANONYMOUS
+#define MAP_ANONYMOUS MAP_ANON
+#endif
+#ifndef ENOMEDIUM
+#define ENOMEDIUM ENODEV
+#endif
+#if !defined(ENOTSUP)
+#define ENOTSUP 4096
+#endif
+#if !defined(ECANCELED)
+#define ECANCELED 4097
+#endif
+#if !defined(EMEDIUMTYPE)
+#define EMEDIUMTYPE 4098
+#endif
+#ifndef TIME_MAX
+#define TIME_MAX LONG_MAX
+#endif
+
 #ifndef MIN
 #define MIN(a, b) (((a) < (b)) ? (a) : (b))
 #endif
-- 
1.9.1

[Qemu-devel] [PATCH 07/12] osdep.h: Add header comment

[Peter Maydell]

Add a header comment to osdep.h, explaining what the header is for
and some rules to avoid circular-include difficulties.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/osdep.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 60ac27d..ab3c876 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -1,3 +1,27 @@
+/*
+ * OS includes and handling of OS dependencies
+ *
+ * This header exists to pull in some common system headers that
+ * most code in QEMU will want, and to fix up some possible issues with
+ * it (missing defines, Windows weirdness, and so on).
+ *
+ * To avoid getting into possible circular include dependencies, this
+ * file should not include any other QEMU headers, with the exceptions
+ * of config-host.h, compiler.h, os-posix.h and os-win32.h, all of which
+ * are doing a similar job to this file and are under similar constraints.
+ *
+ * This header also contains prototypes for functions defined in
+ * os-*.c and util/oslib-*.c; those would probably be better split
+ * out into separate header files.
+ *
+ * In an ideal world this header would contain only:
+ *  (1) things which everybody needs
+ *  (2) things without which code would work on most platforms but
+ *      fail to compile or misbehave on a minority of host OSes
+ *
+ * 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
 #define QEMU_OSDEP_H
 
-- 
1.9.1

[Qemu-devel] [PATCH 08/12] qemu-common.h: Move muldiv64() to host-utils.h

[Peter Maydell]

Move the muldiv64() function from qemu-common.h to host-utils.h.
This puts it together with all the other arithmetic functions
where we provide a version with __int128_t and a fallback
without, and allows headers which need muldiv64() to avoid
including qemu-common.h.

We don't include host-utils from qemu-common.h, to avoid dragging
more things into qemu-common.h than it already has; in practice
everywhere that needs muldiv64() can get it via qemu/timer.h.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu-common.h     | 31 -------------------------------
 include/qemu/host-utils.h | 29 +++++++++++++++++++++++++++++
 include/qemu/timer.h      |  1 +
 3 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index de4ae53..8d8b9d5 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -299,37 +299,6 @@ static inline uint8_t from_bcd(uint8_t val)
     return ((val >> 4) * 10) + (val & 0x0f);
 }
 
-/* compute with 96 bit intermediate result: (a*b)/c */
-#ifdef CONFIG_INT128
-static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
-{
-    return (__int128_t)a * b / c;
-}
-#else
-static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
-{
-    union {
-        uint64_t ll;
-        struct {
-#ifdef HOST_WORDS_BIGENDIAN
-            uint32_t high, low;
-#else
-            uint32_t low, high;
-#endif
-        } l;
-    } u, res;
-    uint64_t rl, rh;
-
-    u.ll = a;
-    rl = (uint64_t)u.l.low * (uint64_t)b;
-    rh = (uint64_t)u.l.high * (uint64_t)b;
-    rh += (rl >> 32);
-    res.l.high = rh / c;
-    res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
-    return res.ll;
-}
-#endif
-
 /* Round number down to multiple */
 #define QEMU_ALIGN_DOWN(n, m) ((n) / (m) * (m))
 
diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index d4f21c9..c27d3dc 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -45,6 +45,12 @@ static inline void muls64(uint64_t *plow, uint64_t *phigh,
     *phigh = r >> 64;
 }
 
+/* compute with 96 bit intermediate result: (a*b)/c */
+static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
+{
+    return (__int128_t)a * b / c;
+}
+
 static inline int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
 {
     if (divisor == 0) {
@@ -75,6 +81,29 @@ void muls64(uint64_t *phigh, uint64_t *plow, int64_t a, int64_t b);
 void mulu64(uint64_t *phigh, uint64_t *plow, uint64_t a, uint64_t b);
 int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor);
 int divs128(int64_t *plow, int64_t *phigh, int64_t divisor);
+
+static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
+{
+    union {
+        uint64_t ll;
+        struct {
+#ifdef HOST_WORDS_BIGENDIAN
+            uint32_t high, low;
+#else
+            uint32_t low, high;
+#endif
+        } l;
+    } u, res;
+    uint64_t rl, rh;
+
+    u.ll = a;
+    rl = (uint64_t)u.l.low * (uint64_t)b;
+    rh = (uint64_t)u.l.high * (uint64_t)b;
+    rh += (rl >> 32);
+    res.l.high = rh / c;
+    res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
+    return res.ll;
+}
 #endif
 
 /**
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 5923d60..9939246 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -4,6 +4,7 @@
 #include "qemu/typedefs.h"
 #include "qemu-common.h"
 #include "qemu/notify.h"
+#include "qemu/host-utils.h"
 
 #define NANOSECONDS_PER_SECOND 1000000000LL
 
-- 
1.9.1

[Qemu-devel] [PATCH 09/12] apic_internal.h: Include cpu.h directly

[Peter Maydell]

apic_internal.h relies on cpu.h having been included (for the
X86CPU type); include it directly rather than relying on it
being pulled in via one of the other includes like timer.h.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/i386/apic_internal.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index dc7a89d..26632ac 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -20,6 +20,7 @@
 #ifndef QEMU_APIC_INTERNAL_H
 #define QEMU_APIC_INTERNAL_H
 
+#include "cpu.h"
 #include "exec/memory.h"
 #include "hw/cpu/icc_bus.h"
 #include "qemu/timer.h"
-- 
1.9.1

[Qemu-devel] [PATCH 10/12] timer.h: Don't include qemu-common.h

[Peter Maydell]

Include the specific headers we need, rather than qemu-common.h.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/timer.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 9939246..6205024 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -1,8 +1,9 @@
 #ifndef QEMU_TIMER_H
 #define QEMU_TIMER_H
 
+#include "osdep.h"
+#include "glib-compat.h"
 #include "qemu/typedefs.h"
-#include "qemu-common.h"
 #include "qemu/notify.h"
 #include "qemu/host-utils.h"
 
-- 
1.9.1

[Qemu-devel] [PATCH 11/12] event_notifier.h: Don't include qemu-common.h

[Peter Maydell]

Don't include qemu-common.h or windows.h in event_notifier.h; instead
include osdep.h and typedefs.h, which is all we need.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/event_notifier.h | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/qemu/event_notifier.h b/include/qemu/event_notifier.h
index 88b57af..ca7491b 100644
--- a/include/qemu/event_notifier.h
+++ b/include/qemu/event_notifier.h
@@ -13,11 +13,8 @@
 #ifndef QEMU_EVENT_NOTIFIER_H
 #define QEMU_EVENT_NOTIFIER_H
 
-#include "qemu-common.h"
-
-#ifdef _WIN32
-#include <windows.h>
-#endif
+#include "osdep.h"
+#include "qemu/typedefs.h"
 
 struct EventNotifier {
 #ifdef _WIN32
-- 
1.9.1

[Qemu-devel] [PATCH 12/12] throttle.h: Don't include qemu-common.h

[Peter Maydell]

Replace an include of qemu-common.h with osdep.h and
qemu/typedefs.h.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/throttle.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 995b2d5..88dec1d 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -25,8 +25,8 @@
 #ifndef THROTTLE_H
 #define THROTTLE_H
 
-#include <stdint.h>
-#include "qemu-common.h"
+#include "osdep.h"
+#include "qemu/typedefs.h"
 #include "qemu/timer.h"
 
 typedef enum {
-- 
1.9.1

Re: [Qemu-devel] [PATCH 00/12] qemu-common.h/osdep.h refactoring

[Paolo Bonzini]

On 22/07/2015 18:43, Peter Maydell wrote:
> The real question here I guess is whether people like the
> direction I'm trying to go with this. If so, we can further
> reduce the number of qemu-common.h includes without too much
> difficulty with further patches.

I like it, and it nicely complements some patches I had to remove the
bswap.h and cpu.h includes from qemu-common.h.  Thanks!

Paolo

Re: [Qemu-devel] [PATCH 00/12] qemu-common.h/osdep.h refactoring

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> This series makes a start at cleaning up some of our headers
> to avoid the common problem of header files including qemu-common.h
> (which then in turn can lead to awkward circular includes).

Yes, please!

> One common cause of this is that we don't have any header
> which will include the basic things most header files require,
> except for qemu-common.h. This series fixes that by making
> 'osdep.h' that "common basic stuff" header. The idea is that:
>  * osdep.h can be included from anywhere, since it doesn't
>    include any other QEMU headers itself except a few very
>    restricted special purpose ones (config-host.h, compiler.h, etc)
>  * osdep.h provides:
>    + things everybody needs, like NULL, int32_t, container_of,
>      the CONFIG_* defines, etc
>    + things that will cause subtle problems if they're not
>      present everywhere (eg directly using system headers and
>      not getting the portability fixups will result in something
>      that builds on most but not all hosts)

I'd file the CONFIG_FOO under "things that may cause subtle problems if
they're not present everywhere" rather than "things everybody needs",
because

1. loads of code doesn't need any, and

2. code that does need it is prone to compile fine but misbehave when
the header providing its CONFIG_FOOs isn't included.

> So most places can just include osdep.h, not the full qemu-common.h.
>
> (I initially thought about defining a new header for this
> purpose, "qemu/basics.h" or some such, but in fact osdep.h
> was already very close to what I wanted so it didn't really
> seem like it was very useful to switch everything over.)
>
> The series has some minor cleanups, some shuffling around of
> things between qemu-common.h, osdep.h and compiler.h, and
> a couple of patches at the end that use osdep.h to allow
> dropping a qemu-common.h include from various header files,
> as a demonstration.
>
> The real question here I guess is whether people like the
> direction I'm trying to go with this. If so, we can further
> reduce the number of qemu-common.h includes without too much
> difficulty with further patches.

For what it's worth, here's how Autoconf wants things done: every .c
file includes config.h *first*, and nothing else includes it.  Easy to
check mechanically, safely avoids the subtle miscompiles mentioned
above.

config.h normally contains just #defines computed by configure, but you
can add whatever else you like, with AH_TOP() or AH_BOTTOM().

It normally is the "things that may cause subtle problems if they're not
present everywhere", but not "things everybody needs (but doesn't want
to include again and again)".  However, nothing stops you from adding
the latter to config.h as well.

My point is:

* Your envisaged role of osdep.h is a common, proven technique.

* Consider requiring every .c to include it before anything else, with
  no other includes allowed.

[...]

Re: [Qemu-devel] [PATCH 01/12] qapi/qmp-event.c: Don't manually include os-win32.h/os-posix.h

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:44:00PM +0100, Peter Maydell wrote:
> qmp-event.c already includes qemu-common.h, so manually including
> os-win32.h/os-posix.h is unnecessary (and potentially fragile,
> since it's duplicating the #ifdef logic that chooses which of the
> two we need). Remove the unnecessary include logic.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  qapi/qmp-event.c | 8 --------
>  1 file changed, 8 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 02/12] osdep.h: Remove qemu_printf

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:44:01PM +0100, Peter Maydell wrote:
> qemu_printf is an ancient remnant which has been a simple #define to
> printf for over a decade, and is used in only a few places. Expand
> it out in those places and remove the #define.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu/osdep.h | 2 --
>  monitor.c            | 4 ++--
>  user-exec.c          | 4 ++--
>  3 files changed, 4 insertions(+), 6 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 03/12] osdep.h: Move some compiler-specific things to compiler.h

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:44:02PM +0100, Peter Maydell wrote:
> osdep.h has a few things which are really compiler specific;
> move them to compiler.h, and include compiler.h from osdep.h.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu/compiler.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/qemu/osdep.h    | 48 +-----------------------------------------------
>  2 files changed, 48 insertions(+), 47 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 04/12] compiler.h: Use glue() in QEMU_BUILD_BUG_ON define

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:44:03PM +0100, Peter Maydell wrote:
> Rather than rolling custom concatenate-strings macros for the
> QEMU_BUILD_BUG_ON macro to use, use the glue() macro we already
> have (since it's now available to us in this header).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu/compiler.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 05/12] qemu-common.h: Move Win32 fixups into os-win32.h

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:44:04PM +0100, Peter Maydell wrote:
> qemu-common.h includes some fixups for things the Win32
> headers don't define or define weirdly. These really
> belong in os-win32.h, so move them there.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu-common.h     | 17 -----------------
>  include/sysemu/os-win32.h | 18 ++++++++++++++++++
>  2 files changed, 18 insertions(+), 17 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 06/12] osdep.h: Move some OS header includes and fixups from qemu-common.h

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:44:05PM +0100, Peter Maydell wrote:
> qemu-common.h has some system header includes and fixups for
> things that might be missing. This is really an OS dependency
> and belongs in osdep.h, so move it across.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu-common.h | 55 +--------------------------------------------------
>  include/qemu/osdep.h  | 49 ++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 49 insertions(+), 55 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 00/12] qemu-common.h/osdep.h refactoring

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:43:59PM +0100, Peter Maydell wrote:
> This series makes a start at cleaning up some of our headers
> to avoid the common problem of header files including qemu-common.h
> (which then in turn can lead to awkward circular includes).
> 
> One common cause of this is that we don't have any header
> which will include the basic things most header files require,
> except for qemu-common.h. This series fixes that by making
> 'osdep.h' that "common basic stuff" header. The idea is that:
>  * osdep.h can be included from anywhere, since it doesn't
>    include any other QEMU headers itself except a few very
>    restricted special purpose ones (config-host.h, compiler.h, etc)
>  * osdep.h provides:
>    + things everybody needs, like NULL, int32_t, container_of,
>      the CONFIG_* defines, etc
>    + things that will cause subtle problems if they're not
>      present everywhere (eg directly using system headers and
>      not getting the portability fixups will result in something
>      that builds on most but not all hosts)
> 
> So most places can just include osdep.h, not the full qemu-common.h.
> 
> (I initially thought about defining a new header for this
> purpose, "qemu/basics.h" or some such, but in fact osdep.h
> was already very close to what I wanted so it didn't really
> seem like it was very useful to switch everything over.)
> 
> The series has some minor cleanups, some shuffling around of
> things between qemu-common.h, osdep.h and compiler.h, and
> a couple of patches at the end that use osdep.h to allow
> dropping a qemu-common.h include from various header files,
> as a demonstration.
> 
> The real question here I guess is whether people like the
> direction I'm trying to go with this. If so, we can further
> reduce the number of qemu-common.h includes without too much
> difficulty with further patches.

I broadly like the direction of this change. I think it is
probably worth being strict about requiring #include of
osdep.h as the first header in every .c file. And validating
such a rule programmatically at make check time or equiv.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 00/12] qemu-common.h/osdep.h refactoring

[Peter Maydell]

On 31 July 2015 at 13:54, Daniel P. Berrange <berrange@redhat.com> wrote:
> I broadly like the direction of this change. I think it is
> probably worth being strict about requiring #include of
> osdep.h as the first header in every .c file. And validating
> such a rule programmatically at make check time or equiv.

Yeah, I think that validating that automatically would be cool.

-- PMM

Re: [Qemu-devel] [PATCH 00/12] qemu-common.h/osdep.h refactoring

[Daniel P. Berrange]

On Fri, Jul 31, 2015 at 01:57:29PM +0100, Peter Maydell wrote:
> On 31 July 2015 at 13:54, Daniel P. Berrange <berrange@redhat.com> wrote:
> > I broadly like the direction of this change. I think it is
> > probably worth being strict about requiring #include of
> > osdep.h as the first header in every .c file. And validating
> > such a rule programmatically at make check time or equiv.
> 
> Yeah, I think that validating that automatically would be cool.

FYI I just copied you on a series that shows one way we can achieve
that

https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg06268.html


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 07/12] osdep.h: Add header comment

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:44:06PM +0100, Peter Maydell wrote:
> Add a header comment to osdep.h, explaining what the header is for
> and some rules to avoid circular-include difficulties.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu/osdep.h | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 60ac27d..ab3c876 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -1,3 +1,27 @@
> +/*
> + * OS includes and handling of OS dependencies
> + *
> + * This header exists to pull in some common system headers that
> + * most code in QEMU will want, and to fix up some possible issues with
> + * it (missing defines, Windows weirdness, and so on).
> + *
> + * To avoid getting into possible circular include dependencies, this
> + * file should not include any other QEMU headers, with the exceptions
> + * of config-host.h, compiler.h, os-posix.h and os-win32.h, all of which
> + * are doing a similar job to this file and are under similar constraints.
> + *
> + * This header also contains prototypes for functions defined in
> + * os-*.c and util/oslib-*.c; those would probably be better split
> + * out into separate header files.
> + *
> + * In an ideal world this header would contain only:
> + *  (1) things which everybody needs
> + *  (2) things without which code would work on most platforms but
> + *      fail to compile or misbehave on a minority of host OSes
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 08/12] qemu-common.h: Move muldiv64() to host-utils.h

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:44:07PM +0100, Peter Maydell wrote:
> Move the muldiv64() function from qemu-common.h to host-utils.h.
> This puts it together with all the other arithmetic functions
> where we provide a version with __int128_t and a fallback
> without, and allows headers which need muldiv64() to avoid
> including qemu-common.h.
> 
> We don't include host-utils from qemu-common.h, to avoid dragging
> more things into qemu-common.h than it already has; in practice
> everywhere that needs muldiv64() can get it via qemu/timer.h.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu-common.h     | 31 -------------------------------
>  include/qemu/host-utils.h | 29 +++++++++++++++++++++++++++++
>  include/qemu/timer.h      |  1 +
>  3 files changed, 30 insertions(+), 31 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 09/12] apic_internal.h: Include cpu.h directly

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:44:08PM +0100, Peter Maydell wrote:
> apic_internal.h relies on cpu.h having been included (for the
> X86CPU type); include it directly rather than relying on it
> being pulled in via one of the other includes like timer.h.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/i386/apic_internal.h | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Danie P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 10/12] timer.h: Don't include qemu-common.h

[Daniel P. Berrange]

On Wed, Jul 22, 2015 at 05:44:09PM +0100, Peter Maydell wrote:
> Include the specific headers we need, rather than qemu-common.h.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu/timer.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index 9939246..6205024 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -1,8 +1,9 @@
>  #ifndef QEMU_TIMER_H
>  #define QEMU_TIMER_H
>  
> +#include "osdep.h"
> +#include "glib-compat.h"

I think there's probably a reasonable argument for glib-compat.h to
be part of osdep.h as essentially all source files use glib now and
so could potentially require the compat definitions.

As mentioned in the cover letter, my preference would be for the
osdep.h header to be mandated in all the .c files, which would
mean we wouldn't need to add it here.

Looks like the same points apply to your patches 11 & 12, so I
won't repeat it again in those.

>  #include "qemu/typedefs.h"
> -#include "qemu-common.h"
>  #include "qemu/notify.h"
>  #include "qemu/host-utils.h"

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 00/12] qemu-common.h/osdep.h refactoring

[Peter Maydell]

On 22 July 2015 at 17:43, Peter Maydell <peter.maydell@linaro.org> wrote:
> This series makes a start at cleaning up some of our headers
> to avoid the common problem of header files including qemu-common.h
> (which then in turn can lead to awkward circular includes).

The general consensus here seems to be that patches 1-9
are OK (and those have an R-by from Daniel), but that the
last three would be better dealt with by having osdep.h
be mandatory for .c files.

My suggestion is thus that we commit 1-9 now, which will
avoid them getting stale, and give us a sensible base for
mandating osdep.h inclusion everywhere.

I can just apply these directly to master, which seems the
easiest way to do it, unless anybody objects or would
particularly like to see them going through some other tree.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/12] qemu-common.h/osdep.h refactoring

[Peter Maydell]

On 13 August 2015 at 18:37, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 22 July 2015 at 17:43, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This series makes a start at cleaning up some of our headers
>> to avoid the common problem of header files including qemu-common.h
>> (which then in turn can lead to awkward circular includes).
>
> The general consensus here seems to be that patches 1-9
> are OK (and those have an R-by from Daniel), but that the
> last three would be better dealt with by having osdep.h
> be mandatory for .c files.
>
> My suggestion is thus that we commit 1-9 now, which will
> avoid them getting stale, and give us a sensible base for
> mandating osdep.h inclusion everywhere.
>
> I can just apply these directly to master, which seems the
> easiest way to do it, unless anybody objects or would
> particularly like to see them going through some other tree.

Those first 9 patches now committed to master; thanks.
I'll rework/reroll the rest at some point.

-- PMM