From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2 2/5] libxl: vcpuset: Return error values. Date: Wed, 18 Mar 2015 13:06:18 +0000 Message-ID: <1426683978.14291.14.camel@citrix.com> References: <1426278403-12959-1-git-send-email-konrad.wilk@oracle.com> <1426278403-12959-3-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 1YYDg0-00058d-Eu for xen-devel@lists.xenproject.org; Wed, 18 Mar 2015 13:06:24 +0000 In-Reply-To: <1426278403-12959-3-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 function does not return any values at all. Convert the > internal libxl ones (ERROR_FAIL, ..., etc) to positive values > and for the other cases just return standard libxl values. It's not clear why you want to do this, in particular returning -ERROR_INVAL and inverting libxl error codes seems like a very strange thing to be doing. I think you should either use ERROR_INVAL (not inverted) and propagate libxl rc's directly or convert them into something which suits xl, i.e. 0 and 1. > Signed-off-by: Konrad Rzeszutek Wilk > --- > tools/libxl/xl_cmdimpl.c | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 2d7145f..454a895 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -5013,17 +5013,18 @@ int main_vcpupin(int argc, char **argv) > return rc; > } > > -static void vcpuset(uint32_t domid, const char* nr_vcpus, int check_host) > +static int vcpuset(uint32_t domid, const char* nr_vcpus, int check_host) > { > char *endptr; > unsigned int max_vcpus, i; > libxl_bitmap cpumap; > + int rc; > > libxl_bitmap_init(&cpumap); > max_vcpus = strtoul(nr_vcpus, &endptr, 10); > if (nr_vcpus == endptr) { > fprintf(stderr, "Error: Invalid argument.\n"); > - return; > + return -ERROR_INVAL; > } > > /* > @@ -5036,22 +5037,25 @@ static void vcpuset(uint32_t domid, const char* nr_vcpus, int check_host) > 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; > + return -ERROR_INVAL; > } > /* NB: This also limits how many are set in the bitmap */ > max_vcpus = (max_vcpus > host_cpu ? host_cpu : max_vcpus); > } > - if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) { > - fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n"); > - return; > + rc = libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus); > + if (rc) { > + fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", rc); > + return -rc; > } > for (i = 0; i < max_vcpus; i++) > libxl_bitmap_set(&cpumap, i); > > - if (libxl_set_vcpuonline(ctx, domid, &cpumap) < 0) > - fprintf(stderr, "libxl_set_vcpuonline failed domid=%d max_vcpus=%d\n", domid, max_vcpus); > + rc = libxl_set_vcpuonline(ctx, domid, &cpumap); > + if (rc) > + fprintf(stderr, "libxl_set_vcpuonline failed domid=%d max_vcpus=%d, rc: %d\n", domid, max_vcpus, rc); > > libxl_bitmap_dispose(&cpumap); > + return rc ? -rc : 0; > } > > int main_vcpuset(int argc, char **argv) > @@ -5070,8 +5074,7 @@ int main_vcpuset(int argc, char **argv) > break; > } > > - vcpuset(find_domain(argv[optind]), argv[optind + 1], check_host); > - return 0; > + return vcpuset(find_domain(argv[optind]), argv[optind + 1], check_host); > } > > static void output_xeninfo(void)