vircgroup: enforce range limit for cpu.shares

Before the conversion to using systemd DBus API to set the cpu.shares
there was some magic conversion done by kernel which was documented in
virsh manpage as well. Now systemd errors out if the value is out of
range.

Since we enforce the range for other cpu cgroup attributes 'quota' and
'period' it makes sense to do the same for 'shares' as well.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 937b0da012293790a5f8273abb26ccbf46be8c93..b434ada8f081bab6ff280cf8a7c5ba64aa486ed6 100644 (file)
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -742,7 +742,8 @@ CPU Tuning
    the domain. If this is omitted, it defaults to the OS provided defaults. NB,
    There is no unit for the value, it's a relative measure based on the setting
    of other VM, e.g. A VM configured with value 2048 will get twice as much CPU
-   time as a VM configured with value 1024. :since:`Since 0.9.0`
+   time as a VM configured with value 1024. The value should be in range
+   [2, 262144]. :since:`Since 0.9.0`
 ``period``
    The optional ``period`` element specifies the enforcement interval (unit:
    microseconds). Within ``period``, each vCPU of the domain will not be allowed

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 8a4328faa0d9cbe528800639b0fc2d54fbcb992d..d44c36e2e9806c1e8d7a579aad0b7bf7e7b26bc3 100644 (file)
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -3807,10 +3807,7 @@ If *--config* is specified, affect the next start of a persistent guest.
 If *--current* is specified, it is equivalent to either *--live* or
 *--config*, depending on the current state of the guest.
 
-``Note``: The cpu_shares parameter has a valid value range of 0-262144; Negative
-values are wrapped to positive, and larger values are capped at the maximum.
-Therefore, -1 is a useful shorthand for 262144. On the Linux kernel, the
-values 0 and 1 are automatically converted to a minimal value of 2.
+``Note``: The cpu_shares parameter has a valid value range of 2-262144.
 
 ``Note``: The weight and cap parameters are defined only for the
 XEN_CREDIT scheduler.

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index b4e09e21feb8b05505781899bc50472fee860655..61cd0a07e5eddeb25f8b5cd7de12dd750b2ef8f1 100644 (file)
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1229,6 +1229,16 @@ virDomainDefOSValidate(const virDomainDef *def,
 static int
 virDomainDefCputuneValidate(const virDomainDef *def)
 {
+    if (def->cputune.shares > 0 &&
+        (def->cputune.shares < VIR_CGROUP_CPU_SHARES_MIN ||
+         def->cputune.shares > VIR_CGROUP_CPU_SHARES_MAX)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Value of cputune 'shares' must be in range [%llu, %llu]"),
+                         VIR_CGROUP_CPU_SHARES_MIN,
+                         VIR_CGROUP_CPU_SHARES_MAX);
+        return -1;
+    }
+
     CPUTUNE_VALIDATE_PERIOD(period);
     CPUTUNE_VALIDATE_PERIOD(global_period);
     CPUTUNE_VALIDATE_PERIOD(emulator_period);

diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index ec0902e3017d2d21d47c179ba99d19a4b3ad5133..7d9172d664f5d8e894896ec5643e5cd9a17afb54 100644 (file)
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -230,6 +230,8 @@ int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares);
 int virCgroupSetupCpuShares(virCgroupPtr cgroup, unsigned long long shares,
                             unsigned long long *realValue);
 
+#define VIR_CGROUP_CPU_SHARES_MIN 2LL
+#define VIR_CGROUP_CPU_SHARES_MAX 262144LL
 #define VIR_CGROUP_CPU_PERIOD_MIN 1000LL
 #define VIR_CGROUP_CPU_PERIOD_MAX 1000000LL
 #define VIR_CGROUP_CPU_QUOTA_MIN 1000LL

diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
index 79015ddaa4cbb166c554171d7eac8e766d1d2dbe..f8bdca4e78983e2f3546dbd272fc3baf684b88e6 100644 (file)
--- a/src/util/vircgroupv1.c
+++ b/src/util/vircgroupv1.c
@@ -1917,6 +1917,16 @@ static int
 virCgroupV1SetCpuShares(virCgroupPtr group,
                         unsigned long long shares)
 {
+    if (shares < VIR_CGROUP_CPU_SHARES_MIN ||
+        shares > VIR_CGROUP_CPU_SHARES_MAX) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("shares '%llu' must be in range [%llu, %llu]"),
+                       shares,
+                       VIR_CGROUP_CPU_SHARES_MIN,
+                       VIR_CGROUP_CPU_SHARES_MAX);
+        return -1;
+    }
+
     if (group->unitName) {
         GVariant *value = g_variant_new("t", shares);
 

diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index 24c7e64c677c7ca78bc06c1f2ead90ec4fc33a5f..669bf1b52e377978f18f94bfe2a23019b15908c9 100644 (file)
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -1505,6 +1505,16 @@ static int
 virCgroupV2SetCpuShares(virCgroupPtr group,
                         unsigned long long shares)
 {
+    if (shares < VIR_CGROUP_CPU_SHARES_MIN ||
+        shares > VIR_CGROUP_CPU_SHARES_MAX) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("shares '%llu' must be in range [%llu, %llu]"),
+                       shares,
+                       VIR_CGROUP_CPU_SHARES_MIN,
+                       VIR_CGROUP_CPU_SHARES_MAX);
+        return -1;
+    }
+
     if (group->unitName) {
         GVariant *value = g_variant_new("t", shares);