[libvirt PATCH 0/4] cgroup cpu period and quota fixes
Pavel Hrdina (4): qemu: move cgroup cpu period and quota defines to vircgroup.h vircgroupv1: use defines for cpu period and quota limits vircgroupv2: use defines for cpu period and quota limits vircgroup: fix cpu quota maximum limit src/qemu/qemu_driver.c | 21 ++++++++------------- src/util/vircgroup.h | 7 +++++++ src/util/vircgroupv1.c | 23 ++++++++++++----------- src/util/vircgroupv2.c | 25 +++++++++++++------------ 4 files changed, 40 insertions(+), 36 deletions(-) -- 2.26.2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 21 ++++++++------------- src/util/vircgroup.h | 5 +++++ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 391596ba11..0e0a165e76 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -126,11 +126,6 @@ VIR_LOG_INIT("qemu.qemu_driver"); #define QEMU_NB_NUMA_PARAM 2 -#define QEMU_SCHED_MIN_PERIOD 1000LL -#define QEMU_SCHED_MAX_PERIOD 1000000LL -#define QEMU_SCHED_MIN_QUOTA 1000LL -#define QEMU_SCHED_MAX_QUOTA 18446744073709551LL - #define QEMU_GUEST_VCPU_MAX_ID 4096 #define QEMU_NB_BLKIO_PARAM 6 @@ -9364,7 +9359,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD)) { SCHED_RANGE_CHECK(value_ul, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD, - QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); + VIR_CGROUP_CPU_PERIOD_MIN, VIR_CGROUP_CPU_PERIOD_MAX); if (def && value_ul) { if ((rc = qemuSetVcpusBWLive(vm, priv->cgroup, value_ul, 0))) @@ -9384,7 +9379,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA)) { SCHED_RANGE_CHECK(value_l, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, - QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); + VIR_CGROUP_CPU_QUOTA_MIN, VIR_CGROUP_CPU_QUOTA_MAX); if (def && value_l) { if ((rc = qemuSetVcpusBWLive(vm, priv->cgroup, 0, value_l))) @@ -9404,7 +9399,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_GLOBAL_PERIOD)) { SCHED_RANGE_CHECK(value_ul, VIR_DOMAIN_SCHEDULER_GLOBAL_PERIOD, - QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); + VIR_CGROUP_CPU_PERIOD_MIN, VIR_CGROUP_CPU_PERIOD_MAX); if (def && value_ul) { if ((rc = qemuSetGlobalBWLive(priv->cgroup, value_ul, 0))) @@ -9424,7 +9419,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_GLOBAL_QUOTA)) { SCHED_RANGE_CHECK(value_l, VIR_DOMAIN_SCHEDULER_GLOBAL_QUOTA, - QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); + VIR_CGROUP_CPU_QUOTA_MIN, VIR_CGROUP_CPU_QUOTA_MAX); if (def && value_l) { if ((rc = qemuSetGlobalBWLive(priv->cgroup, 0, value_l))) @@ -9444,7 +9439,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD)) { SCHED_RANGE_CHECK(value_ul, VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, - QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); + VIR_CGROUP_CPU_PERIOD_MIN, VIR_CGROUP_CPU_PERIOD_MAX); if (def && value_ul) { if ((rc = qemuSetEmulatorBandwidthLive(priv->cgroup, @@ -9465,7 +9460,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA)) { SCHED_RANGE_CHECK(value_l, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, - QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); + VIR_CGROUP_CPU_QUOTA_MIN, VIR_CGROUP_CPU_QUOTA_MAX); if (def && value_l) { if ((rc = qemuSetEmulatorBandwidthLive(priv->cgroup, @@ -9486,7 +9481,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_IOTHREAD_PERIOD)) { SCHED_RANGE_CHECK(value_ul, VIR_DOMAIN_SCHEDULER_IOTHREAD_PERIOD, - QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); + VIR_CGROUP_CPU_PERIOD_MIN, VIR_CGROUP_CPU_PERIOD_MAX); if (def && value_ul) { if ((rc = qemuSetIOThreadsBWLive(vm, priv->cgroup, value_ul, 0))) @@ -9506,7 +9501,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_IOTHREAD_QUOTA)) { SCHED_RANGE_CHECK(value_l, VIR_DOMAIN_SCHEDULER_IOTHREAD_QUOTA, - QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); + VIR_CGROUP_CPU_QUOTA_MIN, VIR_CGROUP_CPU_QUOTA_MAX); if (def && value_l) { if ((rc = qemuSetIOThreadsBWLive(vm, priv->cgroup, 0, value_l))) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index f7eed983cc..2a9b341985 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -230,6 +230,11 @@ int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares); int virCgroupSetupCpuShares(virCgroupPtr cgroup, unsigned long long shares, unsigned long long *realValue); +#define VIR_CGROUP_CPU_PERIOD_MIN 1000LL +#define VIR_CGROUP_CPU_PERIOD_MAX 1000000LL +#define VIR_CGROUP_CPU_QUOTA_MIN 1000LL +#define VIR_CGROUP_CPU_QUOTA_MAX 18446744073709551LL + int virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period); int virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period); int virCgroupGetCpuPeriodQuota(virCgroupPtr cgroup, unsigned long long *period, -- 2.26.2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroupv1.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 984cd50409..06849efd38 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1881,13 +1881,13 @@ static int virCgroupV1SetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period) { - /* The cfs_period should be greater or equal than 1ms, and less or equal - * than 1s. - */ - if (cfs_period < 1000 || cfs_period > 1000000) { + if (cfs_period < VIR_CGROUP_CPU_PERIOD_MIN || + cfs_period > VIR_CGROUP_CPU_PERIOD_MAX) { virReportError(VIR_ERR_INVALID_ARG, - _("cfs_period '%llu' must be in range (1000, 1000000)"), - cfs_period); + _("cfs_period '%llu' must be in range (%llu, %llu)"), + cfs_period, + VIR_CGROUP_CPU_PERIOD_MIN, + VIR_CGROUP_CPU_PERIOD_MAX); return -1; } @@ -1911,13 +1911,14 @@ static int virCgroupV1SetCpuCfsQuota(virCgroupPtr group, long long cfs_quota) { - /* The cfs_quota should be greater or equal than 1ms */ if (cfs_quota >= 0 && - (cfs_quota < 1000 || - cfs_quota > ULLONG_MAX / 1000)) { + (cfs_quota < VIR_CGROUP_CPU_QUOTA_MIN || + cfs_quota > VIR_CGROUP_CPU_QUOTA_MAX)) { virReportError(VIR_ERR_INVALID_ARG, - _("cfs_quota '%lld' must be in range (1000, %llu)"), - cfs_quota, ULLONG_MAX / 1000); + _("cfs_quota '%lld' must be in range (%llu, %llu)"), + cfs_quota, + VIR_CGROUP_CPU_QUOTA_MIN, + VIR_CGROUP_CPU_QUOTA_MAX); return -1; } -- 2.26.2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroupv2.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 2b32f614e4..22da3a5c6a 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1482,12 +1482,12 @@ virCgroupV2SetCpuCfsPeriod(virCgroupPtr group, g_autofree char *str = NULL; char *tmp; - /* The cfs_period should be greater or equal than 1ms, and less or equal - * than 1s. - */ - if (cfs_period < 1000 || cfs_period > 1000000) { + if (cfs_period < VIR_CGROUP_CPU_PERIOD_MIN || + cfs_period > VIR_CGROUP_CPU_PERIOD_MAX) { virReportError(VIR_ERR_INVALID_ARG, - _("cfs_period '%llu' must be in range (1000, 1000000)"), + _("cfs_period '%llu' must be in range (%llu, %llu)"), + VIR_CGROUP_CPU_PERIOD_MIN, + VIR_CGROUP_CPU_PERIOD_MAX, cfs_period); return -1; } @@ -1543,17 +1543,18 @@ static int virCgroupV2SetCpuCfsQuota(virCgroupPtr group, long long cfs_quota) { - /* The cfs_quota should be greater or equal than 1ms */ if (cfs_quota >= 0 && - (cfs_quota < 1000 || - cfs_quota > ULLONG_MAX / 1000)) { + (cfs_quota < VIR_CGROUP_CPU_QUOTA_MIN || + cfs_quota > VIR_CGROUP_CPU_QUOTA_MAX)) { virReportError(VIR_ERR_INVALID_ARG, - _("cfs_quota '%lld' must be in range (1000, %llu)"), - cfs_quota, ULLONG_MAX / 1000); + _("cfs_quota '%lld' must be in range (%llu, %llu)"), + cfs_quota, + VIR_CGROUP_CPU_QUOTA_MIN, + VIR_CGROUP_CPU_QUOTA_MAX); return -1; } - if (cfs_quota == ULLONG_MAX / 1000) { + if (cfs_quota == VIR_CGROUP_CPU_QUOTA_MAX) { return virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_CPU, "cpu.max", "max"); @@ -1578,7 +1579,7 @@ virCgroupV2GetCpuCfsQuota(virCgroupPtr group, } if (STREQLEN(str, "max", 3)) { - *cfs_quota = ULLONG_MAX / 1000; + *cfs_quota = VIR_CGROUP_CPU_QUOTA_MAX; return 0; } -- 2.26.2
Kernel commit <d505b8af58912ae1e1a211fabc9995b19bd40828> added proper check for cpu quota maximum limit to prevent internal overflow. Even though this change is not present in all kernels it makes sense to enforce the same limit in libvirt. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1750315 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 2a9b341985..ec0902e301 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -233,7 +233,9 @@ int virCgroupSetupCpuShares(virCgroupPtr cgroup, unsigned long long shares, #define VIR_CGROUP_CPU_PERIOD_MIN 1000LL #define VIR_CGROUP_CPU_PERIOD_MAX 1000000LL #define VIR_CGROUP_CPU_QUOTA_MIN 1000LL -#define VIR_CGROUP_CPU_QUOTA_MAX 18446744073709551LL +/* Based on kernel code ((1ULL << MAX_BW_BITS) - 1) where MAX_BW_BITS is + * (64 - BW_SHIFT) and BW_SHIFT is 20 */ +#define VIR_CGROUP_CPU_QUOTA_MAX 17592186044415LL int virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period); int virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period); -- 2.26.2
On 11/25/20 11:37 AM, Pavel Hrdina wrote:
Pavel Hrdina (4): qemu: move cgroup cpu period and quota defines to vircgroup.h vircgroupv1: use defines for cpu period and quota limits vircgroupv2: use defines for cpu period and quota limits vircgroup: fix cpu quota maximum limit
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
src/qemu/qemu_driver.c | 21 ++++++++------------- src/util/vircgroup.h | 7 +++++++ src/util/vircgroupv1.c | 23 ++++++++++++----------- src/util/vircgroupv2.c | 25 +++++++++++++------------ 4 files changed, 40 insertions(+), 36 deletions(-)
participants (2)
-
Daniel Henrique Barboza -
Pavel Hrdina