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;
+ }
+}
+
void error_reportf_err(Error *err, const char *fmt, ...)
{
va_list ap;
--
2.48.1
On Thu, 30 Jan 2025 14:43:38 +0100
Cédric Le Goater <clg@redhat.com> wrote:
> 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);
> +
Turning it into a macro would do what you want:
#define warn_report_once_err(err) ({ \
static bool print_once_; \
if (!print_once_) { \
warn_report_err(err); \
print_once_ = true; \
} \
})
So long as we only want once per call site and not once per object,
which would pull in something like warn_report_once_cond(). Thanks,
Alex
> /*
> * 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;
> + }
> +}
> +
> void error_reportf_err(Error *err, const char *fmt, ...)
> {
> va_list ap;
On 1/30/25 18:55, Alex Williamson wrote:
> On Thu, 30 Jan 2025 14:43:38 +0100
> Cédric Le Goater <clg@redhat.com> wrote:
>
>> 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);
>> +
>
> Turning it into a macro would do what you want:
>
> #define warn_report_once_err(err) ({ \
> static bool print_once_; \
> if (!print_once_) { \
> warn_report_err(err); \
> print_once_ = true; \
> } \
> })
>
> So long as we only want once per call site and not once per object,
> which would pull in something like warn_report_once_cond(). Thanks,
yeah. I came up with this :
/*
* TODO: move to util/
*/
static bool warn_report_once_err_cond(bool *printed, Error *err)
{
if (*printed) {
error_free(err);
return false;
}
*printed = true;
warn_report_err(err);
return true;
}
#define warn_report_once_err(err) \
({ \
static bool print_once_; \
warn_report_once_err_cond(&print_once_, err); \
})
I don't know where to put it though. It sits in between qapi/error.h
and qemu/error-report.h.
Thanks,
C.
> Alex
>
>> /*
>> * 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;
>> + }
>> +}
>> +
>> void error_reportf_err(Error *err, const char *fmt, ...)
>> {
>> va_list ap;
>
Cédric Le Goater <clg@redhat.com> writes:
> On 1/30/25 18:55, Alex Williamson wrote:
>> On Thu, 30 Jan 2025 14:43:38 +0100
>> Cédric Le Goater <clg@redhat.com> wrote:
>>
>>> 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);
>>> +
>> Turning it into a macro would do what you want:
>> #define warn_report_once_err(err) ({ \
>> static bool print_once_; \
>> if (!print_once_) { \
>> warn_report_err(err); \
>> print_once_ = true; \
>> } \
>> })
>> So long as we only want once per call site and not once per object,
>> which would pull in something like warn_report_once_cond(). Thanks,
>
> yeah. I came up with this :
>
> /*
> * TODO: move to util/
> */
> static bool warn_report_once_err_cond(bool *printed, Error *err)
> {
> if (*printed) {
> error_free(err);
> return false;
> }
> *printed = true;
> warn_report_err(err);
> return true;
> }
>
> #define warn_report_once_err(err) \
> ({ \
> static bool print_once_; \
> warn_report_once_err_cond(&print_once_, err); \
> })
>
>
> I don't know where to put it though. It sits in between qapi/error.h
> and qemu/error-report.h.
Stuff involving the Error type should not go into qemu/error-report.h.
Precedence: warn_report_err() & friends are in qapi/error.h even though
they're straightforward wrappers around warn_report() & friends for easy
use with Error.
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?
The related function warn_report_once_cond() takes the flag as a
parameter. Only the calls using the same flag suppress each other.
> +
> void error_reportf_err(Error *err, const char *fmt, ...)
> {
> va_list ap;
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.
>> 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. But vfio uses ->hint too which adds complexity. I will keep the custom version I have in vfio for now. Thanks, C.
© 2016 - 2025 Red Hat, Inc.