Re: [PATCH v2 1/9] util/error: Introduce warn_report_once_err()

["Cédric Le Goater" <clg@redhat.com>]

On 1/30/25 15:25, Markus Armbruster wrote:
> Cédric Le Goater <clg@redhat.com> writes:
> 
>> Depending on the configuration, a passthrough device may produce
>> recurring DMA mapping errors at runtime and produce a lot of
>> output. It is useful to report only once.
>>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   include/qapi/error.h | 5 +++++
>>   util/error.c         | 9 +++++++++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/include/qapi/error.h b/include/qapi/error.h
>> index 71f8fb2c50eee9a544992d0c05263c9793956fe1..b6ea274882b9788b64d4bb213c3458d7c674a881 100644
>> --- a/include/qapi/error.h
>> +++ b/include/qapi/error.h
>> @@ -448,6 +448,11 @@ void error_free_or_abort(Error **errp);
>>    */
>>   void warn_report_err(Error *err);
>>   
>> +/*
>> + * Convenience function to call warn_report_err() once.
>> + */
>> +void warn_report_once_err(Error *err);
>> +
>>   /*
>>    * Convenience function to error_report() and free @err.
>>    * The report includes hints added with error_append_hint().
>> diff --git a/util/error.c b/util/error.c
>> index e5e247209a9e0796074a9794f5598325f22f8d35..b8a211cccaa609a93091b86316144a0ad0a02662 100644
>> --- a/util/error.c
>> +++ b/util/error.c
>> @@ -247,6 +247,15 @@ void warn_report_err(Error *err)
>>       error_free(err);
>>   }
>>   
>> +void warn_report_once_err(Error *err)
>> +{
>> +        static bool print_once_;
>> +        if (!print_once_) {
>> +            warn_report_err(err);
>> +            print_once_ = true;
>> +        }
>> +}
> 
> Any particular reason for the trailing _ in @print_once_?>
> The first warning suppresses all subsequent warnings, even if they're
> completely different.  PATCH 5 uses this to emit a (rather cryptic)
> warning about unexpected mappings once.  If we later add another use
> elsewhere, these uses will suppress each other.  Is this what we want?

This is copy paste of a static routine that served one purpose in vfio.
I wanted to make it common and didn't think enough about the implication.
Sorry about that.

> The related function warn_report_once_cond() takes the flag as a
> parameter.  Only the calls using the same flag suppress each other.

yep. I wonder if we could use warn_report_once_cond() in a similar
macro.


Thanks,

C.


C.