[PATCH v1 04/14] vircgroup: add virCgroupSetAndRetrieveCpuShares()

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

The code that calls virCgroupSetCpuShares() and virCgroupGetCpuShares()
is repeated in 4 different places. Let's put it in a new
virCgroupSetAndRetrieveCpuShares() 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@xxxxxxxxx>
---
 src/libvirt_private.syms |  1 +
 src/lxc/lxc_cgroup.c     |  6 +++---
 src/lxc/lxc_driver.c     |  7 +++----
 src/qemu/qemu_cgroup.c   |  7 ++++---
 src/qemu/qemu_driver.c   |  6 ++----
 src/util/vircgroup.c     | 21 +++++++++++++++++++++
 src/util/vircgroup.h     |  3 +++
 7 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 817f73fc98..6aa3f670db 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1710,6 +1710,7 @@ virCgroupNewSelf;
 virCgroupNewThread;
 virCgroupPathOfController;
 virCgroupRemove;
+virCgroupSetAndRetrieveCpuShares;
 virCgroupSetBlkioDeviceReadBps;
 virCgroupSetBlkioDeviceReadIops;
 virCgroupSetBlkioDeviceWeight;
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 3f15614c7f..2ccc1ae5a1 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -39,11 +39,11 @@ 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 (virCgroupSetAndRetrieveCpuShares(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 f7376188f0..51f1284d56 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1959,10 +1959,9 @@ 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 (virCgroupSetAndRetrieveCpuShares(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 0181e869fe..0ca62ba3ee 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -893,11 +893,12 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)
 
     if (vm->def->cputune.sharesSpecified) {
         unsigned long long val;
-        if (virCgroupSetCpuShares(priv->cgroup, vm->def->cputune.shares) < 0)
-            return -1;
 
-        if (virCgroupGetCpuShares(priv->cgroup, &val) < 0)
+        if (virCgroupSetAndRetrieveCpuShares(priv->cgroup,
+                                             vm->def->cputune.shares,
+                                             &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 2813f084cd..0d58893d7e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10667,10 +10667,8 @@ 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 (virCgroupSetAndRetrieveCpuShares(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 75037ff8dd..cfb34a0f0e 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3670,3 +3670,24 @@ 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
+virCgroupSetAndRetrieveCpuShares(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 0c84bb762b..ed8ee30a58 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -290,3 +290,6 @@ bool virCgroupControllerAvailable(int controller);
 int virCgroupSetupBlkioTune(virCgroupPtr cgroup, virDomainBlkiotune blkio);
 int virCgroupSetupMemtune(virCgroupPtr cgroup, virDomainMemtune mem);
 int virCgroupSetupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask);
+int virCgroupSetAndRetrieveCpuShares(virCgroupPtr cgroup,
+                                     unsigned long long shares,
+                                     unsigned long long *realValue);
-- 
2.24.1