From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v4 for Xen 4.6 4/4] xl: enable per-VCPU parameter settings for RTDS scheduler Date: Tue, 28 Jul 2015 11:25:14 +0200 Message-ID: <1438075514.2889.33.camel@citrix.com> References: <1436590356-3706-1-git-send-email-chong.li@wustl.edu> <1436590356-3706-5-git-send-email-chong.li@wustl.edu> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3723423001337439760==" Return-path: In-Reply-To: <1436590356-3706-5-git-send-email-chong.li@wustl.edu> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Chong Li Cc: Chong Li , wei.liu2@citrix.com, Sisu Xi , george.dunlap@eu.citrix.com, xen-devel@lists.xen.org, Meng Xu , dgolomb@seas.upenn.edu List-Id: xen-devel@lists.xenproject.org --===============3723423001337439760== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-8zQXQz6XR9jp6MJxTZj3" --=-8zQXQz6XR9jp6MJxTZj3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2015-07-10 at 23:52 -0500, Chong Li wrote: > Change main_sched_rtds and related output functions to support > per-VCPU settings. >=20 This patch also looks nice. A few comments provided inline. > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index c858068..da7c8a6 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -5630,6 +5630,37 @@ static int sched_domain_set(int domid, const libxl= _domain_sched_params *scinfo) > return rc; > } > =20 > +static int sched_vcpu_get(libxl_scheduler sched, int domid, > + libxl_vcpu_sched_params *scinfo) > +{ > + int rc; > + > + rc =3D libxl_vcpu_sched_params_get(ctx, domid, scinfo); > + if (rc) { > + fprintf(stderr, "libxl_vcpu_sched_params_get failed.\n"); > + return rc; > + } > + if (scinfo->sched !=3D sched) { > + fprintf(stderr, "libxl_vcpu_sched_params_get returned %s not %s.= \n", > + libxl_scheduler_to_string(scinfo->sched), > + libxl_scheduler_to_string(sched)); > + return ERROR_INVAL; > We are in xl, so there's no need to use libxl error code. It's probably not strictly forbidden either, and xl itself is really inconsistent about this, I know. Personally, I never use them, and I think that using them may be confusing, and create even more inconsistency. > +static int sched_vcpu_set(int domid, const libxl_vcpu_sched_params *scin= fo) > +{ > + int rc; > + > + rc =3D libxl_vcpu_sched_params_set(ctx, domid, scinfo); > + if (rc) > + fprintf(stderr, "libxl_vcpu_sched_params_set failed.\n"); > + > + return rc; > In xl, when failing, most of the time we exit(-1), or something like that. So, it's important that you log what's happening, but then there's no need to save the return codes and propagate them. > static int sched_credit_params_set(int poolid, libxl_sched_credit_params= *scinfo) > { > int rc; > @@ -5786,6 +5817,38 @@ out: > return rc; > } > =20 > +static int sched_rtds_vcpu_output( > + int domid, libxl_vcpu_sched_params *scinfo) > +{ > + char *domname; > + int rc =3D 0; > + int i; > + > + if (domid < 0) { > Style (indentation). This is not the only instance of something like this in this patch. > + printf("%-33s %4s %4s %9s %9s\n", "Name", "ID", > + "VCPU", "Period", "Budget"); > + return 0; > + } > + > + rc =3D sched_vcpu_get(LIBXL_SCHEDULER_RTDS, domid, scinfo); > + if (rc) > + goto out; > + > + domname =3D libxl_domid_to_name(ctx, domid); > + for( i =3D 0; i < scinfo->num_vcpus; i++ ) { > Style: for (i =3D 0; i < scinfo->num_vcpus; i++) { and indentation, I think. > + printf("%-33s %4d %4d %9"PRIu32" %9"PRIu32"\n", > + domname, > + domid, > + scinfo->vcpus[i].vcpuid, > + scinfo->vcpus[i].period, > + scinfo->vcpus[i].budget); > + } > + free(domname); > + > +out: > + return rc; > +} > + Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-8zQXQz6XR9jp6MJxTZj3 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 iEYEABECAAYFAlW3SnoACgkQk4XaBE3IOsTj3QCgh7zwKxW5VduwoAojWW1LxD1C VEUAnRiiGVW1PDEgudJ9fPSnXtptN5WG =7dCS -----END PGP SIGNATURE----- --=-8zQXQz6XR9jp6MJxTZj3-- --===============3723423001337439760== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============3723423001337439760==--