From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v5 08/24] libxl: introduce libxl__vnuma_config_check Date: Tue, 17 Feb 2015 16:38:02 +0000 Message-ID: <1424191079.4235.88.camel@citrix.com> References: <1423770294-9779-1-git-send-email-wei.liu2@citrix.com> <1423770294-9779-9-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1383712305151545528==" Return-path: In-Reply-To: <1423770294-9779-9-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 --===============1383712305151545528== Content-Language: en-US Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-Ylbp5cyRFV/UpUwhVTgr" --=-Ylbp5cyRFV/UpUwhVTgr 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 function is used to check whether vNUMA configuration (be it > auto-generated or supplied by user) is valid. >=20 > Define a new error code ERROR_VNUMA_CONFIG_INVALID. >=20 > The checks performed can be found in the comment of the function. >=20 > This vNUMA function (and future ones) is placed in a new file called > libxl_vnuma.c >=20 I'm not sure whether having more files is a good or a bad thing. I would say that libxl_numa.c is rather small, and can certainly accept more code, if this kind of consolidation is desirable. If it were me doing this, I'd put things there, but I don't have a super strong opinion, and I'm also aware that I'm saying this pretty late, so I'll just state this as a preference, and leave it to you and the (other) maintainers. > Signed-off-by: Wei Liu > Cc: Ian Campbell > Cc: Ian Jackson > Cc: Dario Faggioli > Cc: Elena Ufimtseva > --- > Changes in v5: > 1. Define and use new error code. > 2. Use LOG macro. > 3. Fix hard tabs. >=20 > Changes in v4: > 1. Adapt to new interface. >=20 > Changes in v3: > 1. Rewrite commit log. > 2. Shorten two error messages. > --- > tools/libxl/Makefile | 2 +- > tools/libxl/libxl_internal.h | 7 +++ > tools/libxl/libxl_types.idl | 1 + > tools/libxl/libxl_vnuma.c | 131 +++++++++++++++++++++++++++++++++++++= ++++++ > 4 files changed, 140 insertions(+), 1 deletion(-) > create mode 100644 tools/libxl/libxl_vnuma.c >=20 > diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile > index 7329521..1b16598 100644 > --- a/tools/libxl/Makefile > +++ b/tools/libxl/Makefile > @@ -93,7 +93,7 @@ LIBXL_LIBS +=3D -lyajl > LIBXL_OBJS =3D flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o= \ > libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \ > libxl_internal.o libxl_utils.o libxl_uuid.o \ > - libxl_json.o libxl_aoutils.o libxl_numa.o \ > + libxl_json.o libxl_aoutils.o libxl_numa.o libxl_vnuma.o \ > libxl_save_callout.o _libxl_save_msgs_callout.o \ > libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y) > LIBXL_OBJS +=3D libxl_genid.o > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 6d3ac58..258be0d 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -3394,6 +3394,13 @@ void libxl__numa_candidate_put_nodemap(libxl__gc *= gc, > libxl_bitmap_copy(CTX, &cndt->nodemap, nodemap); > } > =20 > +/* Check if vNUMA config is valid. Returns 0 if valid, > + * ERROR_VNUMA_CONFIG_INVALID otherwise. > + */ > +int libxl__vnuma_config_check(libxl__gc *gc, > + const libxl_domain_build_info *b_info, > + const libxl__domain_build_state *state); > + > _hidden int libxl__ms_vm_genid_set(libxl__gc *gc, uint32_t domid, > const libxl_ms_vm_genid *id); > =20 > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 14c7e7c..23951fc 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -63,6 +63,7 @@ libxl_error =3D Enumeration("error", [ > (-17, "DEVICE_EXISTS"), > (-18, "REMUS_DEVOPS_DOES_NOT_MATCH"), > (-19, "REMUS_DEVICE_NOT_SUPPORTED"), > + (-20, "VNUMA_CONFIG_INVALID"), > ], value_namespace =3D "") > =20 > libxl_domain_type =3D Enumeration("domain_type", [ > diff --git a/tools/libxl/libxl_vnuma.c b/tools/libxl/libxl_vnuma.c > new file mode 100644 > index 0000000..fa5aa8d > --- /dev/null > +++ b/tools/libxl/libxl_vnuma.c > @@ -0,0 +1,131 @@ > +/* > + * Copyright (C) 2014 Citrix Ltd. > + * Author Wei Liu > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU Lesser General Public License as publis= hed > + * by the Free Software Foundation; version 2.1 only. with the special > + * exception on linking described in file LICENSE. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU Lesser General Public License for more details. > + */ > +#include "libxl_osdeps.h" /* must come before any other headers */ > +#include "libxl_internal.h" > +#include > + > +/* Sort vmemranges in ascending order with "start" */ > +static int compare_vmemrange(const void *a, const void *b) > +{ > + const xen_vmemrange_t *x =3D a, *y =3D b; > + if (x->start < y->start) > + return -1; > + if (x->start > y->start) > + return 1; > + return 0; > +} > + > +/* Check if vNUMA configuration is valid: > + * 1. all pnodes inside vnode_to_pnode array are valid > + * 2. one vcpu belongs to and only belongs to one vnode > something like "each vcpu belongs to one and only one vnode" sounds better here... > + * 3. each vmemrange is valid and doesn't overlap with each other > "doesn't overlap with any other one" perhaps (of course, I'm not a native speaker, so I can be very wrong! :-D) > + */ > +int libxl__vnuma_config_check(libxl__gc *gc, > + const libxl_domain_build_info *b_info, > + const libxl__domain_build_state *state) > +{ > + int i, j, rc =3D ERROR_VNUMA_CONFIG_INVALID, nr_nodes; > You should init nr_nodes to =3D 0 (see below). > + libxl_numainfo *ninfo =3D NULL; > + uint64_t total_memkb =3D 0; > + libxl_bitmap cpumap; > + libxl_vnode_info *p; > + *v, or *vnode, or *vinfo, all sounds better than *p. > + rc =3D 0; > +out: > + if (ninfo) libxl_numainfo_dispose(ninfo); > What you want here, to free a libxl_numainfo, is libxl_numainfo_list_free(ninfo,nr_nodes), which, BTW, can be called without checking whether ninfo is NULL, _provided_ you initialize nr_nodes to 0. Regards, Dario --=-Ylbp5cyRFV/UpUwhVTgr 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 iEYEABECAAYFAlTjbmcACgkQk4XaBE3IOsSsqwCfZMSZOn4L2HJxll+RXPaw4I7t Ur0AoKC8SrQikGvKCoarlEv4cow+9SGP =lvxE -----END PGP SIGNATURE----- --=-Ylbp5cyRFV/UpUwhVTgr-- --===============1383712305151545528== 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 --===============1383712305151545528==--