From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v5 24/24] xl: vNUMA support Date: Tue, 24 Feb 2015 16:19:02 +0000 Message-ID: <1424794739.4742.118.camel@citrix.com> References: <1423770294-9779-1-git-send-email-wei.liu2@citrix.com> <1423770294-9779-25-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3669569402003614415==" Return-path: In-Reply-To: <1423770294-9779-25-git-send-email-wei.liu2@citrix.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: "JBeulich@suse.com" , Andrew Cooper , "xen-devel@lists.xen.org" , "ufimtseva@gmail.com" , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org --===============3669569402003614415== Content-Language: en-US Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-Z+nmpT8+7AGFqew7aJo+" --=-Z+nmpT8+7AGFqew7aJo+ Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2015-02-12 at 19:44 +0000, Wei Liu wrote: > This patch includes configuration options parser and documentation. >=20 > Please find the hunk to xl.cfg.pod.5 for more information. >=20 > Signed-off-by: Wei Liu > Cc: Ian Campbell > Cc: Ian Jackson > This all looks pretty good to me. I only have one comment and a question. > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index ec7fb2d..f52daf9 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c [...] > + if (!strcmp("pnode", option)) { > + val =3D strtoul(value, &endptr, 10); > + ABORT_IF_FAILED(value); > + if (val >=3D nr_nodes) { > + fprintf(stderr, > + "xl: invalid pnode number: %lu\n", val); > + exit(1); > + } > + p->pnode =3D val; > This is, to all the effects, a form of placement so, if this part of vNUMA specification is present, you should disable the automatic placement happening in libxl. This is all it takes to do so (look inside parse_vcpu_affinity() if you need more insights): libxl_defbool_set(&b_info->numa_placement, false); > + } else if (!strcmp("size", option)) { > + val =3D strtoul(value, &endptr, 10); > + ABORT_IF_FAILED(value); > + p->memkb =3D val << 10; > + } else if (!strcmp("vcpus", option)) { > + libxl_string_list cpu_spec_list; > + int cpu; > + unsigned long s, e; > + > + split_string_into_string_list(value, ",", &cpu_spec_= list); > + len =3D libxl_string_list_length(&cpu_spec_list); > + > + for (j =3D 0; j < len; j++) { > + parse_range(cpu_spec_list[j], &s, &e); > + for (cpu =3D s; cpu <=3De; cpu++) > + libxl_bitmap_set(&p->vcpus, cpu); > + } > + libxl_string_list_dispose(&cpu_spec_list); > I think that using vcpupin_parse() for "vcpus=3D" would allow for more flexible syntax (i.e., things like "3-8,^5"), and save some code. The only downside is that it also accepts things like "nodes:1", which we clearly don't want in here... is that why you are not going for it? If you decide to use it, BTW, you may want to change its name (again!) Regards, Dario --=-Z+nmpT8+7AGFqew7aJo+ 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 iEYEABECAAYFAlTspHMACgkQk4XaBE3IOsSGvQCgrRVaVL50h1UOq0J+xuqCqr0Q efYAnA4ugdNNUSi8cz3pR4YM6B5V1r++ =5HUh -----END PGP SIGNATURE----- --=-Z+nmpT8+7AGFqew7aJo+-- --===============3669569402003614415== 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 --===============3669569402003614415==--