Josh Kunz <j...@google.com> writes:
> This change introduces a new logging mask "LOG_USER", which is used for
> masking general user-mode logging. This change also switches all non-strace
> uses of `gemu_log' in linux-user/ to use `qemu_log_mask(LOG_USER, ...)'
> instead. This allows the user to easily log to a file, and to mask out
> these log messages if they desire.
>
> Signed-off-by: Josh Kunz <j...@google.com>
> ---
> include/qemu/log.h | 2 ++
> linux-user/arm/cpu_loop.c | 5 ++--
> linux-user/fd-trans.c | 55 +++++++++++++++++++++++++--------------
> linux-user/main.c | 24 +++++++++++++++++
> linux-user/syscall.c | 30 ++++++++++++---------
> linux-user/vm86.c | 3 ++-
> util/log.c | 3 +++
> 7 files changed, 86 insertions(+), 36 deletions(-)
>
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index e0f4e40628..503e4f88d5 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -62,6 +62,8 @@ static inline bool qemu_log_separate(void)
> #define CPU_LOG_TB_OP_IND (1 << 16)
> #define CPU_LOG_TB_FPU (1 << 17)
> #define CPU_LOG_PLUGIN (1 << 18)
> +/* LOG_USER is used for some informational user-mode logging. */
> +#define LOG_USER (1 << 19)
As Laurent said I think LOG_UNIMP is perfectly fine for stuff we haven't
done. I don't think any of the cases warrant LOG_GUEST_ERROR.
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 8718d03ee2..c4f3de77db 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -60,6 +60,9 @@ unsigned long mmap_min_addr;
> unsigned long guest_base;
> int have_guest_base;
>
> +/* Used to implement backwards-compatibility for user-mode logging. */
> +static bool force_user_mode_logging = true;
> +
I'm not sure want to bother with this. I know we like to avoid
regression but isn't this all debug log stuff? If we must keep it can we
invert the variable to save the initialisation.
> /*
> * 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.
> @@ -399,6 +402,11 @@ static void handle_arg_abi_call0(const char *arg)
> }
> #endif
>
> +static void handle_arg_no_force_user_mode_logging(const char *arg)
> +{
> + force_user_mode_logging = false;
> +}
> +
> static QemuPluginList plugins = QTAILQ_HEAD_INITIALIZER(plugins);
>
> #ifdef CONFIG_PLUGIN
> @@ -469,6 +477,10 @@ static const struct qemu_argument arg_table[] = {
> {"xtensa-abi-call0", "QEMU_XTENSA_ABI_CALL0", false,
> handle_arg_abi_call0,
> "", "assume CALL0 Xtensa ABI"},
> #endif
> + {"no-force-user-mode-logging", "", false,
> + handle_arg_no_force_user_mode_logging,
> + "", "disable forced user-mode logging, other logging options "
> + "will be used exactly as provided" },
> {NULL, NULL, false, NULL, NULL, NULL}
> };
>
> @@ -661,6 +673,18 @@ int main(int argc, char **argv, char **envp)
>
> optind = parse_args(argc, argv);
>
> + if (force_user_mode_logging) {
> + /*
> + * Backwards Compatibility: gemu_log for non-strace messages was not
> + * configurable, and was always on. Unless the user explicitly
> disables
> + * forced LOG_USER, force LOG_USER into the mask.
> + */
> + qemu_add_log(LOG_USER);
> + }
> +
> + qemu_log_mask(LOG_USER, "--> from user\n");
> + qemu_log_mask(LOG_STRACE, "--> from strace\n");
> +
I mean we jumped through hoops to maintain backwards compatibility and
then added new output? Also LOG_STRACE doesn't exist yet.
> if (!trace_init_backends()) {
> exit(1);
> }
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 171c0caef3..7e23dd6327 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1555,7 +1555,7 @@ static inline abi_long target_to_host_cmsg(struct
> msghdr *msgh,
> * something more intelligent than "twice the size of the
> * target buffer we're reading from".
> */
> - gemu_log("Host cmsg overflow\n");
> + qemu_log_mask(LOG_USER, "Host cmsg overflow\n");
I'm not sure we shouldn't just be asserting this case above. The
comments imply it is a bug on our part. The rest look like good cases
for LOG_UNIMP.
--
Alex Bennée
