Re: [Xen-devel] [PATCH v2 2/5] libxl: vcpuset: Return error values.

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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 <konrad.wilk@xxxxxxxxxx>
> ---
>  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)



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel