From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v3 11/14] libxl: get and set soft affinity Date: Tue, 19 Nov 2013 18:51:18 +0100 Message-ID: <1384883478.19880.170.camel@Abyss> References: <20131118175544.31002.79574.stgit@Solace> <20131118181813.31002.61195.stgit@Solace> <1384881864.16252.48.camel@hastur.hellion.org.uk> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6769312458009998387==" Return-path: In-Reply-To: <1384881864.16252.48.camel@hastur.hellion.org.uk> 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 Campbell Cc: Marcus Granado , Keir Fraser , Matt Wilson , Li Yechen , George Dunlap , Andrew Cooper , Juergen Gross , Ian Jackson , xen-devel@lists.xen.org, Jan Beulich , Justin Weaver , Elena Ufimtseva List-Id: xen-devel@lists.xenproject.org --===============6769312458009998387== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-8VFo6+HP/GpJOk7XLGW7" --=-8VFo6+HP/GpJOk7XLGW7 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On mar, 2013-11-19 at 17:24 +0000, Ian Campbell wrote: > > Signed-off-by: Dario Faggioli > > +int libxl_set_vcpuaffinity2(libxl_ctx *ctx, uint32_t domid, uint32_t v= cpuid, > > + const libxl_bitmap *cpumap, int flags) >=20 > I think if we are going to duplicate hte API in this way then we should > still combine the implementation, either with an internal private helper > or by making the old API a wrapper around the new one. >=20 > The internals of ...2 and ...3 should be shared as far as possible too. >=20 Ok, this can be done, I think. > > +{ > > + libxl_cputopology *topology; > > + libxl_bitmap ecpumap; > > + int nr_cpus =3D 0, rc; > > + > > + topology =3D libxl_get_cpu_topology(ctx, &nr_cpus); > > + if (!topology) { > > + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "failed to retrieve CPU topo= logy"); >=20 > It's not consistent within the file but I think for new functions we > should use the LOG macro variants. >=20 Right, but don't I need a gc to use it? Should I "make up" one just for the purpose of using LOG/LOGE? > > + return ERROR_FAIL; > > + } > > + libxl_cputopology_list_free(topology, nr_cpus); >=20 > Why are you retrieving this only to immediately throw it away? >=20 Because I need nr_cpus. :-) > > +int libxl_set_vcpuaffinity3(libxl_ctx *ctx, uint32_t domid, uint32_t v= cpuid, > > + const libxl_bitmap *cpumap_hard, > > + const libxl_bitmap *cpumap_soft) >=20 > Insert the same comments as ...2, because AFAICT it is mostly the same > function. >=20 I will. > > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > > index c7dceda..504c57b 100644 > > --- a/tools/libxl/libxl.h > > +++ b/tools/libxl/libxl.h > > @@ -82,6 +82,20 @@ > > #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1 > > =20 > > /* > > + * LIBXL_HAVE_VCPUINFO_SOFTAFFINITY indicates that a 'cpumap_soft' > > + * field (of libxl_bitmap type) is present in libxl_vcpuinfo, > > + * containing the soft affinity for the vcpu. > > + */ > > +#define LIBXL_HAVE_VCPUINFO_SOFTAFFINITY 1 > > + > > +/* > > + * LIBXL_HAVE_BUILDINFO_SOFTAFFINITY indicates that a 'cpumap_soft' > > + * field (of libxl_bitmap type) is present in libxl_domain_build_info, > > + * containing the soft affinity for the vcpu. > > + */ > > +#define LIBXL_HAVE_BUILDINFO_SOFTAFFINITY 1 >=20 > Given that they arrive can we just use HAVE_SOFTRAFFINITY? >=20 You mean just introducing one #define? Sure... For some reason I assumed that every new field should come with it's own symbol. But if it's fine to have one, I'm all for it. :-) > > +/* Flags, consistent with domctl.h */ > > +#define LIBXL_VCPUAFFINITY_HARD 1 > > +#define LIBXL_VCPUAFFINITY_SOFT 2 >=20 > Can these be an enum in the idl? >=20 I think yes. I did actually check and, of all the enum-s in the IDL, none are used as flags, they're rather used as "single values". OTOH, the only actual flags I found (I think it was LIBXL_SUSPEND_DEBUG, LIBXL_SUSPEND_LIVE) were defined like I did myself above... That's why I went for it. But again, if you're fine with these being enum, I will make them so. 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) --=-8VFo6+HP/GpJOk7XLGW7 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) iEYEABECAAYFAlKLpRYACgkQk4XaBE3IOsTZswCffArrfNjCH0MVJ2rTn3gSGLIQ PnoAn2WU6xJbvUuwcw7guSB6vAq4zmf5 =T700 -----END PGP SIGNATURE----- --=-8VFo6+HP/GpJOk7XLGW7-- --===============6769312458009998387== 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 --===============6769312458009998387==--