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 01:25:45 +0200 Message-ID: <1439508345.24583.91.camel@citrix.com> References: <1439480480-20939-1-git-send-email-wei.liu2@citrix.com> <1439480480-20939-3-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6198531486955224338==" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZQ1sl-0001Zw-6M for xen-devel@lists.xenproject.org; Thu, 13 Aug 2015 23:25:59 +0000 In-Reply-To: <1439480480-20939-3-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: Xen-devel , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org --===============6198531486955224338== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-1XoXoehEvmpwz3Bxg21J" --=-1XoXoehEvmpwz3Bxg21J Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2015-08-13 at 16:41 +0100, Wei Liu wrote: > ... but allow user to override that check by specifying maxvcpus=3D in xl > configuration file. >=20 Ok, from the discussion on v1, and from the subject of this new submission, I now see that what you're after the "more vcpus (in a single guest) than pcpus" case. This is so uncommon, IMO, that it did not even cross my mind while looking at v1.. Sorry for this. :-) That being said, I agree with IanC's view, as he expressed it during v1 review, and I find this new version of the patch much better! However, although I like the idea, as I said... > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 078acd1..5fde8fa 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -1202,11 +1202,27 @@ static void parse_vnuma_config(const XLU_Config *= config, > } > =20 > /* User has specified maxvcpus=3D */ > - if (b_info->max_vcpus !=3D 0 && b_info->max_vcpus !=3D max_vcpus) { > - 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 mismatch\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 pcpus, = "\ > + "use maxvcpus=3D to override this check.\n"); > ...isn't it too late, when we get to here? In fact, if b_info->max_vcpus is 0, the elements of vcpu_parsed are sized against the host pcpus, and we risk to call libxl_bitmap_set() for vcpus beyond that limit, while parsing the "vcpus" subsection of the vnode specification (which happens _before_ this check). Or am I missing something? 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 of the ranges --as returned by parse_range()-- is beyond host_cpus, we know we'd be going past the limit of the corresponding element of vcpu_parsed, and we can error out. It'll most likely be a bit uglier than this patch, but probably still less complex than v1. :-) Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-1XoXoehEvmpwz3Bxg21J 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 iEYEABECAAYFAlXNJ3kACgkQk4XaBE3IOsSr6ACffnfItOdRTNEe0tugfbhCMbhv 1BAAn3E5zJoVLSimof9H96621HVvgjCL =gVjf -----END PGP SIGNATURE----- --=-1XoXoehEvmpwz3Bxg21J-- --===============6198531486955224338== 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 --===============6198531486955224338==--