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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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?

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) {
-- 
2.1.0


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