Re: [PATCH v3 0/4] migration: Replace gemu_log with qemu_log

[Laurent Vivier]

Le 04/02/2020 à 03:54, Josh Kunz a écrit :
> Summary of v2->v3 changes:
>   * Removed assert for CMSG handling, replaced with LOG_UNIMP. Will
>     switch to assert in follow-up patch.
>   * Fixed BSD-user build (dangling references to qemu_add_log), and
>     verified the user-mode build works.
> 
> Summary of v1->v2 changes:
>   * Removed backwards-compatibility code for non-strace log statements.
>   * Removed new qemu_log interface for adding or removing fields from
>     the log mask.
>   * Removed LOG_USER and converted all uses (except one) to LOG_UNIMP.
>     * One gemu_log statement was converted to an assert.
>   * Some style cleanup.
> 
> The linux-user and bsd-user trees both widely use a function called
> `gemu_log` (notice the 'g') for miscellaneous and strace logging. This
> function predates the newer `qemu_log` function, and has a few drawbacks
> compared to `qemu_log`:
> 
>   1. Always logs to `stderr`, no logging redirection.
>   2. "Miscellaneous" logging cannot be disabled, so it may mix with guest
>      logging.
>   3. Inconsistency with other parts of the QEMU codebase, and a
>      confusing name.
> 
> The second issue is especially troubling because it can interfere with
> programs that expect to communicate via stderr.
> 
> This change introduces one new logging masks to the `qemu_log` subsystem
> to support its use for user-mode logging: the `LOG_STRACE` mask for
> strace-specific logging. Further, it replaces all existing uses of
> `gemu_log` with the appropriate `qemu_log_mask(LOG_{UNIMP,STRACE}, ...)`
> based on the log message.
> 
> Backwards incompatibility:
>   * Log messages for unimplemented user-mode features are no longer
>     logged by default. They have to be enabled by setting the LOG_UNIMP
>     mask.
>   * Log messages for strace/unimplemented user-mode features may be
>     redirected based on `-D`, instead of always logging to stderr.
> 
> Tested:
>   * Built with clang 9 and g++ 8.3
>   * `make check` run with clang 9 build 
>   * Verified:
>     * QEMU_STRACE/-strace still works for linux-user
>   * `make vm-build-netbsd EXTRA_CONFIGURE_OPTS="--disable-system" \
>      BUILD_TARGET="all"` passed.
> 
> Josh Kunz (4):
>   linux-user: Use `qemu_log' for non-strace logging
>   linux-user: Use `qemu_log' for strace
>   linux-user: remove gemu_log from the linux-user tree
>   bsd-user: Replace gemu_log with qemu_log
> 
>  bsd-user/main.c           |  29 ++-
>  bsd-user/qemu.h           |   2 -
>  bsd-user/strace.c         |  32 ++-
>  bsd-user/syscall.c        |  31 ++-
>  include/qemu/log.h        |   2 +
>  linux-user/arm/cpu_loop.c |   5 +-
>  linux-user/fd-trans.c     |  55 +++--
>  linux-user/main.c         |  39 ++--
>  linux-user/qemu.h         |   2 -
>  linux-user/signal.c       |   2 +-
>  linux-user/strace.c       | 479 +++++++++++++++++++-------------------
>  linux-user/syscall.c      |  48 ++--
>  linux-user/vm86.c         |   3 +-
>  util/log.c                |   2 +
>  14 files changed, 387 insertions(+), 344 deletions(-)
> 

Applied patches 1 to 3 to my linux-user branch.

Thanks,
LAurent