[Qemu-devel] [RFC PATCH 0/2] Eliminate references to logfile/loglevel global vars

[Qemu-devel] [RFC PATCH 0/2] Eliminate references to logfile/loglevel global vars

[Eduardo Habkost]

Hi,

This is actually a long 63-patch series, but for a first request for comments,
I am splitting it in two parts:

1) A set of macros are introduced for almost all usage patterns of the
   logfile/loglevel global variables.
2) 62 patches that convert almost all references to the logfile/loglevel
   global variables to use those macros (the PATCH 1/2 e-mail actually
   has pointers to the patches, either as small individual patches,
   or as a big single patch file).

These macros are _not_ a proposal for a definitive new logging API. They
are just a step for it: a way to group all usage patterns of the
logfile/loglevel variables on a single place. After all usage of those
variables is eliminated, we can more easily refactor the code to have
a proper logging API.

This series needs my previous debugging-#ifdef cleanup series to be applied
before it.

It shouldn't change any qemu behavior, but just move all references
to the logging global variables to a single place, keeping exactly the
same behaviour.

-- 
Eduardo

[Qemu-devel] [RFC PATCH 1/2] Create macros that will help define the logging API

[Eduardo Habkost]

These macros are _not_ a proposal for a definitive new logging API. They
are just a step for it: a way to group all usage patterns of the
logfile/loglevel variables on a single place.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qemu-log.h |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 86 insertions(+), 0 deletions(-)

diff --git a/qemu-log.h b/qemu-log.h
index 1ebea81..fccfb11 100644
--- a/qemu-log.h
+++ b/qemu-log.h
@@ -1,7 +1,93 @@
 #ifndef QEMU_LOG_H
 #define QEMU_LOG_H
 
+/* The deprecated global variables: */
 extern FILE *logfile;
 extern int loglevel;
 
+
+/* 
+ * The new API:
+ *
+ */
+
+/* Log settings checking macros: */
+
+/* Returns true if qemu_log() will really write somewhere
+ */
+#define qemu_log_enabled() (logfile != NULL)
+
+/* Returns true if a bit is set in the current loglevel mask
+ */
+#define qemu_loglevel_mask(b) ((loglevel & (b)) != 0)
+
+
+/* Logging functions: */
+
+/* main logging function
+ */
+#define qemu_log(...) do {                 \
+        if (logfile)                       \
+            fprintf(logfile, ## __VA_ARGS__); \
+    } while (0)
+
+/* vfprintf-like logging function
+ */
+#define qemu_log_vprintf(fmt, va) do {     \
+        if (logfile)                       \
+            vfprintf(logfile, fmt, va);    \
+    } while (0)
+
+/* log only if a bit is set on the current loglevel mask
+ */
+#define qemu_log_mask(b, ...) do {         \
+        if (loglevel & (b))                \
+            fprintf(logfile, ## __VA_ARGS__); \
+    } while (0)
+
+
+
+
+/* Special cases: */
+
+/* cpu_dump_state() logging functions: */
+#define log_cpu_state(env, f) cpu_dump_state((env), logfile, fprintf, (f));
+#define log_cpu_state_mask(b, env, f) do {           \
+      if (loglevel & (b)) log_cpu_state((env), (f)); \
+  } while (0)
+
+/* disas() and target_disas() to logfile: */
+#define log_target_disas(start, len, flags) \
+        target_disas(logfile, (start), (len), (flags))
+#define log_disas(start, len) \
+        disas(logfile, (start), (len))
+
+/* page_dump() output to the log file: */
+#define log_page_dump() page_dump(logfile)
+
+
+
+/* Maintenance: */
+
+/* fflush() the log file */
+#define qemu_log_flush() fflush(logfile)
+
+/* Close the log file */
+#define qemu_log_close() do { \
+        fclose(logfile);      \
+        logfile = NULL;       \
+    } while (0)
+
+/* Set up a new log file */
+#define qemu_log_set_file(f) do { \
+        logfile = (f);            \
+    } while (0)
+
+/* Set up a new log file, only if none is set */
+#define qemu_log_try_set_file(f) do { \
+        if (!logfile)                 \
+            logfile = (f);            \
+    } while (0)
+
+
 #endif
-- 
1.5.5.GIT

[Qemu-devel] [RFC PATCH URL 2/2] Big conversion to the new logging macros

[Eduardo Habkost]

This is a really big series, so I've made it available via HTTP instead,
at:

  http://raisama.net/tmp/qemu-log-macros/

You can get the 62 individual patches there, or look at all changes on
a big single patch at:

  http://raisama.net/tmp/qemu-log-macros/0000-BIG-SINGLE-PATCH.patch

With this series, almost all references to the logfile/loglevel global
variables outside of qemu-log.h were eliminated. except for:

* cpu_set_log*() functions at exec.c, that I plan to move to a qemu-log.c
  file on further patches; and

