From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 2/3] xl: convert vcpu related return codes to EXIT_[SUCCESS|FAILURE] Date: Fri, 23 Oct 2015 12:41:21 +0200 Message-ID: <1445596881.5117.92.camel@citrix.com> References: <1445586491-15093-1-git-send-email-write.harmandeep@gmail.com> <1445586491-15093-2-git-send-email-write.harmandeep@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0839349758104586704==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZpZmp-0001ti-EC for xen-devel@lists.xenproject.org; Fri, 23 Oct 2015 10:41:27 +0000 In-Reply-To: <1445586491-15093-2-git-send-email-write.harmandeep@gmail.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: Harmandeep Kaur , xen-devel@lists.xenproject.org Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, ian.jackson@eu.citrix.com, george.dunlap@citrix.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org --===============0839349758104586704== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-WNDui9U0CqhDzZDg3eSf" --=-WNDui9U0CqhDzZDg3eSf Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2015-10-23 at 13:18 +0530, Harmandeep Kaur wrote: > turning vcpu manipulation functions xl exit codes toward using the > EXIT_[SUCCESS|FAILURE] macros, instead of instead of arbitrary > numbers > or libxl return codes. >=20 So, this patch is, technically, mostly fine. The observations on the subject made when reviewing patch 1 applies here too, of course. Something on the selection of functions the functions. The title says the patch will address "vcpu related functions". However, there are a bunch of parse_*() functions in the diff. I appreciate that, for instance, parse_vcpu_affinity() can be considered a vcpu related function, but that applies a lot less to parse_vnuma_config(), IMO. I'd therefore exclude the latter from this patch. Alternatively, get rid of both and do another patch specifically for dealing with parse_*() items. There are not many of them that are actual exit paths and/or contain calls to exit() (parse_config_data() is big enough, and a bit more complex, that it may well deserve its own patch, and you can leave it alone, just mention that in the changelog). A few more comments below. > @@ -5461,7 +5461,7 @@ static int vcpuset(uint32_t domid, const char* > nr_vcpus, int check_host) > =20 > rc =3D libxl_domain_info(ctx, &dominfo, domid); > if (rc) > - return 1; > + return EXIT_FAILURE; > =20 if (libxl_domain_info(ctx, &dominfo, domid)) return EXIT_FAILURE; > if (max_vcpus > dominfo.vcpu_online && max_vcpus > host_cpu) > { > fprintf(stderr, "You are overcommmitting! You have %d > physical" \ > @@ -5471,12 +5471,12 @@ static int vcpuset(uint32_t domid, const > char* nr_vcpus, int check_host) > } > libxl_dominfo_dispose(&dominfo); > if (rc) > - return 1; > + return EXIT_FAILURE; > } > rc =3D libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus); > if (rc) { > fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", > rc); > - return 1; > + return EXIT_FAILURE; > if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) return EXIT_FAILURE; 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) --=-WNDui9U0CqhDzZDg3eSf 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 iEYEABECAAYFAlYqDtEACgkQk4XaBE3IOsREMgCgqHpiYAqkAPAp9htyNxQBqlrK PI4AoKvPZVznjW3nBmXPO/T1LbCVp5Nf =XTeW -----END PGP SIGNATURE----- --=-WNDui9U0CqhDzZDg3eSf-- --===============0839349758104586704== 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 --===============0839349758104586704==--