On 28/11/2022 08:21, Jan Beulich wrote: > On 26.11.2022 23:19, Julien Grall wrote: >> >> 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".
Hi guys, and thank you very much for the feedback. As I'm sure you've guessed I'm a newbie in Xen terms, so apologies for not getting things quite right. Varstored dropped support for buffered ioreqs, hence the persistent error message(s), and the proposed fix was derived from discussion in Citrix's hypervisor team. The 'partial' parameter could arguably be considered a case of (undesirable) special case handling, but ioreq_broadcast() is called from only two places in the code, so this seemed to be the lightest and simplest solution. I'll have to defer to more knowledgeable team members for further thoughts on this. Best, -- Per
