From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 5/9] libxl: introduce libxl_cpupool_cpu{add, remove}_cpumap() Date: Mon, 9 Mar 2015 11:31:09 +0000 Message-ID: <1425900667.2729.30.camel@citrix.com> References: <20150306170758.7269.53821.stgit@Solace.station> <20150306172132.7269.83077.stgit@Solace.station> <20150309103935.GH18491@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0282921770152681383==" Return-path: In-Reply-To: <20150309103935.GH18491@zion.uk.xensource.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: "JGross@suse.com" , Ian Jackson , Stefano Stabellini , Ian Campbell , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org --===============0282921770152681383== Content-Language: en-US Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-98oWhfV0SGYd3bFUW6lv" --=-98oWhfV0SGYd3bFUW6lv Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2015-03-09 at 10:39 +0000, Wei Liu wrote: > On Fri, Mar 06, 2015 at 06:21:32PM +0100, Dario Faggioli wrote: > > --- a/tools/libxl/libxl.c > > +++ b/tools/libxl/libxl.c > > @@ -6343,17 +6343,31 @@ out: > > =20 > > int libxl_cpupool_cpuadd(libxl_ctx *ctx, uint32_t poolid, int cpu) > > { > > + GC_INIT(ctx); > > int rc; > > =20 > > rc =3D xc_cpupool_addcpu(ctx->xch, poolid, cpu); > > if (rc) { > > - LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, > > - "Error moving cpu to cpupool"); > > + LOGE(ERROR, "Error moving cpu %d to cpupool", cpu); > > + GC_FREE; > > return ERROR_FAIL; > > } > > + GC_FREE; >=20 > Please use "goto" idiom. Same applies to libxl_cpupool_cpuremove. >=20 Ok. > > return 0; > > } > > =20 > > +int libxl_cpupool_cpuadd_cpumap(libxl_ctx *ctx, uint32_t poolid, > > + const libxl_bitmap *cpumap) > > +{ > > + int c, ncpus =3D 0; > > + > > + libxl_for_each_set_bit(c, *cpumap) { > > + if (!libxl_cpupool_cpuadd(ctx, poolid, c)) > > + ncpus++; > > + } > > + return ncpus; > > +} >=20 > I think returning a libxl error code on error and 0 on success is > better. At least this stay in line with libxl_cpupool_cpuadd_node. >=20 Will do. > > --- a/tools/libxl/libxl.h > > +++ b/tools/libxl/libxl.h > > @@ -1456,8 +1456,12 @@ int libxl_cpupool_destroy(libxl_ctx *ctx, uint32= _t poolid); > > int libxl_cpupool_rename(libxl_ctx *ctx, const char *name, uint32_t po= olid); > > int libxl_cpupool_cpuadd(libxl_ctx *ctx, uint32_t poolid, int cpu); > > int libxl_cpupool_cpuadd_node(libxl_ctx *ctx, uint32_t poolid, int nod= e, int *cpus); > > +int libxl_cpupool_cpuadd_cpumap(libxl_ctx *ctx, uint32_t poolid, > > + const libxl_bitmap *cpumap); > > int libxl_cpupool_cpuremove(libxl_ctx *ctx, uint32_t poolid, int cpu); > > int libxl_cpupool_cpuremove_node(libxl_ctx *ctx, uint32_t poolid, int = node, int *cpus); > > +int libxl_cpupool_cpuremove_cpumap(libxl_ctx *ctx, uint32_t poolid, > > + const libxl_bitmap *cpumap); > > int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t= domid); > > int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32= _t poolid); > > =20 >=20 > Missing #define LIBXl_HAVE_$FOO. >=20 So, do we need to do this every time we add a new function, even if not changing any existing one, not adding fields to any data structure, etc.? Regards, Dario --=-98oWhfV0SGYd3bFUW6lv 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 iEYEABECAAYFAlT9hHsACgkQk4XaBE3IOsSmFACeP7YNXSbmgd4jvvovnVYBEdhk 7IEAn1rnF7YI+hD+4dvKq4WCSNSi4P3V =JHbj -----END PGP SIGNATURE----- --=-98oWhfV0SGYd3bFUW6lv-- --===============0282921770152681383== 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 --===============0282921770152681383==--