Re: [PATCH 12/12] error: Kill @error_warn

[Markus Armbruster <armbru@redhat.com>]

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

> On Fri, Aug 08, 2025 at 10:08:23AM +0200, Markus Armbruster wrote:
>> We added @error_warn some two years ago in commit 3ffef1a55ca (error:
>> add global &error_warn destination).  It has multiple issues:
>> 
>> * error.h's big comment was not updated for it.
>> 
>> * Function contracts were not updated for it.
>> 
>> * ERRP_GUARD() is unaware of @error_warn, and fails to mask it from
>>   error_prepend() and such.  These crash on @error_warn, as pointed
>>   out by Akihiko Odaki.
>> 
>> All fixable.  However, after more than two years, we had just of 15
>> uses, of which the last few patches removed eight as unclean or
>> otherwise undesirable.  I didn't look closely enough at the remaining
>> seven to decide whether they are desirable or not.
>> 
>> I don't think this feature earns its keep.  Drop it.
>> 
>> Thanks-to: Akihiko  Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/qapi/error.h           |  6 ------
>>  hw/display/virtio-gpu.c        |  8 ++++++--
>>  hw/net/virtio-net.c            |  8 +++++++-
>>  tests/unit/test-error-report.c | 17 -----------------
>>  ui/gtk.c                       |  6 +++++-
>>  util/error.c                   |  5 +----
>>  6 files changed, 19 insertions(+), 31 deletions(-)
>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 6b5b5dace3..7848e26278 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -1289,6 +1289,8 @@ exit:
>>  
>>  static bool virtio_net_load_ebpf(VirtIONet *n, Error **errp)
>>  {
>> +    Error *err = NULL;
>> +
>>      if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
>>          return true;
>>      }
>> @@ -1306,7 +1308,11 @@ static bool virtio_net_load_ebpf(VirtIONet *n, Error **errp)
>>          return virtio_net_load_ebpf_fds(n, errp);
>>      }
>>  
>> -    ebpf_rss_load(&n->ebpf_rss, &error_warn);
>> +    ebpf_rss_load(&n->ebpf_rss, &err);
>> +    /* Beware, ebpf_rss_load() can return false with @err unset */
>
> Per our other mail, this is a bug we should fix

Yes, but I still don't know how to fix it.  The remaining open question
is "Is it a programming error when it happens?" in
Message-ID: <87sehfsife.fsf@pond.sub.org>

Help!

>> +    if (err) {
>> +        warn_report_err(err);
>> +    }
>>      return true;
>>  }
>>  
>
> With regards,
> Daniel