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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On 28/11/2022 12:26, Jan Beulich wrote:
> On 28.11.2022 12:06, Roger Pau Monné wrote:
> > On Mon, Nov 28, 2022 at 09:21:47AM +0100, Jan Beulich wrote:
> > > On 26.11.2022 23:19, Julien Grall wrote:
> > > > On 25/11/2022 14:15, Per Bilse wrote:
> > > > > 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".
> > So introduce an IOREQ_STATUS_UNREGISTERED return code and don't
> > increase failed if buffered == true and UNREGISTERED is returned,
> > would that be acceptable?
> Yes afaic, but Paul is the maintainer of this code. And of course the
> new error indicator shouldn't surprise any existing callers.
A new status code does indeed seem like the cleanest way forward.

  Paul