[Xen-changelog] [xen master] libxl: Make 'xl vcpu-set' work properly on overcommited hosts with an override.

[patchbot@xxxxxxx]

commit afab6f6788b3e1eeed7fd7d2550bca96edca2720
Author:     Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
AuthorDate: Mon May 13 15:29:14 2013 -0400
Commit:     Ian Campbell <ian.campbell@xxxxxxxxxx>
CommitDate: Tue May 14 10:02:13 2013 +0100

    libxl: Make 'xl vcpu-set' work properly on overcommited hosts with an 
override.
    
    The libxl_cpu_bitmap_alloc(..) function, if provided with a zero
    value for max CPUs will call xc_get_max_cpus() which will retrieve
    the number of physical CPUs the host has. This is usually
    OK if the guest's maxvcpus <= host pcpus. But if the value
    is different, then the bitmap for VCPUs is limited by the
    number of CPUs the host has.
    
    This is incorrect as what we want is to hotplug in the guest
    the amount of CPUs that the user specified on the command line
    and not be limited by the amount of physical CPUs.
    
    This means that a guest config like this:
    
    vcpus=8
    maxvcpus=32
    
    and on a 4 PCPU machine doing
    
    xl vcpu-set <guest name> 16
    
    won't work. This is b/c the the size of the bitmap is one byte
    so it can only hold up to 8 VCPUs. Hence anything above that
    is going to be ignored.
    
    Note that this patch also fixes the bitmap setting - as it
    would set all of the bits allowed. Meaning if the user had a 4PCPU
    host we would still allow the user to set 8VCPUs. This second
    iteration of the patch fixes this.
    
    Note that all of the libxl_cpu_bitmap_[test|set] silently ignore
    any test or sets above its size:
    
         if (bit >= bitmap->size * 8)
             return 0;
    
    so we were never notified off this bug.
    
    This patch warns the user if they are trying to do this. If the
    user really wants to do this they have to provide the --ignore-host
    parameter to bypass this check.
    
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
    Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
    Acked-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---
 tools/libxl/xl_cmdimpl.c  |   35 +++++++++++++++++++++++++++++------
 tools/libxl/xl_cmdtable.c |    3 ++-
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 497d84d..e13a64e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4487,7 +4487,7 @@ int main_vcpupin(int argc, char **argv)
     return 0;
 }
 
-static void vcpuset(uint32_t domid, const char* nr_vcpus)
+static void vcpuset(uint32_t domid, const char* nr_vcpus, int check_host)
 {
     char *endptr;
     unsigned int max_vcpus, i;
@@ -4499,7 +4499,22 @@ static void vcpuset(uint32_t domid, const char* nr_vcpus)
         return;
     }
 
-    if (libxl_cpu_bitmap_alloc(ctx, &cpumap, 0)) {
+    /*
+     * Maximum amount of vCPUS the guest is allowed to set is limited
+     * by the host's amount of pCPUs.
+     */
+    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);
+            return;
+        }
+        /* 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;
     }
@@ -4514,13 +4529,21 @@ static void vcpuset(uint32_t domid, const char* 
nr_vcpus)
 
 int main_vcpuset(int argc, char **argv)
 {
-    int opt;
+    static struct option opts[] = {
+        {"ignore-host", 0, 0, 'i'},
+        {0, 0, 0, 0}
+    };
+    int opt, check_host = 1;
 
-    SWITCH_FOREACH_OPT(opt, "", NULL, "vcpu-set", 2) {
-        /* No options */
+    SWITCH_FOREACH_OPT(opt, "i", opts, "vcpu-set", 2) {
+    case 'i':
+        check_host = 0;
+        break;
+    default:
+        break;
     }
 
-    vcpuset(find_domain(argv[optind]), argv[optind+1]);
+    vcpuset(find_domain(argv[optind]), argv[optind + 1], check_host);
     return 0;
 }
 
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 26f5025..44b42b0 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -217,7 +217,8 @@ struct cmd_spec cmd_table[] = {
     { "vcpu-set",
       &main_vcpuset, 0, 1,
       "Set the number of active VCPUs allowed for the domain",
-      "<Domain> <vCPUs>",
+      "[option] <Domain> <vCPUs>",
+      "-i, --ignore-host  Don't limit the vCPU based on the host CPU count",
     },
     { "vm-list",
       &main_vm_list, 0, 0,
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog