Josh Kunz <j...@google.com> writes:
> This change switches linux-user strace logging to use the newer `qemu_log` > logging subsystem rather than the older `gemu_log` (notice the "g") > logger. `qemu_log` has several advantages, namely that it allows logging > to a file, and provides a more unified interface for configuration > of logging (via the QEMU_LOG environment variable or options). > > Signed-off-by: Josh Kunz <j...@google.com> > --- > include/qemu/log.h | 13 ++ > linux-user/main.c | 17 +- > linux-user/qemu.h | 1 - > linux-user/signal.c | 3 +- > linux-user/strace.c | 479 ++++++++++++++++++++++--------------------- > linux-user/syscall.c | 13 +- > util/log.c | 2 + > 7 files changed, 282 insertions(+), 246 deletions(-) > > diff --git a/include/qemu/log.h b/include/qemu/log.h > index 503e4f88d5..8f044c1716 100644 > --- a/include/qemu/log.h > +++ b/include/qemu/log.h > @@ -64,6 +64,7 @@ static inline bool qemu_log_separate(void) > #define CPU_LOG_PLUGIN (1 << 18) > /* LOG_USER is used for some informational user-mode logging. */ > #define LOG_USER (1 << 19) > +#define LOG_STRACE (1 << 20) > > /* Lock output for a series of related logs. Since this is not needed > * for a single qemu_log / qemu_log_mask / qemu_log_mask_and_addr, we > @@ -154,6 +155,18 @@ void qemu_set_dfilter_ranges(const char *ranges, Error > **errp); > bool qemu_log_in_addr_range(uint64_t addr); > int qemu_str_to_log_mask(const char *str); > > +/* Add (union) the given "log_flags" to the current log mask. */ > +static inline void qemu_add_log(int log_flags) > +{ > + qemu_set_log(qemu_loglevel | log_flags); > +} > + > +/* Remove (subtract) the given log flags from the current log mask. */ > +static inline void qemu_del_log(int log_flags) > +{ > + qemu_set_log(qemu_loglevel & ~(log_flags)); > +} > + > /* Print a usage message listing all the valid logging categories > * to the specified FILE*. > */ > diff --git a/linux-user/main.c b/linux-user/main.c > index c4f3de77db..0bf40c4d27 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -63,6 +63,12 @@ int have_guest_base; > /* Used to implement backwards-compatibility for user-mode logging. */ > static bool force_user_mode_logging = true; > > +/* > + * Used to implement backwards-compatibility for the `-strace`, and > + * QEMU_STRACE options. > + */ > +static bool enable_strace; > + > /* > * When running 32-on-64 we should make sure we can fit all of the possible > * guest address space into a contiguous chunk of virtual host memory. > @@ -378,7 +384,7 @@ static void handle_arg_singlestep(const char *arg) > > static void handle_arg_strace(const char *arg) > { > - do_strace = 1; > + enable_strace = true; Could we cut out the middle-man and just qemu_add_log(LOG_STRACE) here and drop the enable_strace variable. > } > > static void handle_arg_version(const char *arg) > @@ -672,6 +678,15 @@ int main(int argc, char **argv, char **envp) > qemu_plugin_add_opts(); > > optind = parse_args(argc, argv); > + /* > + * Backwards Compatability: If handle_arg_strace just enabled strace > + * logging directly, then it could be accidentally turned off by a > + * QEMU_LOG/-d option. To make sure that strace logging is always enabled > + * when QEMU_STRACE/-strace is set, re-enable LOG_STRACE here. > + */ > + if (enable_strace) { > + qemu_add_log(LOG_STRACE); > + } > > if (force_user_mode_logging) { > /* > diff --git a/linux-user/qemu.h b/linux-user/qemu.h > index f6f5fe5fbb..02c6890c8a 100644 > --- a/linux-user/qemu.h > +++ b/linux-user/qemu.h > @@ -385,7 +385,6 @@ void print_syscall_ret(int num, abi_long arg1); > * --- SIGSEGV {si_signo=SIGSEGV, si_code=SI_KERNEL, si_addr=0} --- > */ > void print_taken_signal(int target_signum, const target_siginfo_t *tinfo); > -extern int do_strace; > > /* signal.c */ > void process_pending_signals(CPUArchState *cpu_env); > diff --git a/linux-user/signal.c b/linux-user/signal.c > index 5ca6d62b15..2ff0065804 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -864,9 +864,8 @@ static void handle_pending_signal(CPUArchState *cpu_env, > int sig, > handler = sa->_sa_handler; > } > > - if (do_strace) { > + if (unlikely(qemu_loglevel_mask(LOG_STRACE))) > print_taken_signal(sig, &k->info); > - } Please don't drop the brace - c.f. CODING_STYLE.rst Side note, print_taken_signal might want to consider using the qemu_log_lock() functionality to keep sequential qemu_log's together. I suspect you might want to do the same for syscalls. -- Alex Bennée