Re: [RFC] util/error-report: Add "error: " prefix for error-level report

[Thomas Huth]

On 27/03/2024 12.46, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
>
> When vreport() was introduced, there was no prefix for error-level
> (REPORT_TYPE_ERROR) report. The original reason is "To maintain
> compatibility we don't add anything here" as Alistair said in his
> RFC v3 series [1].
>
> This was done in the context of inheriting the original error_report()
> interface without the prefix style. And it was also useful to have a
> means of error handling, such as exit(), when error occurs, so that the
> error message - the most serious level - can be noticed by the user.
>
> Nowadays, however, error_report() and its variants have a tendency to be
> "abused": it is used a lot just for the sake of logging something more
> noticeable than the "warn" or "info" level, in the absence of
> appropriate error handling logic.
>
> But, in the use case above, due to the lack of a prefix, it is in fact
> less informative to the user than warn_report()/info_report() (with
> "warn:" or "info: " prfix), which does not match its highest level.
>
> Therefore, add "error: " prefix for error-level report.
>
> [1]: https://lore.kernel.org/qemu-devel/87r2xuay5h.fsf@dusky.pond.sub.org/#t
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   util/error-report.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/util/error-report.c b/util/error-report.c
> index 6e44a5573217..e981c0b032f0 100644
> --- a/util/error-report.c
> +++ b/util/error-report.c
> @@ -213,6 +213,7 @@ static void vreport(report_type type, const char *fmt, 
> va_list ap)
>   
>       switch (type) {
>       case REPORT_TYPE_ERROR:
> +        error_printf("error: ");
>           break;
>       case REPORT_TYPE_WARNING:
>           error_printf("warning: ");

Sounds like a good idea to me, but I think you should then also remove
the hard-coded "error:" strings in the various error_reports():

$ grep -ri 'error_report.*"error:'
migration/block-dirty-bitmap.c:        error_report("Error: block device name is not 
set");
migration/block-dirty-bitmap.c:                error_report("Error: Unknown bitmap 
alias '%s' on node "
migration/block-dirty-bitmap.c:                error_report("Error: unknown dirty 
bitmap "
migration/block-dirty-bitmap.c:        error_report("Error: block device name is not 
set");
target/i386/kvm/kvm.c:        error_report("error: failed to set MSR 0x%" PRIx32 " 
to 0x%" PRIx64,
target/i386/kvm/kvm.c:        error_report("error: failed to get MSR 0x%" 
PRIx32,
util/vhost-user-server.c:        error_report("Error: too big message request: %d, 
"
accel/hvf/hvf-all.c:        error_report("Error: HV_ERROR");
accel/hvf/hvf-all.c:        error_report("Error: HV_BUSY");
accel/hvf/hvf-all.c:        error_report("Error: HV_BAD_ARGUMENT");
accel/hvf/hvf-all.c:        error_report("Error: HV_NO_RESOURCES");
accel/hvf/hvf-all.c:        error_report("Error: HV_NO_DEVICE");
accel/hvf/hvf-all.c:        error_report("Error: HV_UNSUPPORTED");
accel/hvf/hvf-all.c:        error_report("Error: HV_DENIED");
monitor/hmp-cmds.c:        error_reportf_err(err, "Error: ");
hw/arm/xlnx-zcu102.c:        error_report("ERROR: RAM size 0x%" PRIx64 " above max 
supported of "
hw/block/dataplane/xen-block.c:        error_report("error: unknown operation 
(%d)", request->req.operation);
hw/block/dataplane/xen-block.c:        error_report("error: write req for ro 
device");
hw/block/dataplane/xen-block.c:            error_report("error: nr_segments too 
big");
hw/block/dataplane/xen-block.c:            error_report("error: first > last 
sector");
hw/block/dataplane/xen-block.c:            error_report("error: page crossing");
hw/block/dataplane/xen-block.c:        error_report("error: access beyond end of 
file");
hw/rdma/rdma_backend.c:            rdma_error_report("Error: Not a MAD request, 
skipping");
hw/s390x/s390-skeys.c:        error_report("Error: Setting storage keys for pages 
with unallocated "
hw/s390x/s390-skeys.c:        error_report("Error: Getting storage keys for pages 
with unallocated "
hw/usb/bus.c:        error_report("Error: no usb bus to attach usbdevice %s, "
gdbstub/gdbstub.c:            error_report("Error: Bad gdb register numbering for 
'%s', "

 Thomas