From mboxrd@z Thu Jan  1 00:00:00 1970
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: [PATCH 1/2] xl: neuter vcpu-set --ignore-host.
Date: Wed, 25 Sep 2013 16:40:16 -0400
Message-ID: <1380141617-8981-2-git-send-email-konrad.wilk@oracle.com>
References: <1380141617-8981-1-git-send-email-konrad.wilk@oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1380141617-8981-1-git-send-email-konrad.wilk@oracle.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: George.Dunlap@eu.citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xensource.com, Ian.Campbell@citrix.com
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
List-Id: xen-devel@lists.xenproject.org

When Xen 4.3 was released we had a discussion whether we should
allow the vcpu-set command to allow the user to set more than
physical CPUs for a guest (it didn't). The author brought up:
 - Xend used to do it,
 - If a user wants to do it, let them do it,
 - The original author of the change did not realize the
   side-effect his patch caused this and had no intention of changing it.
 - The user can already boot a massively overcommitted guest by
   having a large 'vcpus=' value in the guest config and we allow
   that.

Since we were close to the release we added --ignore-host parameter
as a mechanism for a user to still set more vCPUs that the physical
machine as a stop-gate.

This patch keeps said option but neuters the check so that we
can overcommit. In other words - by default the user is
allowed to set as many vCPUs as they would like.

Furthermore mention this parameter change in the man-page.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 docs/man/xl.pod.1         | 15 ++++++++++++++-
 tools/libxl/xl_cmdimpl.c  | 28 ++++++++++++----------------
 tools/libxl/xl_cmdtable.c |  2 +-
 3 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 5975d7b..1199d01 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -597,7 +597,7 @@ This command is only available for HVM domains.
 Moves a domain out of the paused state.  This will allow a previously
 paused domain to now be eligible for scheduling by the Xen hypervisor.
 
-=item B<vcpu-set> I<domain-id> I<vcpu-count>
+=item B<vcpu-set> I<OPTION> I<domain-id> I<vcpu-count>
 
 Enables the I<vcpu-count> virtual CPUs for the domain in question.
 Like mem-set, this command can only allocate up to the maximum virtual
@@ -614,6 +614,19 @@ quietly ignored.
 Some guests may need to actually bring the newly added CPU online
 after B<vcpu-set>, go to B<SEE ALSO> section for information.
 
+B<OPTION>
+
+=over 4
+
+=item B<-i>, B<--ignore-host>
+
+Deprecated. Used to allow the user to increase the current number of
+active VCPUs, if it was greater than physical number of CPUs.
+This seatbelt option was introduced due to being (depending on the type
+of workload and guest OS) performance drawbacks of CPU overcommitting.
+
+=back
+
 =item B<vcpu-list> [I<domain-id>]
 
 Lists VCPU information for a specific domain.  If no domain is
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3d7eaad..ecab9a6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4536,11 +4536,12 @@ int main_vcpupin(int argc, char **argv)
     return 0;
 }
 
-static void vcpuset(uint32_t domid, const char* nr_vcpus, int check_host)
+static void vcpuset(uint32_t domid, const char* nr_vcpus)
 {
     char *endptr;
     unsigned int max_vcpus, i;
     libxl_bitmap cpumap;
+    unsigned int host_cpu;
 
     max_vcpus = strtoul(nr_vcpus, &endptr, 10);
     if (nr_vcpus == endptr) {
@@ -4549,19 +4550,14 @@ static void vcpuset(uint32_t domid, const char* nr_vcpus, int check_host)
     }
 
     /*
-     * Maximum amount of vCPUS the guest is allowed to set is limited
-     * by the host's amount of pCPUs.
+     * Warn if maximum amount of vCPUS the guest wants is higher than
+     * 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);
+    host_cpu = libxl_get_max_cpus(ctx);
+    if (max_vcpus > host_cpu) {
+        fprintf(stderr, "WARNING: You are overcommmitting! You have %d" \
+                " physical CPUs and want %d vCPUs! Continuing..\n",
+                host_cpu, max_vcpus);
     }
     if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) {
         fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n");
@@ -4582,17 +4578,17 @@ int main_vcpuset(int argc, char **argv)
         {"ignore-host", 0, 0, 'i'},
         {0, 0, 0, 0}
     };
-    int opt, check_host = 1;
+    int opt;
 
     SWITCH_FOREACH_OPT(opt, "i", opts, "vcpu-set", 2) {
     case 'i':
-        check_host = 0;
+        /* deprecated. */;
         break;
     default:
         break;
     }
 
-    vcpuset(find_domain(argv[optind]), argv[optind + 1], check_host);
+    vcpuset(find_domain(argv[optind]), argv[optind + 1]);
     return 0;
 }
 
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 326a660..2ed9715 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -219,7 +219,7 @@ struct cmd_spec cmd_table[] = {
       &main_vcpuset, 0, 1,
       "Set the number of active VCPUs allowed for the domain",
       "[option] <Domain> <vCPUs>",
-      "-i, --ignore-host  Don't limit the vCPU based on the host CPU count",
+      "-i, --ignore-host  Don't limit the vCPU based on the host CPU count (deprecated)",
     },
     { "vm-list",
       &main_vm_list, 0, 0,
-- 
1.8.3.1