xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
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>
Subject: [PATCH 1/2] xl: neuter vcpu-set --ignore-host.
Date: Wed, 25 Sep 2013 16:40:16 -0400	[thread overview]
Message-ID: <1380141617-8981-2-git-send-email-konrad.wilk@oracle.com> (raw)
In-Reply-To: <1380141617-8981-1-git-send-email-konrad.wilk@oracle.com>

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

  reply	other threads:[~2013-09-25 20:40 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-25 20:40 [RFC] Make xl vcpu-set work in overcommit and with PV guests. (v2) Konrad Rzeszutek Wilk
2013-09-25 20:40 ` Konrad Rzeszutek Wilk [this message]
2013-09-26  7:23   ` [PATCH 1/2] xl: neuter vcpu-set --ignore-host Dario Faggioli
2013-09-26 12:45     ` Konrad Rzeszutek Wilk
2013-09-26  9:06   ` Ian Campbell
2013-09-26 12:48     ` Konrad Rzeszutek Wilk
2013-09-26 16:25       ` George Dunlap
2013-09-27  1:44         ` Konrad Rzeszutek Wilk
2013-09-26 15:28     ` Konrad Rzeszutek Wilk
2013-09-26 15:47       ` Ian Campbell
2013-09-26 16:01         ` Andrew Cooper
2013-09-26 16:05           ` Ian Campbell
2013-09-27  1:52         ` Konrad Rzeszutek Wilk
2013-09-27  8:41           ` Ian Campbell
2013-09-30 18:40             ` Konrad Rzeszutek Wilk
2013-09-25 20:40 ` [PATCH 2/2] xl/vcpuset: Make it work for PV guests Konrad Rzeszutek Wilk
2013-09-26  9:10   ` Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1380141617-8981-2-git-send-email-konrad.wilk@oracle.com \
    --to=konrad.wilk@oracle.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).