From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH for Xen 4.6 1/5] tools/libxl: introduce libxl_socket_bitmap_fill Date: Mon, 28 Sep 2015 16:53:58 +0200 Message-ID: <1443452038.3276.77.camel@citrix.com> References: <1443441293-4287-1-git-send-email-chao.p.peng@linux.intel.com> <1443441293-4287-2-git-send-email-chao.p.peng@linux.intel.com> <20150928141227.GB13821@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2630760224052207591==" Return-path: In-Reply-To: <20150928141227.GB13821@zion.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: Wei Liu , Chao Peng Cc: xen-devel@lists.xen.org, Ian.Jackson@eu.citrix.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org --===============2630760224052207591== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-3X8gTmidmtKSSyx3+Oyo" --=-3X8gTmidmtKSSyx3+Oyo Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2015-09-28 at 15:12 +0100, Wei Liu wrote: > On Mon, Sep 28, 2015 at 07:54:49PM +0800, Chao Peng wrote: > > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > > index 5f9047c..5a91687 100644 > > --- a/tools/libxl/libxl.h > > +++ b/tools/libxl/libxl.h > > =20 > > /* > > - * LIBXL_HAVE_SOCKET_BITMAP_ALLOC > > + * LIBXL_HAVE_SOCKET_BITMAP > > * > > - * If this is defined, then libxl_socket_bitmap_alloc exists. > > + * If this is defined, then libxl_socket_bitmap_alloc and > > + * libxl_socket_bitmap_Fill exist. >=20 > _Fill -> _fill. >=20 Right. However, it seems to me that the function would be better named libxl_get_online_sockets() or something like that. I see that you want the actual map. For CPUs and NUMA nodes, we do have libxl_get_online_{cpus,nodes}(), but they return the number of online CPUs and nodes, so for consistency, libxl_get_online_sockets() would better (if necessary at some point) behave similarly. What about libxl_get_online_socketmap() ? This is still a nice (IMO) name for what you're after here, and it leaves us room to implement libxl_get_online_cpumap() and libxl_get_online_nodemap(), if we'll ever need those. Thoughs? > > */ > > -#define LIBXL_HAVE_SOCKET_BITMAP_ALLOC 1 > > +#define LIBXL_HAVE_SOCKET_BITMAP 1 > > =20 >=20 > Normally we don't / can't delete existing macros. However this macro > was > introduced in this cycle, so we have the liberty to change it before > releasing. >=20 I was about to say exactly the same thing. > > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c > > index bfc9699..557f279 100644 > > --- a/tools/libxl/libxl_utils.c > > +++ b/tools/libxl/libxl_utils.c > > +int libxl_socket_bitmap_fill(libxl_ctx *ctx, libxl_bitmap > > *socketmap) > > +{ > > + libxl_cputopology *tinfo =3D NULL; > > + int nr_cpus =3D 0, i, rc =3D 0; > > + > > + tinfo =3D libxl_get_cpu_topology(ctx, &nr_cpus); > > + if (tinfo =3D=3D NULL) { > > + rc =3D ERROR_FAIL; > > + goto out; > > + } > > + > > + libxl_bitmap_set_none(socketmap); > > + for (i =3D 0; i < nr_cpus; i++) > > + if (tinfo[i].socket !=3D XEN_INVALID_SOCKET_ID > > + && !libxl_bitmap_test(socketmap, tinfo[i].socket)) > > + libxl_bitmap_set(socketmap, tinfo[i].socket); > > + out: > > + libxl_cputopology_list_free(tinfo, nr_cpus); > > + return rc; > > + > > +} >=20 > Need extra blank line. >=20 And, at the same time, the blank line between 'return rc;' and the '{' is not needed. IOW, we want this: ... libxl_cputopology_list_free(tinfo, nr_cpus); return rc; } int libxl_nodemap_to_cpumap(... ... Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-3X8gTmidmtKSSyx3+Oyo 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 iEYEABECAAYFAlYJVIYACgkQk4XaBE3IOsSAmACdHTSkdEbdgQWRIRajYehcpi1O xFAAn3ca9REfF4b0yj8u0g3kAbhW7m+D =sOAp -----END PGP SIGNATURE----- --=-3X8gTmidmtKSSyx3+Oyo-- --===============2630760224052207591== 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 --===============2630760224052207591==--