Re: [PATCH v2 2/4] linux-user: Use `qemu_log' for strace

[Laurent Vivier]

Le 17/01/2020 à 20:28, Josh Kunz a écrit :
> 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).
> 
> This change introduces a new log mask: `LOG_STRACE` which is used for
> logging of user-mode strace messages.
> 
> Signed-off-by: Josh Kunz <address@hidden>
> ---
>  include/qemu/log.h   |   2 +
>  linux-user/main.c    |  30 ++-
>  linux-user/qemu.h    |   1 -
>  linux-user/signal.c  |   2 +-
>  linux-user/strace.c  | 479 ++++++++++++++++++++++---------------------
>  linux-user/syscall.c |  13 +-
>  util/log.c           |   2 +
>  7 files changed, 278 insertions(+), 251 deletions(-)
> 
...
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 629f3a21b5..54e60f3807 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -12098,14 +12098,15 @@ abi_long do_syscall(void *cpu_env, int num, 
> abi_long arg1,
>      record_syscall_start(cpu, num, arg1,
>                           arg2, arg3, arg4, arg5, arg6, arg7, arg8);
>  
> -    if (unlikely(do_strace)) {
> +    if (unlikely(qemu_loglevel_mask(LOG_STRACE))) {
>          print_syscall(num, arg1, arg2, arg3, arg4, arg5, arg6);
> -        ret = do_syscall1(cpu_env, num, arg1, arg2, arg3, arg4,
> -                          arg5, arg6, arg7, arg8);
> +    }
> +
> +    ret = do_syscall1(cpu_env, num, arg1, arg2, arg3, arg4,
> +                      arg5, arg6, arg7, arg8);
> +
> +    if (unlikely(qemu_loglevel_mask(LOG_STRACE))) {
>          print_syscall_ret(num, ret);
> -    } else {
> -        ret = do_syscall1(cpu_env, num, arg1, arg2, arg3, arg4,
> -                          arg5, arg6, arg7, arg8);
>      }
>  
>      record_syscall_return(cpu, num, ret);

In term of performance perhaps it sould be better to only test once for
the mask as it is done before?

For the other parts:

Reviewed-by: Laurent Vivier <address@hidden>

Thanks,
Laurent