Re: [PATCH v2 3/5] libxl: vcpuset: Check max_vcpus argument against the maximum number of vCPUs the guest has set.

[Ian Campbell <ian.campbell@citrix.com>]

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 <konrad.wilk@oracle.com>
> [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.