From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH RESEND 02/12] xl: allow for node-wise specification of vcpu pinning Date: Fri, 8 Nov 2013 10:24:09 +0100 Message-ID: <1383902649.25387.15.camel@Solace> References: <20131105142844.30446.78671.stgit@Solace> <20131105143424.30446.56692.stgit@Solace> <21115.55622.157240.413537@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2164837866441246236==" Return-path: In-Reply-To: <21115.55622.157240.413537@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: Marcus Granado , Keir Fraser , Ian Campbell , Li Yechen , George Dunlap , Andrew Cooper , Juergen Gross , xen-devel@lists.xen.org, Jan Beulich , Justin Weaver , Daniel De Graaf , Matt Wilson , Elena Ufimtseva List-Id: xen-devel@lists.xenproject.org --===============2164837866441246236== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-jubrhw8hQPPSgdJLRDWM" --=-jubrhw8hQPPSgdJLRDWM Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On gio, 2013-11-07 at 18:17 +0000, Ian Jackson wrote: > Dario Faggioli writes ("[PATCH RESEND 02/12] xl: allow for node-wise spec= ification of vcpu pinning"): > > Making it possible to use something like the following: > > * "nodes:0-3": all pCPUs of nodes 0,1,2,3; > > * "nodes:0-3,^node:2": all pCPUS of nodes 0,1,3; > > * "1,nodes:1-2,^6": pCPU 1 plus all pCPUs of nodes 1,2 > > but not pCPU 6; > > * ... >=20 > Thanks. This parsing is a lot clearer now. >=20 Good to hear that, again (yep, you said this before in a previous review of a different series still including this patch :-D). > > @@ -59,6 +59,11 @@ > > } = \ > > }) > > =20 > > +#define STR_HAS_PREFIX( a, b ) \ > > + ( strncmp(a, b, strlen(b)) =3D=3D 0 ) > > +#define STR_SKIP_PREFIX( a, b ) \ > > + ( STR_HAS_PREFIX(a, b) ? (a) +=3D strlen(b) : NULL ) >=20 > I think it might be worth making the type of STR_SKIP_PREFIX be > explicitly boolean. Eg, > + ( STR_HAS_PREFIX(a, b) ? ((a) +=3D strlen(b), 1) : 0 ) >=20 > Since the returned pointer value isn't very helpful. >=20 Good point. Will do. > > -static int vcpupin_parse(char *cpu, libxl_bitmap *cpumap) > > +static int parse_range(const char *str, unsigned long *a, unsigned lon= g *b) > > +{ > > + char *nstr, *endptr; >=20 > Missing consts ? >=20 Is it? For instance, I'm using endptr like this: *a =3D *b =3D strtoul(str, &endptr, 10); And the prototype of strtoul is: unsigned long int strtoul(const char *nptr, char **endptr, int base); So it won't work, not for endptr at least. What I can do is the following: const char *nstr; char *endptr; And I will, if you think it's better. > > + if (STR_HAS_PREFIX(str, "all")) { > > libxl_bitmap_set_any(cpumap); > > - return 0; > > + goto out; >=20 > I think this does the wrong thing with "^all". >=20 Good point again. I'll fix that. > > + for (ptr =3D strtok_r(cpu, ",", &saveptr); ptr; > > + ptr =3D strtok_r(NULL, ",", &saveptr)) { >=20 > A minor style complaint: If you are going to split two of these three > items onto their own line, please give them all their own line. >=20 You mean ptr should have its own line? Like this? + for (ptr =3D strtok_r(cpu, ",", &saveptr); + ptr; + ptr =3D strtok_r(NULL, ",", &saveptr)) { If yes, I sure can do that, although the result really looks super unpleasant to me (but that's a matter of taste, I guess). 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) --=-jubrhw8hQPPSgdJLRDWM 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 v1.4.15 (GNU/Linux) iEYEABECAAYFAlJ8rbkACgkQk4XaBE3IOsQ8WACfV9LwFQIn66D1lN0aOUTDNYbC z2gAoKNio5fqhlWsbwTxMo1vu4gAiAZc =8UF6 -----END PGP SIGNATURE----- --=-jubrhw8hQPPSgdJLRDWM-- --===============2164837866441246236== 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 --===============2164837866441246236==--