From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH V5 03/32] xl / libxl: push VCPU affinity pinning down to libxl Date: Thu, 15 May 2014 02:59:13 +0200 Message-ID: <1400115553.20215.9.camel@Solace> References: <1400018054-26038-1-git-send-email-wei.liu2@citrix.com> <1400018054-26038-4-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6963124778447419152==" Return-path: In-Reply-To: <1400018054-26038-4-git-send-email-wei.liu2@citrix.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 Cc: ian.jackson@eu.citrix.com, ian.campbell@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org --===============6963124778447419152== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-DI26PuPIRZDBeRQwHx7J" --=-DI26PuPIRZDBeRQwHx7J Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On mar, 2014-05-13 at 22:53 +0100, Wei Liu wrote: > This patch introduces a key value list called "vcpu_affinity" in libxl > IDL to preserve VCPU to PCPU mapping. This is necessary for libxl to > preserve all information to construct a domain. >=20 > Also define LIBXL_HAVE_AFFINITY_KEY_VALUE_LIST in libxl.h to mark the > change in API. >=20 > Signed-off-by: Wei Liu > Cc: Dario Faggioli > Reviewed-and-Tested-by: Dario Faggioli One question: I've skimmed through the rest of the series, and it looks like you are not doing anything particular with the other maps, already existing in b_info (cpumap and nodemap) yet, is that right? Also, a very minor comment on the code... > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index 661999c..b818815 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -263,6 +263,54 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > libxl_domain_set_nodeaffinity(ctx, domid, &info->nodemap); > libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, &info->cpuma= p); > =20 > + /* If we have vcpu affinity list, pin vcpu to pcpu. */ > + if (d_config->b_info.vcpu_affinity) { > + int i; > + libxl_bitmap vcpu_cpumap; > + int *vcpu_to_pcpu, sz =3D sizeof(int) * d_config->b_info.max_vcp= us; > + > + vcpu_to_pcpu =3D libxl__zalloc(gc, sz); > + memset(vcpu_to_pcpu, -1, sz); > + > + for (i =3D 0; i < d_config->b_info.max_vcpus; i++) { > + libxl_key_value_list kvl =3D d_config->b_info.vcpu_affinity; > + const char *key, *val; > + int k, v; > + > + key =3D kvl[i * 2]; > + if (!key) > + break; > + val =3D kvl[i * 2 + 1]; > + > + k =3D atoi(key); > + v =3D atoi(val); > + vcpu_to_pcpu[k] =3D v; > + } > + > + rc =3D libxl_cpu_bitmap_alloc(ctx, &vcpu_cpumap, 0); > + if (rc) { > + libxl_bitmap_dispose(&vcpu_cpumap); > + return ERROR_FAIL; > + } > + Perhaps you can move this up, i.e., try the allocation first so that, if it fails, you can bail before getting into decoding the key/value list? Also, if libxl_cpu_bitmap_alloc() fails, do you really have to call libxl_bitmap_dispose()? I thought not. Anyway, as said, these both are minor issues. Just give them a thought if you respin the series but, if not, the patch is fine as it is for me. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-DI26PuPIRZDBeRQwHx7J 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.0.22 (GNU/Linux) iEYEABECAAYFAlN0EWIACgkQk4XaBE3IOsTiXgCffVgHesYKioyDVPPysMu5xYw2 MkAAoJC0a41XOQmOEOzTxobR2RCD+zQQ =93Mr -----END PGP SIGNATURE----- --=-DI26PuPIRZDBeRQwHx7J-- --===============6963124778447419152== 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 --===============6963124778447419152==--