* these two cases at tcg/tcg.c:
  tcg/tcg.c:1920:        tcg_dump_ops(s, logfile);
  tcg/tcg.c:1936:        tcg_dump_ops(s, logfile);


The whole series is also available on a git reposity at:

  git://github.com/ehabkost/qemu-hacks.git hacking/log-macros

---

The following changes since commit 303dccf5fba94342c5001648d60ed2caf88898a1:
  Eduardo Habkost (1):
        Define macros that will become the new logging API

are available in the git repository at:

  git://github.com/ehabkost/qemu-hacks.git hacking/log-macros

Eduardo Habkost (62):
      target-i386/op_helper.c: qemu_log_mask() replacements
      target-i386/op_helper.c: simultaneous qemu_log_mask()/log_cpu_state_mask() replacements
      target-i386/op_helper.c: qemu_log()/log_cpu_state() replacements
      target-i386/op_helper.c: debug macros: use qemu_log*()
      target-i386/translate.c: use log_cpu_state_mask()
      target-i386/translate.c: use qemu_log()/log_target_disas()
      target-mips/helper.c: use qemu_log()/log_cpu_state()
      target-mips/helper.c: remaining qemu_log() replacements
      target-mips/op_helper.c: move loglevel check into debug_pre_eret()
      target-mips/op_helper.c: move loglevel check into debug_post_eret()
      target-mips/op_helper.c: qemu_log() replacements
      target-mips/op_helper.c: remaining qemu_log() replacement
      target-mips/translate.c: debug macros: use qemu_log_mask()
      target-mips/translate.c: remaining qemu_log*() conversions
      hw/mips_timer.c: qemu_log() conversions
      target-ppc/helper.c: qemu_log() conversions
      target-ppc/helper.c: qemu_log_mask() conversions
      target-ppc/helper.c: qemu_log_enabled()/qemu_log() conversions
      target-ppc/helper.c: move loglevel check into dump_syscall()
      target-ppc/helper.c: dump_syscall(): use qemu_log_mask()
      target-ppc/helper.c: debug macros: use qemu_log()
      target-ppc/helper.c: cpu_dump_rfi(): use qemu_log()
      target-ppc/op_helper.c: qemu_log() conversions
      target-ppc/op_helper.c: LOG_SWTLB: use qemu_log()
      target-ppc/translate.c: qemu_log() conversions
      target-ppc/translate.c: qemu_log_enabled()/qemu_log() conversions
      target-ppc/translate.c: LOG_DISAS: use qemu_log_mask()
      target-ppc/translate.c: remaining qemu_log*() conversions
      hw/ppc.c: debug macros: use qemu_log*()
      hw/ppc4xx_devs.c: debug macros: use qemu_log*()
      target-sparc/op_helper.c: qemu_log*() conversions
      target-sparc/translate.c: qemu_log*() conversions
      target-cris/op_helper.c: helper_dump: use qemu_log()
      target-cris/translate.c: qemu_log*() conversions
      target-cris/translate.c: LOG_DIS: use qemu_log_mask()
      target-cris/translate.c: use qemu_log_try_set_file()
      target-cris/*: D_LOG: use qemu_log()
      hw/alpha_palcode.c: qemu_log() conversions
      target-alpha/translate.c: qemu_log*() conversions
      target-alpha/translate.c: LOG_DISAS: use qemu_log()
      target-sh4/helper.c: qemu_log*() conversions
      target-sh4/translate.c: qemu_log_*() conversions
      target-m68k/translate.c: qemu_log*() conversions
      target-arm/translate.c: qemu_log*() conversions
      kqemu.c: LOG_INT: use qemu_log_mask()
      kqemu.c: LOG_INT_STATE: use log_cpu_state_mask()
      linux-user/main.c: qemu_log*() conversions
      linux-user/vm86.c: LOG_VM86: use qemu_log()
      linux-user/elfload.c: load_elf_binary(): use qemu_log_enabled()
      bsd-user/main.c: use log_page_dump()/qemu_log()
      bsd-user/elfload.c: load_elf_binary(): use qemu_log_enabled()
      darwin-user/main.c: qemu_log*() conversions
      darwin-user: DPRINTF: use qemu_log()
      block-raw-posix.c: qemu_log*() conversions
      translate-all.c: qemu_log*() conversions
      cpu-exec.c: qemu_log*() conversions
      exec.c: qemu_log*() conversions
      exec.c: cpu_abort(): use qemu_log_close()
      tcg.c: qemu_log() conversions
      vl.c: LOG_IOPORT: use qemu_log_mask()
      global s/fflush(logfile)/qemu_log_flush()/
      global s/loglevel & X/qemu_loglevel_mask(X)/

 block-raw-posix.c        |    4 +-
 bsd-user/elfload.c       |    2 +-
 bsd-user/main.c          |   28 ++++++-------
 cpu-exec.c               |   35 +++++++---------
 darwin-user/commpage.c   |    4 +-
 darwin-user/machload.c   |    4 +-
 darwin-user/main.c       |    6 +--
 darwin-user/syscall.c    |    4 +-
 exec.c                   |   26 ++++++-------
 hw/alpha_palcode.c       |   15 ++-----
 hw/mips_timer.c          |    4 +-
 hw/ppc.c                 |   10 +----
 hw/ppc4xx_devs.c         |    5 +--
 hw/ppc_prep.c            |   11 +----
 kqemu.c                  |   11 +----
 linux-user/elfload.c     |    2 +-
 linux-user/main.c        |   34 +++++++---------
 linux-user/vm86.c        |    2 +-
 target-alpha/translate.c |   17 +++-----
 target-arm/translate.c   |   10 ++--
 target-cris/helper.c     |    2 +-
 target-cris/mmu.c        |    2 +-
 target-cris/op_helper.c  |    4 +-
 target-cris/translate.c  |   28 ++++++-------
 target-i386/op_helper.c  |   79 +++++++++++++-----------------------
 target-i386/translate.c  |   24 +++++------
 target-m68k/translate.c  |   12 +++---
 target-mips/helper.c     |   39 +++++++------------
 target-mips/op_helper.c  |   68 ++++++++++++++++----------------
 target-mips/translate.c  |   41 +++++++------------
 target-ppc/helper.c      |   98 +++++++++++++--------------------------------
 target-ppc/op_helper.c   |   33 ++++------------
 target-ppc/translate.c   |   57 ++++++++++----------------
 target-sh4/helper.c      |    8 ++--
 target-sh4/translate.c   |   19 ++++-----
 target-sparc/op_helper.c |   24 ++++++------
 target-sparc/translate.c |   19 ++++-----
 tcg/tcg.c                |   12 +++---
 translate-all.c          |   10 ++--
 vl.c                     |    5 +--
 40 files changed, 321 insertions(+), 497 deletions(-)

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH URL 2/2] Big conversion to the new logging macros

[Anthony Liguori]

Eduardo Habkost wrote:
> This is a really big series, so I've made it available via HTTP instead,
> at:
>
>   http://raisama.net/tmp/qemu-log-macros/
>
> You can get the 62 individual patches there, or look at all changes on
> a big single patch at:
>
>   http://raisama.net/tmp/qemu-log-macros/0000-BIG-SINGLE-PATCH.patch
>   

I like it and inclined to commit the big patch once everyone's gotten a 
chance to weigh in.

Regards,

Anthony Liguori

Re: [Qemu-devel] [RFC PATCH URL 2/2] Big conversion to the new logging macros

[Blue Swirl]

On 12/12/08, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Eduardo Habkost wrote:
>
> > This is a really big series, so I've made it available via HTTP instead,
> > at:
> >
> >  http://raisama.net/tmp/qemu-log-macros/
> >
> > You can get the 62 individual patches there, or look at all changes on
> > a big single patch at:
> >
> >
> http://raisama.net/tmp/qemu-log-macros/0000-BIG-SINGLE-PATCH.patch
> >
> >
>
>  I like it and inclined to commit the big patch once everyone's gotten a
> chance to weigh in.

Looking good.

Re: [Qemu-devel] [RFC PATCH 0/2] Eliminate references to logfile/loglevel global vars

[Dor Laor]

Eduardo Habkost wrote:
> Hi,
>
> This is actually a long 63-patch series, but for a first request for comments,
> I am splitting it in two parts:
>
> 1) A set of macros are introduced for almost all usage patterns of the
>    logfile/loglevel global variables.
> 2) 62 patches that convert almost all references to the logfile/loglevel
>    global variables to use those macros (the PATCH 1/2 e-mail actually
>    has pointers to the patches, either as small individual patches,
>    or as a big single patch file).
>
> These macros are _not_ a proposal for a definitive new logging API. They
> are just a step for it: a way to group all usage patterns of the
> logfile/loglevel variables on a single place. After all usage of those
> variables is eliminated, we can more easily refactor the code to have
> a proper logging API.
>
> This series needs my previous debugging-#ifdef cleanup series to be applied
> before it.
>
> It shouldn't change any qemu behavior, but just move all references
> to the logging global variables to a single place, keeping exactly the
> same behaviour.
>
>   
It is a blessed addition.

What about providing configure option to enable all the debug #ifdef?
Once we have it, we can set the debug level using the monitor for all debug
targets. For kvm is shouldn't introduce a measurable overhead since
it mostly uses the device model. It will be also ease debugging qemu
when there is no clue about the root cause it hard to predict at compile 
time.

We can have something like ./configure --full-debug
and #ifdef (FULL_DEBUG || DEBUG_IO_PORTS) for example.