Josh Kunz 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
> ---
> 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, >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
