From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Ian vs Ian, round 0 Was:Re: [PATCH v3 7/7] libxl/vcpu-set - allow to decrease vcpu count on overcommitted guests (v3) Date: Tue, 24 Mar 2015 11:56:04 -0400 Message-ID: <20150324155604.GG14418@l.oracle.com> References: <1427134861-18881-1-git-send-email-konrad.wilk@oracle.com> <1427134861-18881-8-git-send-email-konrad.wilk@oracle.com> <1427211706.21742.444.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YaRBd-0000zE-G1 for xen-devel@lists.xenproject.org; Tue, 24 Mar 2015 15:56:13 +0000 Content-Disposition: inline In-Reply-To: <1427211706.21742.444.camel@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: Ian Campbell , ian.jackson@eu.citrix.com, wei.liu2@citrix.com Cc: xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Tue, Mar 24, 2015 at 03:41:46PM +0000, Ian Campbell wrote: > On Mon, 2015-03-23 at 14:21 -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 fix the extra space in the message. > > > > Signed-off-by: Konrad Rzeszutek Wilk > > > > --- > > [v2: Remove crud code as spotted by Boris] > > [v3: Moved crud code removal to another patch] > > --- > > tools/libxl/xl_cmdimpl.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > > index b121d75..d7bd5d5 100644 > > --- a/tools/libxl/xl_cmdimpl.c > > +++ b/tools/libxl/xl_cmdimpl.c > > @@ -5238,9 +5238,18 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, int check_host) > > */ > > if (check_host) { > > 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 " \ > > + libxl_dominfo dominfo; > > + > > + rc = libxl_domain_info(ctx, &dominfo, domid); > > + if (rc) > > + { > > + if (rc == ERROR_DOMAIN_NOTFOUND) > > + fprintf(stderr, "Domain %u does not exist.\n", domid); > > + return 1; > > Indentation is wrong on the if. Right! > > More generally all of these rc == ERROR_DOMAIN_NOTFOUND check and prints > are going to get rather tiresome, especially if/when there are other > interesting error codes. Perhaps we could arrange for something further > down the stack on libxl to log this sort of thing, such that xl can rely > on it already having been mentioned? > > e.g. libxl_domain_info could do it perhaps? That would be the easiest way. Should I drop libxl: Add ERROR_DOMAIN_NOTFOUND for libxl_domain_info when it cannot find the domain and just add an LIBXL__LOG_ERRNO and keep on returning the old error value (ERROR_INVAL)? Ian J recommended to add this new ERROR_DOMAIN_NOTFOUND in libxl_domain_info so I think I will let you two fight it out! (changing the title to catch Ian J's attention) > > Or perhaps a helper (in xl?) which produces some common logging for > common errors which could be called? Nah, I am all for simplicity and just need these errors to be printed out. We could also just leave the libxl: Add ERROR_DOMAIN_NOTFOUND for libxl_domain_info when it cannot find the domain and I can add another patch that just adds an nice LIBXL__LOG_ERRNO in the libxl_domain_info? >