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@xxxxxxxxx> --- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 5 +---- src/lxc/lxc_driver.c | 6 ++---- src/qemu/qemu_cgroup.c | 5 ++--- src/qemu/qemu_driver.c | 5 +---- src/util/vircgroup.c | 20 ++++++++++++++++++++ src/util/vircgroup.h | 2 ++ 7 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a58a5ed78b..8be5fe7d9f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1731,6 +1731,7 @@ virCgroupSetupBlkioDeviceWeight; virCgroupSetupBlkioDeviceWriteBps; virCgroupSetupBlkioDeviceWriteIops; virCgroupSetupCpusetCpus; +virCgroupSetupCpuShares; virCgroupSupportsCpuBW; virCgroupTerminateMachine; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 618063bf43..4c8464bd97 100644 --- 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 439cc317c6..e66aa7b8f5 100644 --- 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 ee08b99064..f969469931 100644 --- 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 c1a79dcda6..e9feb50220 100644 --- 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 dcbd55c0f1..a1093e410d 100644 --- 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 55132dedb6..305d5c9853 100644 --- 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); -- 2.24.1