[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Mon, 28 Nov 2022 12:06:53 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=NYMcg9OfEUIYPH64Wy/RlaqryI1n+V9p+oLi3+s4AQg=; b=c4z0BIjyedZfqb8ck8aMwJWw6/1CUKUQUahgnKjxjq2kAwxka/pP0W6w/Q/iZqhQJKn5GUbBMQHnorrJtZSSbiBv/GaYH0jAboskXiicob27Gm3uk53rJH94bKgQYINNfcoiXzxhDJlsZZiFqlpKPjiOqXZbQfNZ6DOl8LGhXeYcpdxgKREVvhDCePDg2aGu7W9nMik1IypoR0LU46lh4Kmw0GpZQawAhVbSZ9ArBEFjC+NN3CC5+YEui/mA1XTak6w9innYyfU0SVLWuniVOLnGU08qHIrFsJzX3Xm6aUUgP4I1hWYTne9XYgkUuodlv8exXwSnCQ6PVwCROIgDkg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hZwevQZRQ8Kb+Dq0wQAxTP61h0Dq5Zq1UknEkIqxkkMjV+QGKiJgRPcM4azoXvpyqC+8TIb+bVZZ0jqMY0eY8qek9BXkhBVXGLKHfjBVyQRLxSHNmd6kJfGCAZv0b73jLzDS7kNWDVkSaGI2yCuo5T7fZCoaRgStgmE9M1LytR1YZ2hvflolzhCbWwkEb6nQ0yUoCxm5gl4fJ3EnUP2KZuN83kWMVSvK/ee32TDxtzRsNqEi6a55Una71i+dxVlzN+TlH1iCKTq/TMGEVz6r0D2DbCr+Vi1cTlMRAKaXqLq2IkBEMAH9oJUuF6zEwKJWJi1vsHX5p4vzxMoegJhMyQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Per Bilse <per.bilse@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Julien Grall <julien@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 28 Nov 2022 11:07:31 +0000
  • Ironport-data: A9a23:rqZHa6xxlSQfwQYhT6t6t+fyxyrEfRIJ4+MujC+fZmUNrF6WrkUHy mpNUWyGP62CZzajLtx0YI2w/BsHu8DUz9VrSFY//yAxQypGp/SeCIXCJC8cHc8wwu7rFxs7s ppEOrEsCOhuExcwcz/0auCJQUFUjP3OHfykTbaeYUidfCc8IA85kxVvhuUltYBhhNm9Emult Mj75sbSIzdJ4RYtWo4vw//F+U0HUMja4mtC5AVnP6AT5TcyqlFOZH4hDfDpR5fHatE88t6SH 47r0Ly/92XFyBYhYvvNfmHTKxBirhb6ZGBiu1IOM0SQqkEqSh8ai87XAME0e0ZP4whlqvgqo Dl7WT5cfi9yVkHEsLx1vxC1iEiSN4UekFPMCSDXXcB+UyQq2pYjqhljJBheAGEWxgp4KVxO6 cUjGTEuUg2OnfqS0bGAYOddjNt2eaEHPKtH0p1h5RfwKK98BLzmHeDN79Ie2yosjMdTG/qYf 9AedTdkcBXHZVtIJ0sTD5U92uyvgxETcRUB8A7T+fVxvjmVlVIhuFTuGIO9ltiiX8Jak1zev mvb12/4HgsbJJqUzj/tHneE1rKVxnmmA9J6+LuQysN2gHedl1UoEQA3CGKRrP6BgGOAcocKQ 6AT0m90xUQoz2SpRNTgWxyzoFafowURHdFXFoUS8AiQzoLE7gDfAXILJhZebPQ2uclwQiYlv neLgtfoCDpHoLCTD3WH+d+8vT60fCQYM2IGTSsFVhcepcnuppkpiRDCRcolF7S65uAZAhn1y jGO6SQ72bMaiJdR073hpA6XxTWxupLOUwg5oB3NWX6o5R94Y4jjYJG07V/c7rBLK4PxokS9g UXoUvO2tIgmZaxhXgTdKAnRNNlFP8q4DQA=
  • Ironport-hdrordr: A9a23:dTQBvKElrfbe9l+qpLqFgpLXdLJyesId70hD6qkvc3Fom52j/f xGws5x6faVslkssb8b6LW90Y27MAvhHP9OkPAs1NKZMDUO11HJEGgP1/qA/9SkIVyEygc/79 YdT0EdMqyWMbESt6+TjmiF+pQbsb+6GciT9JrjJhxWPGVXgs9bnmVE4lHxKDwNeOAKP+tPKL Osou584xawc3Ueacq2QlEDQuj4vtXO0L72fBIcABYjyQ+WyRel8qTzHRS01goXF2on+8ZozU H11yjCoomzufCyzRHRk0fV8pRtgdPkjvdTGcCWjcARCzP0ziKlfp5oVbGutC085Muv9FEput /RpApIBbU611rhOkWO5Tf90Qjp1zgjr1fk1F+jmHPm5ej0XigzBcZtjZ9QNkKx0TtogPhMlI Zwm06JvZteCh3N2Az7+tjzThlv0m65u2Arn+I/h2FWFaEedLhSh4oC+149KuZ3IAvKrKQcVM V+BsDV4/hbNXuccnDip2FqhOehW3widy32MHQqi4iw6Xx7jXp5x0wXyIg0hXEb7q8wTJFC+q DtLrlovKsmdL5YUYtNQMM6BeenAG3ERhzBdEiIJ078Ka0BM3XR77bq/bQO4v2wcpBg9up/pH 34aiIYiYcOQTOvNSXXt6c7sSwlAV/NEAgF8/suqaSQ4dbHNfjW2S7qciFcryLvmYRbPiThYY fDBHtnOY6dEYLQI/c24+TfYegmFZBMarxghv8LH3Szn+nsFqrG8sTmTde7HsucLd9jYBK0Pk c+
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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:
> >> A change to XAPI varstored causes
> > 
> > For those unfamiliar with XAPI (like me), can you explain what was the 
> > change made?

One ioreq client used by XenServer doesn't handle buffered ioreqs, so
the broadcasted TIMEOFFSET always triggers an error due to that ioreq
not handling it.  While not harmful, it's still annoying to get the
messages on the hypervisor console for something that's not really an
error.

The description can indeed be reworded to not mention XenServer
specific components, something like:

"Avoid printing an error message 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."

> >> 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".

So introduce an IOREQ_STATUS_UNREGISTERED return code and don't
increase failed if buffered == true and UNREGISTERED is returned,
would that be acceptable?

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.