All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: "Markus Armbruster" <armbru@redhat.com>,
	qemu-devel@nongnu.org,
	"Manos Pitsidianakis" <manos.pitsidianakis@linaro.org>,
	"Stefan Weil" <sw@weilnetz.de>,
	"Dr. David Alan Gilbert" <dave@treblig.org>,
	"Pierrick Bouvier" <pierrick.bouvier@linaro.org>,
	devel@lists.libvirt.org,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Kevin Wolf" <kwolf@redhat.com>,
	qemu-block@nongnu.org, qemu-rust@nongnu.org,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Christian Schoenebeck" <qemu_oss@crudebyte.com>,
	"Richard Henderson" <richard.henderson@linaro.org>
Subject: Re: [PATCH v6 25/27] util: add support for formatting a program name in messages
Date: Thu, 26 Feb 2026 08:11:57 +0100	[thread overview]
Message-ID: <871pi8htc2.fsf@pond.sub.org> (raw)
In-Reply-To: <aZ8iSclFzGgPEH1t@redhat.com> ("Daniel P. Berrangé"'s message of "Wed, 25 Feb 2026 16:24:41 +0000")

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Feb 19, 2026 at 11:08:45AM +0100, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > The error_report function can include the program name in any
>> > messages it prints. The qemu_log function has no equivalent
>> > behaviour.
>> >
>> > This introduces support for a "program name" in the new
>> > messages API, which will be included by default for all
>> > binaries.
>> >
>> > This change tweaks the output of the error_report function,
>> > adding a space between the program name and the location
>> > info.
>> 
>> Why?
>
> Looking again, arguably that is fixing a pre-existing bug
>
> The current code is:
>
>     if (!monitor_cur() && g_get_prgname()) {
>         error_printf("%s:", g_get_prgname());
>         sep = " ";
>     }
>     switch (cur_loc->kind) {
>     case LOC_CMDLINE:
>         argp = cur_loc->ptr;
>         for (i = 0; i < cur_loc->num; i++) {
>             error_printf("%s%s", sep, argp[i]);
>             sep = " ";
>         }
>         error_printf(": ");
>         break;
>     case LOC_FILE:
>         error_printf("%s:", (const char *)cur_loc->ptr);
>         if (cur_loc->num) {
>             error_printf("%d:", cur_loc->num);
>         }
>         error_printf(" ");
>         break;
>     default:
>         error_printf("%s", sep);
>     }
>
>
> Notice how we honour "sep" for LOC_CMDLINE but ignore
> it for LOC_FILE.

This works exactly as intended.  To illustrate, add

    error_report("test, ignore");

to qdev_device_add(), then:

    $ cat t.cfg
    [device]
      driver = "usb-kbd"
    $ qemu-system-x86_64 -S -display none -machine usb=on -device usb-kbd -readconfig t.cfg -monitor stdio
    QEMU 10.2.50 monitor - type 'help' for more information
    qemu-system-x86_64: -device usb-kbd: test, ignore
    qemu-system-x86_64:t.cfg:2: test, ignore
    (qemu) device_add usb-kbd
    test, ignore

The format

    program:sourcefile:lineno: message

conforms to the GNU Coding Standards at
<https://www.gnu.org/prep/standards/standards.html#Errors>, and matches
common practice.

> With the program name printing moved out into a separate
> function, the space is always printed. No change for
> LOC_CMDLINE, but new whitespace for LOC_FILE.
>
> Should have changed this in a separate patch.
>
>> >       The qemu_log function will gain the program name. This
>> > can be easily seen with the 'log' trace backend, and how it
>> > is now more closely matching error_report output.
>> >
>> > Before:
>> >
>> >   # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>> >   qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5584e13937f0 dir=fish
>> >   qcrypto_tls_creds_get_path TLS creds path creds=0x5584e13937f0 filename=ca-cert.pem path=<none>
>> >   qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
>> >
>> > After:
>> >
>> >   # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>> >   qemu-system-x86_64: qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5584e13937f0 dir=fish
>> >   qemu-system-x86_64: qcrypto_tls_creds_get_path TLS creds path creds=0x5584e13937f0 filename=ca-cert.pem path=<none>
>> >   qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
>> >
>> > When adding this the '-msg program-name=on|off' option is
>> > introduced, so that the program name (which is enabled by
>> > default) can be supressed if desired. This could be useful
>> > if '-msg guest-name=on' is being used as a more informative
>> > identifier.
>> 
>> Separate patch?
>
> It is a bit of a borderline decision, but I felt the knob to turn it
> off should be in the same patch that force enables it.

Peter objected to making the logs more verbose by default, and I think
he has a point.  Perhaps having errors and logs share a general format
is useful anyway, and perhaps making it even more configurable is as
well.  I figure we'd need separate configuration for logs and for error
messages.

Let's get to rough consensus before we debate the finer points like this
one.

>> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>
> With regards,
> Daniel



  reply	other threads:[~2026-02-26  7:12 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-11 15:24 [PATCH v6 00/27] util: sync error_report & qemu_log output more closely Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 01/27] meson: don't access 'cxx' object without checking cpp lang Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 02/27] qemu-options: remove extraneous [] around arg values Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 03/27] include: define constant for early constructor priority Daniel P. Berrangé
2026-02-18  9:22   ` Markus Armbruster
2026-02-18 10:46     ` Daniel P. Berrangé
2026-02-18 13:23       ` Markus Armbruster
2026-02-11 15:24 ` [PATCH v6 04/27] monitor: initialize global data from a constructor Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 05/27] system: unconditionally enable thread naming Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 06/27] util: fix race setting thread name on Win32 Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 07/27] util: expose qemu_thread_set_name Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 08/27] audio: make jackaudio use qemu_thread_set_name Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 09/27] util: set the name for the 'main' thread on Windows Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 10/27] util: add API to fetch the current thread name Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 11/27] util: introduce some API docs for logging APIs Daniel P. Berrangé
2026-02-18  9:38   ` Markus Armbruster
2026-02-11 15:24 ` [PATCH v6 12/27] util: avoid repeated prefix on incremental qemu_log calls Daniel P. Berrangé
2026-02-18  9:52   ` Markus Armbruster
2026-02-18 10:45     ` Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 13/27] util/log: add missing error reporting in qemu_log_trylock_with_err Daniel P. Berrangé
2026-02-18 10:45   ` Markus Armbruster
2026-02-11 15:24 ` [PATCH v6 14/27] ui: add proper error reporting for password changes Daniel P. Berrangé
2026-02-18 12:10   ` Markus Armbruster
2026-02-25 16:08     ` Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 15/27] ui: remove redundant use of error_printf_unless_qmp() Daniel P. Berrangé
2026-02-18 12:12   ` Markus Armbruster
2026-02-11 15:24 ` [PATCH v6 16/27] monitor: remove redundant error_[v]printf_unless_qmp Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 17/27] monitor: refactor error_vprintf() Daniel P. Berrangé
2026-02-11 15:24 ` [PATCH v6 18/27] monitor: move error_vprintf back to error-report.c Daniel P. Berrangé
2026-02-11 15:25 ` [PATCH v6 19/27] util: fix interleaving of error & trace output Daniel P. Berrangé
2026-02-18 12:41   ` Markus Armbruster
2026-02-18 12:45   ` Markus Armbruster
2026-02-11 15:25 ` [PATCH v6 20/27] util: don't skip error prefixes when QMP is active Daniel P. Berrangé
2026-02-18 12:47   ` Markus Armbruster
2026-02-11 15:25 ` [PATCH v6 21/27] util: fix interleaving of error prefixes Daniel P. Berrangé
2026-02-11 15:25 ` [PATCH v6 22/27] util: introduce common helper for error-report & log code Daniel P. Berrangé
2026-02-18 14:04   ` Markus Armbruster
2026-02-25 16:18     ` Daniel P. Berrangé
2026-02-25 17:51       ` Markus Armbruster
2026-02-11 15:25 ` [PATCH v6 23/27] util: convert error-report & log to message API for timestamp Daniel P. Berrangé
2026-02-11 15:25 ` [PATCH v6 24/27] util: add support for formatting a workload name in messages Daniel P. Berrangé
2026-02-11 15:25 ` [PATCH v6 25/27] util: add support for formatting a program " Daniel P. Berrangé
2026-02-19 10:08   ` Markus Armbruster
2026-02-25 16:24     ` Daniel P. Berrangé
2026-02-26  7:11       ` Markus Armbruster [this message]
2026-02-19 10:23   ` Peter Maydell
2026-02-25 16:38     ` Daniel P. Berrangé
2026-02-25 17:43       ` Peter Maydell
2026-02-25 17:47         ` Daniel P. Berrangé
2026-02-11 15:25 ` [PATCH v6 26/27] util: add support for formatting thread info " Daniel P. Berrangé
2026-02-19 10:14   ` Markus Armbruster
2026-02-25 16:33     ` Daniel P. Berrangé
2026-02-19 10:29   ` Peter Maydell
2026-02-25 16:30     ` Daniel P. Berrangé
2026-02-25 17:39       ` Peter Maydell
2026-02-11 15:25 ` [PATCH v6 27/27] util: add brackets around guest name in message context Daniel P. Berrangé
2026-02-19 10:16   ` Markus Armbruster
2026-02-26  9:51 ` [PATCH v6 00/27] util: sync error_report & qemu_log output more closely Markus Armbruster
2026-02-26  9:58   ` Daniel P. Berrangé

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=871pi8htc2.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dave@treblig.org \
    --cc=devel@lists.libvirt.org \
    --cc=hreitz@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=manos.pitsidianakis@linaro.org \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=pierrick.bouvier@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-rust@nongnu.org \
    --cc=qemu_oss@crudebyte.com \
    --cc=richard.henderson@linaro.org \
    --cc=sw@weilnetz.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.