Re: [PATCH] ioreq_broadcast(): accept partial broadcast success

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.11.2022 23:19, Julien Grall wrote:
> On 25/11/2022 14:15, Per Bilse wrote:
>> A change to XAPI varstored causes
> 
> For those unfamiliar with XAPI (like me), can you explain what was the 
> change made?
> 
>> an unnecessary error message
>> to be logged in hypervisor.log whenever an RTC timeoffset update
>> is broadcast.
>>  In extreme cases this could flood the log file.
> 
> Which should be ratelimited as this is using guest error loglevel. But I 
> think this is irrelevant here. It would be more relevant to explain why 
> it is OK to allow a partial broadcast.
> 
>> This patch modifies ioreq_broadcast() to allow partial success.
> 
> The commit message is quite vague, so it is hard to know what you are 
> trying to solve exactly. AFAIU, there are two reasons for 
> ioreq_broadcast to fails:
>   1) The IOREQ server didn't register the bufioreq
>   2) The IOREQ buffer page is full
> 
> While I would agree that the error message is not necessary for 1) (the 
> IOREQ server doesn't care about the event), I would disagree for 2) 
> because it would indicate something went horribly wrong in the IOREQ 
> server and there is a chance your domain may misbehave afterwards.

In addition I think ignoring failure (and, as said by Julien, only because
of no bufioreq being registered) is (kind of implicitly) valid only for
buffered requests. Hence I'm unconvinced of the need of a new boolean
function parameter. Instead I think we need a new IOREQ_STATUS_... value
representing the "not registered" case. And that could then be used by
ioreq_broadcast() to skip incrementing of "failed".

Jan