On Fri, Mar 29, 2024 at 10:37 AM <no-re...@patchew.org> wrote:
> > 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.
Unfortunately, this is the reason why you _cannot_ do what this patch does.
For example:
error_reportf_err(local_err, "Disconnect client, due to: ");
error_report("terminating on signal %d", shutdown_signal);
This should not be prepending "error" - it's not an error.
error_report_once("%s: detected read error on DMAR slpte "
This is a guest error, so "error:" is probably not a good idea (it
should use qemu_log_mask).
And so on. :(
Paolo
> > 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....@dusky.pond.sub.org/#t
> >
> > Signed-off-by: Zhao Liu <zhao1....@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
>
