|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 7/7] libxl/vcpu-set - allow to decrease vcpu count on overcommitted guests (v3)
On Wed, 2015-03-25 at 14:42 -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 24, 2015 at 03:41:46PM +0000, Ian Campbell wrote:
> > On Mon, 2015-03-23 at 14:21 -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 fix the extra space in the message.
> > >
> > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > >
> > > ---
> > > [v2: Remove crud code as spotted by Boris]
> > > [v3: Moved crud code removal to another patch]
> > > ---
> > > tools/libxl/xl_cmdimpl.c | 15 ++++++++++++---
> > > 1 file changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > > index b121d75..d7bd5d5 100644
> > > --- a/tools/libxl/xl_cmdimpl.c
> > > +++ b/tools/libxl/xl_cmdimpl.c
> > > @@ -5238,9 +5238,18 @@ static int vcpuset(uint32_t domid, const char*
> > > nr_vcpus, int check_host)
> > > */
> > > if (check_host) {
> > > 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 " \
> > > + libxl_dominfo dominfo;
> > > +
> > > + rc = libxl_domain_info(ctx, &dominfo, domid);
> > > + if (rc)
> > > + {
> > > + if (rc == ERROR_DOMAIN_NOTFOUND)
> > > + fprintf(stderr, "Domain %u does not exist.\n", domid);
> > > + return 1;
> >
> > Indentation is wrong on the if.
> >
> > More generally all of these rc == ERROR_DOMAIN_NOTFOUND check and prints
> > are going to get rather tiresome, especially if/when there are other
> > interesting error codes. Perhaps we could arrange for something further
> > down the stack on libxl to log this sort of thing, such that xl can rely
> > on it already having been mentioned?
> >
> > e.g. libxl_domain_info could do it perhaps?
>
> Would this patch work?
Assuming we aren't expecting to lookup domains which don't exist very
often then I think this would be fine.
>
> From 9a0a0e581b29d9aa893d80962053383c235e9ad9 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Date: Wed, 25 Mar 2015 13:36:58 -0400
> Subject: [PATCH v3 4/8] libxl/libxl_domain_info: Log if domain not found.
>
> If we cannot find the domain - log an error (and still
> continue returning an error).
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> ---
> tools/libxl/libxl.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index c37d057..181b5bd 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -707,8 +707,10 @@ int libxl_domain_info(libxl_ctx *ctx, libxl_dominfo
> *info_r,
> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> return ERROR_FAIL;
> }
> - if (ret==0 || xcinfo.domain != domid) return ERROR_DOMAIN_NOTFOUND;
> -
> + if (ret==0 || xcinfo.domain != domid) {
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Domain %d not found!",
> domid);
> + return ERROR_DOMAIN_NOTFOUND;
> + }
> if (info_r)
> xcinfo2xlinfo(ctx, &xcinfo, info_r);
> return 0;
> --
> 2.1.0
>
>
>
> And then this patch would become:
>
> From 37d530f04a266e8d707b811bc7583f9d7b6fb18d Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Date: Mon, 2 Feb 2015 16:18:39 -0500
> Subject: [PATCH] libxl/vcpu-set - allow to decrease vcpu count on
> overcommitted guests (v3)
>
> 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 fix the extra space in the message.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>
> ---
> [v2: Remove crud code as spotted by Boris]
> [v3: Moved crud code removal to another patch]
> [v4: Remove the fprintf as it is done in libxl_domain_info]
> [v5: Fix memory leak]
> ---
> tools/libxl/xl_cmdimpl.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index b121d75..c77c691 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -5238,12 +5238,21 @@ static int vcpuset(uint32_t domid, const char*
> nr_vcpus, int check_host)
> */
> if (check_host) {
> 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);
> + libxl_dominfo dominfo;
> +
> + rc = libxl_domain_info(ctx, &dominfo, domid);
> + if (rc)
> return 1;
> +
> + if (max_vcpus > dominfo.vcpu_online && 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);
> + rc = 1;
> }
> + libxl_dominfo_dispose(&info);
> + if (rc)
> + return 1;
> }
> rc = libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus);
> if (rc) {
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |