Re: [Xen-devel] [PATCH v1 2/2] libxl: vcpu-set - allow to decrease vcpu count on overcommitted guests (v2)

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-06-04 at 09:33 -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 while at it, remove crud code.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

Contrary to $SUBJECT this is an xl patch not a libxl one. Also there is
a spurious "(v2)" in the subject.

> [v2: Remove crud code as spotted by Boris]
> ---
>  tools/libxl/xl_cmdimpl.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 5195914..5b27bd8 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -4754,15 +4754,21 @@ static void vcpuset(uint32_t domid, const char* 
> nr_vcpus, int check_host)
>       * by the host's amount of pCPUs.
>       */
>      if (check_host) {
> +        libxl_dominfo dominfo;
> +
>          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 
> " \
> -                    " continue\n", host_cpu, max_vcpus);
> -            return;
> +
> +        if (libxl_domain_info(ctx, &dominfo, domid) != 0)
> +            dominfo.vcpu_online = host_cpu;
> +
> +        if (max_vcpus > dominfo.vcpu_online) {
> +            if ((max_vcpus > host_cpu)) {

I think this is 
        if (max_vcpus > dominfo.vcpu_online && max_vcpus > host_cpu) {

and if not then the second one has a spurious set of ()s.

> +                fprintf(stderr, "You are overcommmitting! You have %d 
> physical" \

You've carried over the typo here (unless you intended to overcommit on
the number of m's ;-)). Might as well fix while you are here..

> +                        " CPUs and want %d vCPUs! Aborting, use 
> --ignore-host to" \
> +                        " continue\n", host_cpu, max_vcpus);
> +                return;
> +            }
>          }
> -        /* NB: This also limits how many are set in the bitmap */
> -        max_vcpus = (max_vcpus > host_cpu ? host_cpu : max_vcpus);

Where did this go?

>      }
>      if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) {
>          fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n");



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