From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2 3/5] libxl: vcpuset: Check max_vcpus argument against the maximum number of vCPUs the guest has set. Date: Wed, 18 Mar 2015 13:11:50 +0000 Message-ID: <1426684310.14291.18.camel@citrix.com> References: <1426278403-12959-1-git-send-email-konrad.wilk@oracle.com> <1426278403-12959-4-git-send-email-konrad.wilk@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YYE0P-0007yE-4v for xen-devel@lists.xenproject.org; Wed, 18 Mar 2015 13:27:29 +0000 In-Reply-To: <1426278403-12959-4-git-send-email-konrad.wilk@oracle.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: Konrad Rzeszutek Wilk Cc: xen-devel@lists.xenproject.org, ian.jackson@eu.citrix.com, wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On Fri, 2015-03-13 at 16:26 -0400, Konrad Rzeszutek Wilk wrote: > The maximum number of VCPUs the guest can have is determined during > domain creation and is set by 'maxvcpus' parameter (in the guest > config). Trying to set the amount of vCPUs above said value > in vcpuset will result in an error - and we can catch it here > (instead of later in the function) and print a nice warning to the user. I suppose the "later in the function" is in the return value of libxl_set_vcpuonline? I think this check would be better off done inside the library, i.e. in libxl_set_vcpuonline, with a (probably new) suitable error code. The libxl can log and xl can spot the situation and log something specific (if indeed it needs to once libxl does). Ian. > > Signed-off-by: Konrad Rzeszutek Wilk > [v2: Check ERROR_NOTFOUND return value] > --- > tools/libxl/xl_cmdimpl.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 454a895..ba0fd71 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -5018,6 +5018,7 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, int check_host) > char *endptr; > unsigned int max_vcpus, i; > libxl_bitmap cpumap; > + libxl_dominfo dominfo; > int rc; > > libxl_bitmap_init(&cpumap); > @@ -5026,7 +5027,20 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, int check_host) > fprintf(stderr, "Error: Invalid argument.\n"); > return -ERROR_INVAL; > } > - > + rc = libxl_domain_info(ctx, &dominfo, domid); > + if (rc == ERROR_NOTFOUND) { > + fprintf(stderr, "Error: Domain %u does not exist.\n", domid); > + return -rc; > + } > + if (rc) { > + fprintf(stderr, "libxl_domain_info failed (code %d).\n", rc); > + return -rc; > + } > + if (max_vcpus > dominfo.vcpu_max_id + 1) { > + fprintf(stderr, "You have a max of %d vCPUs and you want %d vCPUs!\n", > + dominfo.vcpu_max_id + 1, max_vcpus); > + return -ERROR_INVAL; > + } > /* > * Maximum amount of vCPUS the guest is allowed to set is limited > * by the host's amount of pCPUs.