From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH for-4.6 v2 2/3] xl: error out if vNUMA specifies more vcpus than pcpus Date: Fri, 14 Aug 2015 12:19:07 +0200 Message-ID: <1439547547.27252.3.camel@citrix.com> References: <1439480480-20939-1-git-send-email-wei.liu2@citrix.com> <1439480480-20939-3-git-send-email-wei.liu2@citrix.com> <1439508345.24583.91.camel@citrix.com> <20150813233802.GA4698@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2400481661126576991==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZQC4x-0000WI-V0 for xen-devel@lists.xenproject.org; Fri, 14 Aug 2015 10:19:16 +0000 In-Reply-To: <20150813233802.GA4698@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 Cc: Xen-devel , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org --===============2400481661126576991== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-6RpLl0H/CZ00yfOZKAa3" --=-6RpLl0H/CZ00yfOZKAa3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2015-08-14 at 00:38 +0100, Wei Liu wrote: > On Fri, Aug 14, 2015 at 01:25:45AM +0200, Dario Faggioli wrote: > > > --- a/tools/libxl/xl_cmdimpl.c > > > +++ b/tools/libxl/xl_cmdimpl.c > > > @@ -1202,11 +1202,27 @@ static void parse_vnuma_config(const XLU_Conf= ig *config, > > > } > > > =20 > > > /* User has specified maxvcpus=3D */ > > > - if (b_info->max_vcpus !=3D 0 && b_info->max_vcpus !=3D max_vcpu= s) { > > > - fprintf(stderr, "xl: vnuma vcpus and maxvcpus=3D mismatch\n"= ); > > > - exit(1); > > > - } else > > > + if (b_info->max_vcpus !=3D 0) { > > > + if (b_info->max_vcpus !=3D max_vcpus) { > > > + fprintf(stderr, "xl: vnuma vcpus and maxvcpus=3D mismatc= h\n"); > > > + exit(1); > > > + } > > > + } else { > > > + int host_cpus =3D libxl_get_online_cpus(ctx); > > > + > > > + if (host_cpus < 0) { > > > + fprintf(stderr, "Failed to get online cpus\n"); > > > + exit(1); > > > + } > > > + > > > + if (host_cpus < max_vcpus) { > > > + fprintf(stderr, "xl: vnuma specifies more vcpus than pcp= us, "\ > > > + "use maxvcpus=3D to override this check.\n"); > > > > > ...isn't it too late, when we get to here? > >=20 > That's fine because that function has no effect when you try to set a > bit beyond its size. >=20 Well, right. Fair enough. > > Assuming I'm not, it seems to me that a solution could be to check for > > this situation _inside_ the 'else if (!strcmp("vcpus", option))'. In > > fact, if "maxvcpus" has not been specified, as soon as the end of one o= f > > the ranges --as returned by parse_range()-- is beyond host_cpus, we kno= w > > we'd be going past the limit of the corresponding element of > > vcpu_parsed, and we can error out. > >=20 > > It'll most likely be a bit uglier than this patch, but probably still > > less complex than v1. :-) > >=20 >=20 > That doesn't make any difference in terms of functionality. I would > rather leave the parsing bit as it is and deal with fallout separately. > That would make code cleaner IMHO. >=20 It's certainly cleaner as it is right now, in this version of the patch. I still find the thing above a bit confusing, although correct because of the fact that bitmap set is a nop if performed out of range. Perhaps a short comment? Anyway, I don't have a strong opinion on this so, with or without it (the comment): Reviewed-by: Dario Faggioli Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-6RpLl0H/CZ00yfOZKAa3 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 iEYEABECAAYFAlXNwJsACgkQk4XaBE3IOsRl8QCghBPgI+2tjxb8z4zVVv4fX6YT hZkAn1Wr3w9E5aOshrVCQkmX4AWU0Ljd =g+Xj -----END PGP SIGNATURE----- --=-6RpLl0H/CZ00yfOZKAa3-- --===============2400481661126576991== 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 --===============2400481661126576991==--