xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad@kernel.org>
To: George.Dunlap@eu.citrix.com, Ian.Campbell@citrix.com,
	xen-devel@lists.xensource.com, ian.jackson@eu.citrix.com
Subject: [RFC PATCH 1/3] xl: replace vcpu-set --ignore-host with --ignore-warn
Date: Fri, 19 Jul 2013 11:48:56 -0400	[thread overview]
Message-ID: <1374248938-11232-2-git-send-email-konrad.wilk@oracle.com> (raw)
In-Reply-To: <1374248938-11232-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. 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 removes said option and adds the --ignore-warn option.
By default the user is allowed to set as many vCPUs as they would like.
We will print out a warning if the value is higher than the physical
CPU count. The --ignore-warn will silence said warning.

Furthermore mention this parameter in the man-page and add
the 'WARNING' in the printf.

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

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 5975d7b..e09d330 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-warn>
+
+Ignore the warning if the amount of physical CPUs is lesser
+than the amount of virtual CPUs that is being set. There are
+(depending on the type of workload and guest OS) performance
+drawback 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 4a8feaf..3d23fb4 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4512,11 +4512,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, int ignore_warn)
 {
     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) {
@@ -4525,19 +4526,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 && !ignore_warn) {
+        fprintf(stderr, "WARNING: You are overcommmitting! You have %d" \
+                " physical CPUs and want %d vCPUs! Continuing, use" \
+                " --ignore-warn to silence this.\n", host_cpu, max_vcpus);
     }
     if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) {
         fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n");
@@ -4555,20 +4551,20 @@ static void vcpuset(uint32_t domid, const char* nr_vcpus, int check_host)
 int main_vcpuset(int argc, char **argv)
 {
     static struct option opts[] = {
-        {"ignore-host", 0, 0, 'i'},
+        {"ignore-warn", 0, 0, 'i'},
         {0, 0, 0, 0}
     };
-    int opt, check_host = 1;
+    int opt, ignore_warn = 0;
 
     SWITCH_FOREACH_OPT(opt, "i", opts, "vcpu-set", 2) {
     case 'i':
-        check_host = 0;
+        ignore_warn = 1;
         break;
     default:
         break;
     }
 
-    vcpuset(find_domain(argv[optind]), argv[optind + 1], check_host);
+    vcpuset(find_domain(argv[optind]), argv[optind + 1], ignore_warn);
     return 0;
 }
 
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 326a660..8a98c6a 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-warn  Ignore warning if there are more vCPUs that there are host CPUs",
     },
     { "vm-list",
       &main_vm_list, 0, 0,
-- 
1.7.7.6

  reply	other threads:[~2013-07-19 15:48 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-19 15:48 [RFC PATCH] Add the --ignore-warn parameter (v1) Konrad Rzeszutek Wilk
2013-07-19 15:48 ` Konrad Rzeszutek Wilk [this message]
2013-07-22 23:29   ` [RFC PATCH 1/3] xl: replace vcpu-set --ignore-host with --ignore-warn George Dunlap
2013-07-23 14:01     ` Konrad Rzeszutek Wilk
2013-07-23 17:52       ` George Dunlap
2013-07-23 19:17         ` Konrad Rzeszutek Wilk
2013-07-24 17:14           ` George Dunlap
2013-08-07 10:50             ` Ian Jackson
2013-09-25 20:32               ` Konrad Rzeszutek Wilk
2013-07-19 15:48 ` [RFC PATCH 2/3] xl/create: warn if the 'vcpu' parameter exceeds host physical CPUs Konrad Rzeszutek Wilk
2013-07-19 15:48 ` [RFC PATCH 3/3] xl/create: Sprinkle the check to silence warnings further Konrad Rzeszutek Wilk

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=1374248938-11232-2-git-send-email-konrad.wilk@oracle.com \
    --to=konrad@kernel.org \
    --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).