Daniel P. Berrangé <[email protected]> writes:
> On Mon, Sep 15, 2025 at 09:14:18PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> On 15.09.25 18:45, Peter Xu wrote:
>> > On Mon, Sep 15, 2025 at 04:22:01PM +0300, Vladimir Sementsov-Ogievskiy
>> > wrote:
>> > > Add a pair to &error_warn helper, to reduce the pattern like
>> > >
>> > > Error *local_err = NULL;
>> > >
>> > > ...
>> > >
>> > > if (!foo(..., &local_err)) {
>> > > error_report_err(local_err);
>> > > return false;
>> > > }
>> > >
>> > > to simply
>> > >
>> > > if (!foo(..., &error_reporter)) {
>> > > return false;
>> > > }
>> > >
>> > > Of course, for new interfaces, it's better to always support and handle
>> > > errp argument. But when have to rework the old ones, it's not always
>> > > feasible to convert everything to support/handle errp.
>> > >
>> > > The new helper is used in following commits.
>> >
>> > Could we add some explanation of why we need this if we already have
>> > error_warn?
>> >
>> > I don't see much difference except error_warn will prepend it with
>> > "warning: ", but that doesn't sound like a real problem..
>>
>> Yes, seems it's the only difference.
>>
>> For me it seems strange to call it "warning", when we actually go to failure
>> branch of the code logic.
>> Finally, we do have error_report() and warn_report(). Seems not a problem to
>> have corresponding "magic" variables.
>>
>> If have only one special error variable to simply report the error, I'd
>> prefer it not have "warning: " prefix.
>> I.e. drop error_warn, and keep only error_reporter (or some better name?).
>
> FWIW, this whole debate is liable to be a bit of a can of worms that
> will delay your series from getting merged.
>
> Can I suggest you repost this without &error_reporter usage, and then
> have a separate standalone series that proposes error_report, and
> converts a handful of files to demonstrate its usage.
Seconded.
Please note the killing of &error_warn is in progress:
Subject: [PATCH v2 00/12] Error reporting cleanup, a fix, and &error_warn
removal
Date: Wed, 17 Sep 2025 13:51:55 +0200
Message-ID: <[email protected]>
https://lore.kernel.org/qemu-devel/[email protected]/
Rationale in PATCH 12.
I doubt additional special error destinations are a good idea.
&error_abort definitely is a good idea. &error_fatal is problematic,
but widely used.
Akihiko Odaki <[email protected]> suggested to add one that
at a glance looks just like yours. Please see my reply at
Message-ID: <[email protected]>
https://lore.kernel.org/qemu-devel/[email protected]/
