From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH v1 2/2] libxl: vcpu-set - allow to decrease vcpu count on overcommitted guests (v2) Date: Thu, 5 Jun 2014 13:44:10 -0400 Message-ID: <20140605174409.GC2546@localhost.localdomain> References: <1401888813-30987-1-git-send-email-konrad.wilk@oracle.com> <1401888813-30987-3-git-send-email-konrad.wilk@oracle.com> <1401966177.29759.10.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Wsbi9-000450-Jw for xen-devel@lists.xenproject.org; Thu, 05 Jun 2014 17:44:21 +0000 Content-Disposition: inline In-Reply-To: <1401966177.29759.10.camel@kazak.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: Ian Campbell Cc: xen-devel@lists.xenproject.org, ian.jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On Thu, Jun 05, 2014 at 12:02:57PM +0100, Ian Campbell wrote: > On Wed, 2014-06-04 at 09:33 -0400, Konrad Rzeszutek Wilk wrote: > > We have a check to warn the user if they are overcommitting. > > But the check only checks the hosts CPU amount and does > > not take into account the case when the user is trying to fix > > the overcommit. That is - they want to limit the amount of > > online VCPUs. > > > > This fix allows the user to offline vCPUs without any > > warnings when they are running an overcommitted guest. > > > > Also while at it, remove crud code. > > > > Signed-off-by: Konrad Rzeszutek Wilk > > Contrary to $SUBJECT this is an xl patch not a libxl one. Also there is > a spurious "(v2)" in the subject. > > > [v2: Remove crud code as spotted by Boris] > > --- > > tools/libxl/xl_cmdimpl.c | 20 +++++++++++++------- > > 1 file changed, 13 insertions(+), 7 deletions(-) > > > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > > index 5195914..5b27bd8 100644 > > --- a/tools/libxl/xl_cmdimpl.c > > +++ b/tools/libxl/xl_cmdimpl.c > > @@ -4754,15 +4754,21 @@ static void vcpuset(uint32_t domid, const char* nr_vcpus, int check_host) > > * by the host's amount of pCPUs. > > */ > > if (check_host) { > > + libxl_dominfo dominfo; > > + > > unsigned int host_cpu = libxl_get_max_cpus(ctx); > > - if (max_vcpus > host_cpu) { > > - fprintf(stderr, "You are overcommmitting! You have %d physical " \ > > - " CPUs and want %d vCPUs! Aborting, use --ignore-host to " \ > > - " continue\n", host_cpu, max_vcpus); > > - return; > > + > > + if (libxl_domain_info(ctx, &dominfo, domid) != 0) > > + dominfo.vcpu_online = host_cpu; > > + > > + if (max_vcpus > dominfo.vcpu_online) { > > + if ((max_vcpus > host_cpu)) { > > I think this is > if (max_vcpus > dominfo.vcpu_online && max_vcpus > host_cpu) { > > and if not then the second one has a spurious set of ()s. > > > + fprintf(stderr, "You are overcommmitting! You have %d physical" \ > > You've carried over the typo here (unless you intended to overcommit on > the number of m's ;-)). Might as well fix while you are here.. Mmmmm.. You are riggggghhhhhhhhhhttttttttt. > > > + " CPUs and want %d vCPUs! Aborting, use --ignore-host to" \ > > + " continue\n", host_cpu, max_vcpus); > > + return; > > + } > > } > > - /* NB: This also limits how many are set in the bitmap */ > > - max_vcpus = (max_vcpus > host_cpu ? host_cpu : max_vcpus); > > Where did this go? No need for it actually. As we already do the action if 'max_vcpus > host_cpu' - which is that we return. So in essence that code will set max_vcpus to max_vcpus. > > > } > > if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) { > > fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n"); > >