From: "Alex Bennée" <alex.bennee@linaro.org>
To: Josh Kunz <jkz@google.com>
Cc: armbru@redhat.com, riku.voipio@iki.fi, qemu-devel@nongnu.org,
laurent@vivier.eu
Subject: Re: [PATCH 1/4] linux-user: Use `qemu_log' for non-strace logging
Date: Tue, 14 Jan 2020 10:42:59 +0000 [thread overview]
Message-ID: <87pnfmmi8c.fsf@linaro.org> (raw)
In-Reply-To: <20200114030138.260347-2-jkz@google.com>
Josh Kunz <jkz@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 <jkz@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
next prev parent reply other threads:[~2020-01-14 10:44 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-14 3:01 [PATCH 0/4] migration: Replace gemu_log with qemu_log Josh Kunz
2020-01-14 3:01 ` [PATCH 1/4] linux-user: Use `qemu_log' for non-strace logging Josh Kunz
2020-01-14 8:28 ` Laurent Vivier
2020-01-14 8:30 ` Laurent Vivier
2020-01-14 10:42 ` Alex Bennée [this message]
2020-01-17 19:28 ` Josh Kunz
2020-01-14 3:01 ` [PATCH 2/4] linux-user: Use `qemu_log' for strace Josh Kunz
2020-01-14 9:09 ` Laurent Vivier
2020-01-14 16:15 ` Paolo Bonzini
2020-01-17 19:28 ` Josh Kunz
2020-01-14 10:55 ` Alex Bennée
2020-01-17 19:28 ` Josh Kunz
2020-01-14 3:01 ` [PATCH 3/4] linux-user: remove gemu_log from the linux-user tree Josh Kunz
2020-01-14 10:56 ` Alex Bennée
2020-01-14 3:01 ` [PATCH 4/4] bsd-user: Replace gemu_log with qemu_log Josh Kunz
2020-01-14 3:06 ` [PATCH 0/4] migration: " Warner Losh
2020-01-16 23:13 ` Josh Kunz
2020-01-14 11:02 ` Alex Bennée
2020-01-16 23:12 ` Josh Kunz
2020-01-20 11:36 ` Alex Bennée
2020-02-04 2:55 ` Josh Kunz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pnfmmi8c.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=armbru@redhat.com \
--cc=jkz@google.com \
--cc=laurent@vivier.eu \
--cc=qemu-devel@nongnu.org \
--cc=riku.voipio@iki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).