Re: [Qemu-devel] [PATCH v2 2/2] intel-iommu: start to use error_report_once

[Philippe Mathieu-Daudé]

On 05/22/2018 10:19 PM, Peter Xu wrote:
> On Tue, May 22, 2018 at 06:09:46PM -0300, Philippe Mathieu-Daudé wrote:
>> Hi Peter,
>>
>> On 05/22/2018 12:56 AM, Peter Xu wrote:
>>> Replace existing trace_vtd_err() with error_report_once() then stderr
>>> will capture something if any of the error happens, meanwhile we don't
>>> suffer from any DDOS.  Then remove the trace point.
>>>
>>> Signed-off-by: Peter Xu <address@hidden>
>>> ---
>>>  hw/i386/intel_iommu.c | 34 +++++++++++++++++-----------------
>>>  hw/i386/trace-events  |  1 -
>>>  2 files changed, 17 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>> index fb31de9416..cf655fb9f6 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -285,14 +285,14 @@ static void vtd_generate_fault_event(IntelIOMMUState 
>>> *s, uint32_t pre_fsts)
>>>  {
>>>      if (pre_fsts & VTD_FSTS_PPF || pre_fsts & VTD_FSTS_PFO ||
>>>          pre_fsts & VTD_FSTS_IQE) {
>>> -        trace_vtd_err("There are previous interrupt conditions "
>>> +        error_report_once("There are previous interrupt conditions "
>>>                        "to be serviced by software, fault event "
>>>                        "is not generated.");
>>
>> Can you keep alignment consistent please?
> 
> That was actually intended so reviewers don't need to skim those lines
> only caused by indents.  If I'm going to touch those lines, then maybe
> I can make them even look better by dumping more valid information.

I see.  I agree it is easier to review lines with substantial change,
however I now think it is then nicer to let the code exemplary for the
next one, if possible.  So regardless your preference:

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

Regards,

Phil.