From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 10/10] xl: allow to set the ratelimit value online for Credit2 Date: Fri, 30 Sep 2016 17:54:21 +0200 Message-ID: <1475250861.5315.140.camel@citrix.com> References: <147520253247.22544.10673844222866363947.stgit@Solace.fritz.box> <147520406887.22544.17860143561838604699.stgit@Solace.fritz.box> <22510.16332.254523.761881@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2761240102876513719==" Return-path: Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bq08s-0003m4-0F for xen-devel@lists.xenproject.org; Fri, 30 Sep 2016 15:54:30 +0000 In-Reply-To: <22510.16332.254523.761881@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Ian Jackson Cc: xen-devel@lists.xenproject.org, Wei Liu , George Dunlap List-Id: xen-devel@lists.xenproject.org --===============2761240102876513719== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-XZIM2uMeKYZp9UZejsJS" --=-XZIM2uMeKYZp9UZejsJS Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2016-09-30 at 11:34 +0100, Ian Jackson wrote: > Dario Faggioli writes ("[PATCH v2 10/10] xl: allow to set > theratelimit value online for Credit2"): > > > > +static int sched_credit2_params_set(int poolid, > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0lib= xl_sched_credit2_params > > *scinfo) >=20 > Each of this and the corresponding _get calls the corresponding libxl > function, prints a message, and adjusts the return value. >=20 > But: AFAICT each of these has only one call site; and each of the > underlying libxl functions logs an error too; and each of the call > sites prints a message too. >=20 > So I question whether these functions are valuable. >=20 So, about this last point (utility of sched_*_params_{get,set}() ), I totally agree. I think the reason why the functions exist is for the most part related to convenience and style. E.g., maybe indentation/nesting level was getting too deep within the various main_sched_*(). As said in another email, I don't especially like those functions, and I have the feeling they're a better way to write them, but I haven't really try. I'll put having a look at that in my todo list, but not at super high priority. :-P > At this stage of the release cycle, and the maturity of this series, > I > guess you probably don't want to start messing with the error paths. > Yeah, indeed, thanks for understanding! :-P Although... > The messages that will come out will be overly verbose rather than > inadequate, so the code as it is fine if not as good as it could be. >=20 ...I'm not entirely sure I get what you're saying here. I mean, AFAICT, there's xl calling a libxl function and printing an error if it fails. The libxl function also logs an error --true-- but this seems rather common to me, and apart from that, it looks reasonable as well, isn't it so? Avoiding double/overly verbose logging from either within xl or within libxl is something I totally understand and agree. But when the 'border' is crossed, is it that terrible that both do log? After all, how can xl be sure that libxl will log something? And even if he could, I see the two as different kind and different level of logging. Or was it something else that you were referring to? > But it was a thing I noticed which I wanted to write down and/or have > your opinion on. >=20 Sure! :-) Thanks and Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-XZIM2uMeKYZp9UZejsJS Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJX7oqtAAoJEBZCeImluHPuPDkQAOO4CPhaN9haCOmeqwd4CmWG /JnFN1cxXt2udOMSBPdE9bXrla6cH9Nzh2H3PK6MNVXhOAJN40COx4NRjRjEufrE Ka9yP0sUC1mAQrmJkHhipNMdt+uzQ8OU++jMGit98KDtvz1X1+nohdiBdGxYzW/M UOAnkTFh255/QDo36v9hUcdnI0Ei9u/OYeKmrRI5rJVif0AO3t6RF97qjkqeYTkq UajPjNM8jdMj7uOZhWUNmWzM9GKo10RvBz6OeCCUotlxFDjvCwmh7FyxUECJZ65P WuFv1IcCBSELjhpAOT6+XF7SBmAFIXV9BPzE9c/qrTOSuz6AQKSajr3Gjkw2wM6p 7mXastzt0UlGPdhpoqd2h5yoqOrDGYFdfSVh3zy4GBxpEjqYBmLZFiSQ9tqIUZJc fxGnlan8yRJYrMilD2VAhvRYAMU5J/s+2X2d9AAa9LcKYyZbcw/eTRLXSkhqh6CP U6qfBWQi+2nwnrKGS+QAt7v/20zlHeqQt3fnyqTajdf5OfLU99i66SeMTi2g0lBm TY56cRZx9fwPjmN4Esccmu1N5Zufm0WLGvXyhBRCLifzO+GPXltgH3m5tR2m3NCd qsM3JrEMj41EJ9Xas0OzELEAoL/OzoKFTqVmmeMiwugXSrdRinX7s7PbLPz3DjYf 5gzWz65k8+lvBzFmfNOM =2/Kv -----END PGP SIGNATURE----- --=-XZIM2uMeKYZp9UZejsJS-- --===============2761240102876513719== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============2761240102876513719==--