| Message ID | 9cc56d01a09fcf6e1e3e9d48e60420f435fa34c3.1670348481.git.per.bilse@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ioreq_broadcast(): accept partial broadcast success | expand |
On 06/12/2022 17:52, Per Bilse wrote: > Avoid incorrectly triggering an error when a broadcast buffered ioreq > is not handled by all registered clients, as long as the failure is > strictly because the client doesn't handle buffered ioreqs. > > Signed-off-by: Per Bilse <per.bilse@citrix.com> > --- > v2: Complete rethink with better information. A lot of simplicity was added. > --- Reviewed-by: Paul Durrant <paul@xen.org>
On 06.12.2022 18:52, Per Bilse wrote: > --- a/xen/common/ioreq.c > +++ b/xen/common/ioreq.c > @@ -1317,7 +1317,8 @@ unsigned int ioreq_broadcast(ioreq_t *p, bool buffered) > > FOR_EACH_IOREQ_SERVER(d, id, s) > { > - if ( !s->enabled ) > + if ( !s->enabled || (buffered && > + s->bufioreq_handling == HVM_IOREQSRV_BUFIOREQ_OFF) ) Nit: Bad indentation. Since inserting the missing blanks would make the line too long, the expression wants re-wrapping. Can certainly be done while committing. Jan
(Cc to xen-devel@ re-added; I don't see why it was dropped) On 07.12.2022 11:25, Per Bilse (3P) wrote: > On 07/12/2022 09:05, Jan Beulich wrote: >> On 06.12.2022 18:52, Per Bilse wrote: >>> + if ( !s->enabled || (buffered && >>> + s->bufioreq_handling == HVM_IOREQSRV_BUFIOREQ_OFF) ) >> >> Nit: Bad indentation. Since inserting the missing blanks would make >> the line too long, the expression wants re-wrapping. Can certainly >> be done while committing. > > Thanks for the feedback, but what should the indentation be (and where > would I find the information)? I checked the coding style and it simply > says "Long line should be split at sensible places and the trailing > portions indented." You'll find ample examples throughout the tree. Indentation of pieces of expressions should be such that related pieces align. Hence in the case here it could be if ( !s->enabled || (buffered && s->bufioreq_handling == HVM_IOREQSRV_BUFIOREQ_OFF) ) but since that's too long a line it'll want to be e.g. if ( !s->enabled || (buffered && s->bufioreq_handling == HVM_IOREQSRV_BUFIOREQ_OFF) ) or if ( !s->enabled || (buffered && s->bufioreq_handling == HVM_IOREQSRV_BUFIOREQ_OFF) ) (of which I prefer the former, and that's what I'll convert to if I end up committing this change). Jan
diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c index 4617aef29b..568e7aea91 100644 --- a/xen/common/ioreq.c +++ b/xen/common/ioreq.c @@ -1317,7 +1317,8 @@ unsigned int ioreq_broadcast(ioreq_t *p, bool buffered) FOR_EACH_IOREQ_SERVER(d, id, s) { - if ( !s->enabled ) + if ( !s->enabled || (buffered && + s->bufioreq_handling == HVM_IOREQSRV_BUFIOREQ_OFF) ) continue; if ( ioreq_send(s, p, buffered) == IOREQ_STATUS_UNHANDLED )
Avoid incorrectly triggering an error when a broadcast buffered ioreq is not handled by all registered clients, as long as the failure is strictly because the client doesn't handle buffered ioreqs. Signed-off-by: Per Bilse <per.bilse@citrix.com> --- v2: Complete rethink with better information. A lot of simplicity was added. --- xen/common/ioreq.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)