Re: [PATCH v2] net: remove an assert call in eth_get_gso_type

[Peter Maydell]

On Wed, 21 Oct 2020 at 07:29, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> During the last 2 years I've been sending patches touching
> various QEMU areas, but I never used qemu_log(). I always
> used:
> - qemu_log_mask(LOG_GUEST_ERROR/LOG_UNIMP, ...
> - error_report/warn_report from "qemu/error-report.h"
> - error_setg* from "qapi/error.h"
> - trace events
>
> $ git grep qemu_log\( | wc -l
> 661
>
> This function seems used mostly by very old code.
>
> It is declared in "qemu/log-for-trace.h" which looks like
> an internal API.
>
> Should we add a checkpatch rule to refuse new uses of qemu_log()?

The major use for qemu_log() is when you're constructing
a multi-line log message or a log message which needs to
do some expensive calculations to work out what it is going
to print. In that case the pattern is:

    if (qemu_loglevel_mask(LOG_WHATEVER)) {
        int x = do_my_expensive_calculations();
        qemu_log("line one: foo: 0x%x\n", x);
        for (some loop over a list) {
           qemu_log("and another line per list item\n");
        }
    }

For really complicated logging you might abstract out
the middle bit into functions which call qemu_log()
directly and which are only called inside a check that
some particular log level is enabled.

The uses in tcg/tcg.c are examples of this pattern.

The thing to avoid is a plain qemu_log() call which is
not already guarded by some check on the log-level mask.
You're right that the really common case is fine with
just qemu_log_mask(), but sometimes you need to be
able to split up the "is log level X enabled" and
"log" parts of the task.

thanks
-- PMM