vircgroup.c: add virCgroupSetupCpuShares()

The code that calls virCgroupSetCpuShares() and virCgroupGetCpuShares()
is repeated in 4 different places. Let's put it in a new
virCgroupSetupCpuShares() to avoid code repetition.

There's a reason of why we execute a Get in the same value we
just executed Set, explained in detail by commit 97814d8ab3.
Let's add a gist of the reasoning behind it as a comment in
this new function as well.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0a791f8825c7b8cf64dbc021bbcce6810a8e0916..2580cad253bc76864cc2945bba22944dc3cf6e8f 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1732,6 +1732,7 @@ virCgroupSetupBlkioDeviceWeight;
 virCgroupSetupBlkioDeviceWriteBps;
 virCgroupSetupBlkioDeviceWriteIops;
 virCgroupSetupCpusetCpus;
+virCgroupSetupCpuShares;
 virCgroupSupportsCpuBW;
 virCgroupTerminateMachine;
 

diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 618063bf4398ee17cc0c00a2364d9573418c0b21..4c8464bd97c99198b739ea943c31db9c3f4a0936 100644 (file)
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -40,10 +40,7 @@ static int virLXCCgroupSetupCpuTune(virDomainDefPtr def,
 {
     if (def->cputune.sharesSpecified) {
         unsigned long long val;
-        if (virCgroupSetCpuShares(cgroup, def->cputune.shares) < 0)
-            return -1;
-
-        if (virCgroupGetCpuShares(cgroup, &val) < 0)
+        if (virCgroupSetupCpuShares(cgroup, def->cputune.shares, &val) < 0)
             return -1;
         def->cputune.shares = val;
     }

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 439cc317c6aeb03ddfdfa83e2f0c6566d51c13d6..e66aa7b8f506dd4d5f6bb3ef65dc12b3a8799d3e 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1959,10 +1959,8 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
         if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) {
             if (def) {
                 unsigned long long val;
-                if (virCgroupSetCpuShares(priv->cgroup, params[i].value.ul) < 0)
-                    goto endjob;
-
-                if (virCgroupGetCpuShares(priv->cgroup, &val) < 0)
+                if (virCgroupSetupCpuShares(priv->cgroup, params[i].value.ul,
+                                            &val) < 0)
                     goto endjob;
 
                 def->cputune.shares = val;

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ee08b990649c0d7936de76cb9dc61c37dd31d3b8..f969469931e9ffc32168a9d3ba4f3f610c6957a9 100644 (file)
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -894,11 +894,10 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)
 
     if (vm->def->cputune.sharesSpecified) {
         unsigned long long val;
-        if (virCgroupSetCpuShares(priv->cgroup, vm->def->cputune.shares) < 0)
+        if (virCgroupSetupCpuShares(priv->cgroup, vm->def->cputune.shares,
+                                    &val) < 0)
             return -1;
 
-        if (virCgroupGetCpuShares(priv->cgroup, &val) < 0)
-            return -1;
         if (vm->def->cputune.shares != val) {
             vm->def->cputune.shares = val;
             if (virTypedParamsAddULLong(&eventParams, &eventNparams,

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 411beef537934038ea50c13dd88037904cbd0f0b..06cf3e3c9461cece305c55326b0b982b9b6af32d 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10648,10 +10648,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
         if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) {
             if (def) {
                 unsigned long long val;
-                if (virCgroupSetCpuShares(priv->cgroup, value_ul) < 0)
-                    goto endjob;
-
-                if (virCgroupGetCpuShares(priv->cgroup, &val) < 0)
+                if (virCgroupSetupCpuShares(priv->cgroup, value_ul, &val) < 0)
                     goto endjob;
 
                 def->cputune.shares = val;

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index dcbd55c0f17a821436bf5229d47f274f85ec8eb6..a1093e410d4031ccd1bd74e76ec60c1f2741ee53 100644 (file)
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3681,3 +3681,23 @@ virCgroupSetupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask)
 
     return 0;
 }
+
+
+/* Per commit 97814d8ab3, the Linux kernel can consider a 'shares'
+ * value of '0' and '1' as 2, and any value larger than a maximum
+ * is reduced to maximum.
+ *
+ * The 'realValue' pointer holds the actual 'shares' value set by
+ * the kernel if the function returned success. */
+int
+virCgroupSetupCpuShares(virCgroupPtr cgroup, unsigned long long shares,
+                        unsigned long long *realValue)
+{
+    if (virCgroupSetCpuShares(cgroup, shares) < 0)
+        return -1;
+
+    if (virCgroupGetCpuShares(cgroup, realValue) < 0)
+        return -1;
+
+    return 0;
+}

diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 55132dedb6f8a63cf216518464ee39dfe5c46eb1..305d5c9853560a1d50749b506fb1597ed0f937f4 100644 (file)
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -222,6 +222,8 @@ virCgroupGetDomainTotalCpuStats(virCgroupPtr group,
 
 int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares);
 int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares);
+int virCgroupSetupCpuShares(virCgroupPtr cgroup, unsigned long long shares,
+                            unsigned long long *realValue);
 
 int virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period);
 int virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period);