The current use of the functions that sets and gets BlkioDevice attributes is doing a set(), following by a get() of the same parameter right after. This is done because there is no guarantee that the kernel will accept the desired value given by the set() call, thus we need to execute a get() right after to get the actual value. This patch adds helpers inside vircgroup.c to execute these operations. Next patch will use these helpers to reduce code repetition in LXC and QEMU files. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt_private.syms | 5 +++ src/util/vircgroup.c | 85 ++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 20 ++++++++++ 3 files changed, 110 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 125d1836dd..e26bfab101 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1730,6 +1730,11 @@ virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; virCgroupSetMemSwapHardLimit; virCgroupSetOwner; +virCgroupSetupBlkioDeviceReadBps; +virCgroupSetupBlkioDeviceReadIops; +virCgroupSetupBlkioDeviceWeight; +virCgroupSetupBlkioDeviceWriteBps; +virCgroupSetupBlkioDeviceWriteIops; virCgroupSupportsCpuBW; virCgroupTerminateMachine; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 0680ff7c24..302352aca3 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3581,3 +3581,88 @@ virCgroupDelThread(virCgroupPtr cgroup, return 0; } + + +/** + * Calls virCgroupSetBlkioDeviceWeight() to set up blkio device weight, + * then retrieves the actual value set by the kernel with + * virCgroupGetBlkioDeviceWeight() in the same @weight pointer. + */ +int +virCgroupSetupBlkioDeviceWeight(virCgroupPtr cgroup, const char *path, + unsigned int *weight) +{ + if (virCgroupSetBlkioDeviceWeight(cgroup, path, *weight) < 0 || + virCgroupGetBlkioDeviceWeight(cgroup, path, weight) < 0) + return -1; + + return 0; +} + + +/** + * Calls virCgroupSetBlkioDeviceReadIops() to set up blkio device riops, + * then retrieves the actual value set by the kernel with + * virCgroupGetBlkioDeviceReadIops() in the same @riops pointer. + */ +int +virCgroupSetupBlkioDeviceReadIops(virCgroupPtr cgroup, const char *path, + unsigned int *riops) +{ + if (virCgroupSetBlkioDeviceReadIops(cgroup, path, *riops) < 0 || + virCgroupGetBlkioDeviceReadIops(cgroup, path, riops) < 0) + return -1; + + return 0; +} + + +/** + * Calls virCgroupSetBlkioDeviceWriteIops() to set up blkio device wiops, + * then retrieves the actual value set by the kernel with + * virCgroupGetBlkioDeviceWriteIops() in the same @wiops pointer. + */ +int +virCgroupSetupBlkioDeviceWriteIops(virCgroupPtr cgroup, const char *path, + unsigned int *wiops) +{ + if (virCgroupSetBlkioDeviceWriteIops(cgroup, path, *wiops) < 0 || + virCgroupGetBlkioDeviceWriteIops(cgroup, path, wiops) < 0) + return -1; + + return 0; +} + + +/** + * Calls virCgroupSetBlkioDeviceReadBps() to set up blkio device rbps, + * then retrieves the actual value set by the kernel with + * virCgroupGetBlkioDeviceReadBps() in the same @rbps pointer. + */ +int +virCgroupSetupBlkioDeviceReadBps(virCgroupPtr cgroup, const char *path, + unsigned long long *rbps) +{ + if (virCgroupSetBlkioDeviceReadBps(cgroup, path, *rbps) < 0 || + virCgroupGetBlkioDeviceReadBps(cgroup, path, rbps) < 0) + return -1; + + return 0; +} + + +/** + * Calls virCgroupSetBlkioDeviceWriteBps() to set up blkio device wbps, + * then retrieves the actual value set by the kernel with + * virCgroupGetBlkioDeviceWriteBps() in the same @wbps pointer. + */ +int +virCgroupSetupBlkioDeviceWriteBps(virCgroupPtr cgroup, const char *path, + unsigned long long *wbps) +{ + if (virCgroupSetBlkioDeviceWriteBps(cgroup, path, *wbps) < 0 || + virCgroupGetBlkioDeviceWriteBps(cgroup, path, wbps) < 0) + return -1; + + return 0; +} diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 15263f534a..6bd3f4fe9d 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -175,6 +175,26 @@ int virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, const char *path, unsigned long long *wbps); +int virCgroupSetupBlkioDeviceWeight(virCgroupPtr cgroup, + const char *path, + unsigned int *weight); + +int virCgroupSetupBlkioDeviceReadIops(virCgroupPtr cgroup, + const char *path, + unsigned int *riops); + +int virCgroupSetupBlkioDeviceWriteIops(virCgroupPtr cgroup, + const char *path, + unsigned int *wiops); + +int virCgroupSetupBlkioDeviceReadBps(virCgroupPtr cgroup, + const char *path, + unsigned long long *rbps); + +int virCgroupSetupBlkioDeviceWriteBps(virCgroupPtr cgroup, + const char *path, + unsigned long long *wbps); + int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemoryStat(virCgroupPtr group, unsigned long long *cache, -- 2.24.1

There are code repetition of set() and get() blkio device parameters across lxc and qemu files. Use the new vircgroup helpers to trim the repetition a bit. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/lxc/lxc_cgroup.c | 30 ++++++++++-------------------- src/lxc/lxc_driver.c | 41 +++++++++++------------------------------ src/qemu/qemu_cgroup.c | 32 ++++++++++++-------------------- src/qemu/qemu_driver.c | 41 +++++++++++------------------------------ 4 files changed, 44 insertions(+), 100 deletions(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 7f3701593a..3c7e31c36b 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -112,38 +112,28 @@ static int virLXCCgroupSetupBlkioTune(virDomainDefPtr def, virBlkioDevicePtr dev = &def->blkio.devices[i]; if (dev->weight && - (virCgroupSetBlkioDeviceWeight(cgroup, dev->path, - dev->weight) < 0 || - virCgroupGetBlkioDeviceWeight(cgroup, dev->path, - &dev->weight) < 0)) + virCgroupSetupBlkioDeviceWeight(cgroup, dev->path, + &dev->weight) < 0) return -1; if (dev->riops && - (virCgroupSetBlkioDeviceReadIops(cgroup, dev->path, - dev->riops) < 0 || - virCgroupGetBlkioDeviceReadIops(cgroup, dev->path, - &dev->riops) < 0)) + virCgroupSetupBlkioDeviceReadIops(cgroup, dev->path, + &dev->riops) < 0) return -1; if (dev->wiops && - (virCgroupSetBlkioDeviceWriteIops(cgroup, dev->path, - dev->wiops) < 0 || - virCgroupGetBlkioDeviceWriteIops(cgroup, dev->path, - &dev->wiops) < 0)) + virCgroupSetupBlkioDeviceWriteIops(cgroup, dev->path, + &dev->wiops) < 0) return -1; if (dev->rbps && - (virCgroupSetBlkioDeviceReadBps(cgroup, dev->path, - dev->rbps) < 0 || - virCgroupGetBlkioDeviceReadBps(cgroup, dev->path, - &dev->rbps) < 0)) + virCgroupSetupBlkioDeviceReadBps(cgroup, dev->path, + &dev->rbps) < 0) return -1; if (dev->wbps && - (virCgroupSetBlkioDeviceWriteBps(cgroup, dev->path, - dev->wbps) < 0 || - virCgroupGetBlkioDeviceWriteBps(cgroup, dev->path, - &dev->wbps) < 0)) + virCgroupSetupBlkioDeviceWriteBps(cgroup, dev->path, + &dev->wbps) < 0) return -1; } } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f7376188f0..439cc317c6 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2581,6 +2581,7 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { + virCgroupPtr cgroup = priv->cgroup; size_t ndevices; virBlkioDevicePtr devices = NULL; size_t j; @@ -2595,60 +2596,40 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceWeight(priv->cgroup, - devices[j].path, - devices[j].weight) < 0 || - virCgroupGetBlkioDeviceWeight(priv->cgroup, - devices[j].path, - &devices[j].weight) < 0) { + if (virCgroupSetupBlkioDeviceWeight(cgroup, devices[j].path, + &devices[j].weight) < 0) { ret = -1; break; } } } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceReadIops(priv->cgroup, - devices[j].path, - devices[j].riops) < 0 || - virCgroupGetBlkioDeviceReadIops(priv->cgroup, - devices[j].path, - &devices[j].riops) < 0) { + if (virCgroupSetupBlkioDeviceReadIops(cgroup, devices[j].path, + &devices[j].riops) < 0) { ret = -1; break; } } } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceWriteIops(priv->cgroup, - devices[j].path, - devices[j].wiops) < 0 || - virCgroupGetBlkioDeviceWriteIops(priv->cgroup, - devices[j].path, - &devices[j].wiops) < 0) { + if (virCgroupSetupBlkioDeviceWriteIops(cgroup, devices[j].path, + &devices[j].wiops) < 0) { ret = -1; break; } } } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceReadBps(priv->cgroup, - devices[j].path, - devices[j].rbps) < 0 || - virCgroupGetBlkioDeviceReadBps(priv->cgroup, - devices[j].path, - &devices[j].rbps) < 0) { + if (virCgroupSetupBlkioDeviceReadBps(cgroup, devices[j].path, + &devices[j].rbps) < 0) { ret = -1; break; } } } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceWriteBps(priv->cgroup, - devices[j].path, - devices[j].wbps) < 0 || - virCgroupGetBlkioDeviceWriteBps(priv->cgroup, - devices[j].path, - &devices[j].wbps) < 0) { + if (virCgroupSetupBlkioDeviceWriteBps(cgroup, devices[j].path, + &devices[j].wbps) < 0) { ret = -1; break; } diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 45701b4c6e..da96a60a08 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -611,39 +611,31 @@ qemuSetupBlkioCgroup(virDomainObjPtr vm) if (vm->def->blkio.ndevices) { for (i = 0; i < vm->def->blkio.ndevices; i++) { virBlkioDevicePtr dev = &vm->def->blkio.devices[i]; + virCgroupPtr cgroup = priv->cgroup; + if (dev->weight && - (virCgroupSetBlkioDeviceWeight(priv->cgroup, dev->path, - dev->weight) < 0 || - virCgroupGetBlkioDeviceWeight(priv->cgroup, dev->path, - &dev->weight) < 0)) + virCgroupSetupBlkioDeviceWeight(cgroup, dev->path, + &dev->weight) < 0) return -1; if (dev->riops && - (virCgroupSetBlkioDeviceReadIops(priv->cgroup, dev->path, - dev->riops) < 0 || - virCgroupGetBlkioDeviceReadIops(priv->cgroup, dev->path, - &dev->riops) < 0)) + virCgroupSetupBlkioDeviceReadIops(cgroup, dev->path, + &dev->riops) < 0) return -1; if (dev->wiops && - (virCgroupSetBlkioDeviceWriteIops(priv->cgroup, dev->path, - dev->wiops) < 0 || - virCgroupGetBlkioDeviceWriteIops(priv->cgroup, dev->path, - &dev->wiops) < 0)) + virCgroupSetupBlkioDeviceWriteIops(cgroup, dev->path, + &dev->wiops) < 0) return -1; if (dev->rbps && - (virCgroupSetBlkioDeviceReadBps(priv->cgroup, dev->path, - dev->rbps) < 0 || - virCgroupGetBlkioDeviceReadBps(priv->cgroup, dev->path, - &dev->rbps) < 0)) + virCgroupSetupBlkioDeviceReadBps(cgroup, dev->path, + &dev->rbps) < 0) return -1; if (dev->wbps && - (virCgroupSetBlkioDeviceWriteBps(priv->cgroup, dev->path, - dev->wbps) < 0 || - virCgroupGetBlkioDeviceWriteBps(priv->cgroup, dev->path, - &dev->wbps) < 0)) + virCgroupSetupBlkioDeviceWriteBps(cgroup, dev->path, + &dev->wbps) < 0) return -1; } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f686b858cf..c1a79dcda6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9564,6 +9564,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { + virCgroupPtr cgroup = priv->cgroup; size_t ndevices; virBlkioDevicePtr devices = NULL; size_t j; @@ -9578,60 +9579,40 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceWeight(priv->cgroup, - devices[j].path, - devices[j].weight) < 0 || - virCgroupGetBlkioDeviceWeight(priv->cgroup, - devices[j].path, - &devices[j].weight) < 0) { + if (virCgroupSetupBlkioDeviceWeight(cgroup, devices[j].path, + &devices[j].weight) < 0) { ret = -1; break; } } } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceReadIops(priv->cgroup, - devices[j].path, - devices[j].riops) < 0 || - virCgroupGetBlkioDeviceReadIops(priv->cgroup, - devices[j].path, - &devices[j].riops) < 0) { + if (virCgroupSetupBlkioDeviceReadIops(cgroup, devices[j].path, + &devices[j].riops) < 0) { ret = -1; break; } } } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceWriteIops(priv->cgroup, - devices[j].path, - devices[j].wiops) < 0 || - virCgroupGetBlkioDeviceWriteIops(priv->cgroup, - devices[j].path, - &devices[j].wiops) < 0) { + if (virCgroupSetupBlkioDeviceWriteIops(cgroup, devices[j].path, + &devices[j].wiops) < 0) { ret = -1; break; } } } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceReadBps(priv->cgroup, - devices[j].path, - devices[j].rbps) < 0 || - virCgroupGetBlkioDeviceReadBps(priv->cgroup, - devices[j].path, - &devices[j].rbps) < 0) { + if (virCgroupSetupBlkioDeviceReadBps(cgroup, devices[j].path, + &devices[j].rbps) < 0) { ret = -1; break; } } } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceWriteBps(priv->cgroup, - devices[j].path, - devices[j].wbps) < 0 || - virCgroupGetBlkioDeviceWriteBps(priv->cgroup, - devices[j].path, - &devices[j].wbps) < 0) { + if (virCgroupSetupBlkioDeviceWriteBps(cgroup, devices[j].path, + &devices[j].wbps) < 0) { ret = -1; break; } -- 2.24.1

Previous patch moved all duplicated code that were setting and getting BlkioDevice parameters to vircgroup.c. We can turn them into static and spare a few symbols in libvirt_private.syms. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt_private.syms | 10 ---------- src/util/vircgroup.c | 40 ++++++++++++++++++++-------------------- src/util/vircgroup.h | 40 ---------------------------------------- 3 files changed, 20 insertions(+), 70 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e26bfab101..2bbfa4bd9d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1671,11 +1671,6 @@ virCgroupDenyAllDevices; virCgroupDenyDevice; virCgroupDenyDevicePath; virCgroupFree; -virCgroupGetBlkioDeviceReadBps; -virCgroupGetBlkioDeviceReadIops; -virCgroupGetBlkioDeviceWeight; -virCgroupGetBlkioDeviceWriteBps; -virCgroupGetBlkioDeviceWriteIops; virCgroupGetBlkioIoDeviceServiced; virCgroupGetBlkioIoServiced; virCgroupGetBlkioWeight; @@ -1712,11 +1707,6 @@ virCgroupNewSelf; virCgroupNewThread; virCgroupPathOfController; virCgroupRemove; -virCgroupSetBlkioDeviceReadBps; -virCgroupSetBlkioDeviceReadIops; -virCgroupSetBlkioDeviceWeight; -virCgroupSetBlkioDeviceWriteBps; -virCgroupSetBlkioDeviceWriteIops; virCgroupSetBlkioWeight; virCgroupSetCpuCfsPeriod; virCgroupSetCpuCfsQuota; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 302352aca3..261e63f6ec 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1415,7 +1415,7 @@ virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) * * Returns: 0 on success, -1 on error */ -int +static int virCgroupSetBlkioDeviceReadIops(virCgroupPtr group, const char *path, unsigned int riops) @@ -1433,7 +1433,7 @@ virCgroupSetBlkioDeviceReadIops(virCgroupPtr group, * * Returns: 0 on success, -1 on error */ -int +static int virCgroupSetBlkioDeviceWriteIops(virCgroupPtr group, const char *path, unsigned int wiops) @@ -1451,7 +1451,7 @@ virCgroupSetBlkioDeviceWriteIops(virCgroupPtr group, * * Returns: 0 on success, -1 on error */ -int +static int virCgroupSetBlkioDeviceReadBps(virCgroupPtr group, const char *path, unsigned long long rbps) @@ -1468,7 +1468,7 @@ virCgroupSetBlkioDeviceReadBps(virCgroupPtr group, * * Returns: 0 on success, -1 on error */ -int +static int virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group, const char *path, unsigned long long wbps) @@ -1487,7 +1487,7 @@ virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group, * * Returns: 0 on success, -1 on error */ -int +static int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, const char *path, unsigned int weight) @@ -1504,7 +1504,7 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group, * * Returns: 0 on success, -1 on error */ -int +static int virCgroupGetBlkioDeviceReadIops(virCgroupPtr group, const char *path, unsigned int *riops) @@ -1521,7 +1521,7 @@ virCgroupGetBlkioDeviceReadIops(virCgroupPtr group, * * Returns: 0 on success, -1 on error */ -int +static int virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group, const char *path, unsigned int *wiops) @@ -1538,7 +1538,7 @@ virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group, * * Returns: 0 on success, -1 on error */ -int +static int virCgroupGetBlkioDeviceReadBps(virCgroupPtr group, const char *path, unsigned long long *rbps) @@ -1555,7 +1555,7 @@ virCgroupGetBlkioDeviceReadBps(virCgroupPtr group, * * Returns: 0 on success, -1 on error */ -int +static int virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, const char *path, unsigned long long *wbps) @@ -1572,7 +1572,7 @@ virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, * * Returns: 0 on success, -1 on error */ -int +static int virCgroupGetBlkioDeviceWeight(virCgroupPtr group, const char *path, unsigned int *weight) @@ -2993,7 +2993,7 @@ virCgroupGetBlkioWeight(virCgroupPtr group G_GNUC_UNUSED, } -int +static int virCgroupSetBlkioDeviceWeight(virCgroupPtr group G_GNUC_UNUSED, const char *path G_GNUC_UNUSED, unsigned int weight G_GNUC_UNUSED) @@ -3003,7 +3003,7 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group G_GNUC_UNUSED, return -1; } -int +static int virCgroupSetBlkioDeviceReadIops(virCgroupPtr group G_GNUC_UNUSED, const char *path G_GNUC_UNUSED, unsigned int riops G_GNUC_UNUSED) @@ -3013,7 +3013,7 @@ virCgroupSetBlkioDeviceReadIops(virCgroupPtr group G_GNUC_UNUSED, return -1; } -int +static int virCgroupSetBlkioDeviceWriteIops(virCgroupPtr group G_GNUC_UNUSED, const char *path G_GNUC_UNUSED, unsigned int wiops G_GNUC_UNUSED) @@ -3023,7 +3023,7 @@ virCgroupSetBlkioDeviceWriteIops(virCgroupPtr group G_GNUC_UNUSED, return -1; } -int +static int virCgroupSetBlkioDeviceReadBps(virCgroupPtr group G_GNUC_UNUSED, const char *path G_GNUC_UNUSED, unsigned long long rbps G_GNUC_UNUSED) @@ -3033,7 +3033,7 @@ virCgroupSetBlkioDeviceReadBps(virCgroupPtr group G_GNUC_UNUSED, return -1; } -int +static int virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group G_GNUC_UNUSED, const char *path G_GNUC_UNUSED, unsigned long long wbps G_GNUC_UNUSED) @@ -3043,7 +3043,7 @@ virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group G_GNUC_UNUSED, return -1; } -int +static int virCgroupGetBlkioDeviceWeight(virCgroupPtr group G_GNUC_UNUSED, const char *path G_GNUC_UNUSED, unsigned int *weight G_GNUC_UNUSED) @@ -3053,7 +3053,7 @@ virCgroupGetBlkioDeviceWeight(virCgroupPtr group G_GNUC_UNUSED, return -1; } -int +static int virCgroupGetBlkioDeviceReadIops(virCgroupPtr group G_GNUC_UNUSED, const char *path G_GNUC_UNUSED, unsigned int *riops G_GNUC_UNUSED) @@ -3063,7 +3063,7 @@ virCgroupGetBlkioDeviceReadIops(virCgroupPtr group G_GNUC_UNUSED, return -1; } -int +static int virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group G_GNUC_UNUSED, const char *path G_GNUC_UNUSED, unsigned int *wiops G_GNUC_UNUSED) @@ -3073,7 +3073,7 @@ virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group G_GNUC_UNUSED, return -1; } -int +static int virCgroupGetBlkioDeviceReadBps(virCgroupPtr group G_GNUC_UNUSED, const char *path G_GNUC_UNUSED, unsigned long long *rbps G_GNUC_UNUSED) @@ -3083,7 +3083,7 @@ virCgroupGetBlkioDeviceReadBps(virCgroupPtr group G_GNUC_UNUSED, return -1; } -int +static int virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group G_GNUC_UNUSED, const char *path G_GNUC_UNUSED, unsigned long long *wbps G_GNUC_UNUSED) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 6bd3f4fe9d..d50a8d69d4 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -135,46 +135,6 @@ int virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group, long long *requests_read, long long *requests_write); -int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, - const char *path, - unsigned int weight); - -int virCgroupSetBlkioDeviceReadIops(virCgroupPtr group, - const char *path, - unsigned int riops); - -int virCgroupSetBlkioDeviceWriteIops(virCgroupPtr group, - const char *path, - unsigned int wiops); - -int virCgroupSetBlkioDeviceReadBps(virCgroupPtr group, - const char *path, - unsigned long long rbps); - -int virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group, - const char *path, - unsigned long long wbps); - -int virCgroupGetBlkioDeviceWeight(virCgroupPtr group, - const char *path, - unsigned int *weight); - -int virCgroupGetBlkioDeviceReadIops(virCgroupPtr group, - const char *path, - unsigned int *riops); - -int virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group, - const char *path, - unsigned int *wiops); - -int virCgroupGetBlkioDeviceReadBps(virCgroupPtr group, - const char *path, - unsigned long long *rbps); - -int virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, - const char *path, - unsigned long long *wbps); - int virCgroupSetupBlkioDeviceWeight(virCgroupPtr cgroup, const char *path, unsigned int *weight); -- 2.24.1

There is duplicated code between virt drivers that needs to be moved to avoid code repetition. In the case of duplicated code between lxc_cgroup.c and qemu_cgroup.c a common place would be utils/vircgroup.c. The problem is that this would introduce /conf related definitions that shouldn't be imported to vircgroup.c, which is supposed to be a place for utilitary cgroups functions only. And syntax-check would forbid it anyway due to cross-directory includes being used. An alternative would be to overload domain_conf.c, which already contains all the definitions required. But that file is already crowded with XML handling code and we wouldn't do any favors to it by putting more utilitary, non-XML parsing/formatting code there. In [1], Colesuggested a 'domain_cgroup' file to host common code between lxc_cgroup and qemu_cgroup, and Daniel suggested a 'src/hypervisor' dir to host these type of files. This patch introduces src/hypervisor/domain_cgroup.c and, to get started, introduces a new virDomainCgroupSetupBlkio() function to host shared code between virLXCCgroupSetupBlkioTune() and qemuSetupBlkioCgroup(). [1] https://www.redhat.com/archives/libvir-list/2019-December/msg00817.html Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/Makefile.am | 1 + src/hypervisor/Makefile.inc.am | 14 +++++++ src/hypervisor/domain_cgroup.c | 67 ++++++++++++++++++++++++++++++++++ src/hypervisor/domain_cgroup.h | 27 ++++++++++++++ src/libvirt_private.syms | 4 ++ src/lxc/Makefile.inc.am | 2 + src/lxc/lxc_cgroup.c | 40 +------------------- src/qemu/Makefile.inc.am | 1 + src/qemu/qemu_cgroup.c | 40 +------------------- 9 files changed, 120 insertions(+), 76 deletions(-) create mode 100644 src/hypervisor/Makefile.inc.am create mode 100644 src/hypervisor/domain_cgroup.c create mode 100644 src/hypervisor/domain_cgroup.h diff --git a/src/Makefile.am b/src/Makefile.am index 952dfdbb5f..12dd6b80e1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -108,6 +108,7 @@ include locking/Makefile.inc.am include admin/Makefile.inc.am include rpc/Makefile.inc.am include test/Makefile.inc.am +include hypervisor/Makefile.inc.am include esx/Makefile.inc.am include hyperv/Makefile.inc.am include vmx/Makefile.inc.am diff --git a/src/hypervisor/Makefile.inc.am b/src/hypervisor/Makefile.inc.am new file mode 100644 index 0000000000..961b4e2b95 --- /dev/null +++ b/src/hypervisor/Makefile.inc.am @@ -0,0 +1,14 @@ +# vim: filetype=automake + +HYPERVISOR_SOURCES = \ + hypervisor/domain_cgroup.h \ + hypervisor/domain_cgroup.c \ + $(NULL) + +noinst_LTLIBRARIES += libvirt_hypervisor.la +libvirt_la_BUILT_LIBADD += libvirt_hypervisor.la +libvirt_hypervisor_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) \ + $(NULL) +libvirt_hypervisor_la_SOURCES = $(HYPERVISOR_SOURCES) diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c new file mode 100644 index 0000000000..bef60f56c5 --- /dev/null +++ b/src/hypervisor/domain_cgroup.c @@ -0,0 +1,67 @@ +/* + * domain_cgroup.c: cgroup functions shared between hypervisor drivers + * + * Copyright IBM Corp. 2020 + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "domain_cgroup.h" + + +int +virDomainCgroupSetupBlkio(virCgroupPtr cgroup, virDomainBlkiotune blkio) +{ + size_t i; + + if (blkio.weight != 0 && + virCgroupSetBlkioWeight(cgroup, blkio.weight) < 0) + return -1; + + if (blkio.ndevices) { + for (i = 0; i < blkio.ndevices; i++) { + virBlkioDevicePtr dev = &blkio.devices[i]; + + if (dev->weight && + virCgroupSetupBlkioDeviceWeight(cgroup, dev->path, + &dev->weight) < 0) + return -1; + + if (dev->riops && + virCgroupSetupBlkioDeviceReadIops(cgroup, dev->path, + &dev->riops) < 0) + return -1; + + if (dev->wiops && + virCgroupSetupBlkioDeviceWriteIops(cgroup, dev->path, + &dev->wiops) < 0) + return -1; + + if (dev->rbps && + virCgroupSetupBlkioDeviceReadBps(cgroup, dev->path, + &dev->rbps) < 0) + return -1; + + if (dev->wbps && + virCgroupSetupBlkioDeviceWriteBps(cgroup, dev->path, + &dev->wbps) < 0) + return -1; + } + } + + return 0; +} diff --git a/src/hypervisor/domain_cgroup.h b/src/hypervisor/domain_cgroup.h new file mode 100644 index 0000000000..af244bd2d2 --- /dev/null +++ b/src/hypervisor/domain_cgroup.h @@ -0,0 +1,27 @@ +/* + * domain_cgroup.h: cgroup functions shared between hypervisor drivers + * + * Copyright IBM Corp. 2020 + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "vircgroup.h" +#include "domain_conf.h" + + +int virDomainCgroupSetupBlkio(virCgroupPtr cgroup, virDomainBlkiotune blkio); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2bbfa4bd9d..dc06b1ae24 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1389,6 +1389,10 @@ virSetConnectSecret; virSetConnectStorage; +# hypervisor/domain_cgroup.h +virDomainCgroupSetupBlkio; + + # libvirt_internal.h virConnectSupportsFeature; virDomainMigrateBegin3; diff --git a/src/lxc/Makefile.inc.am b/src/lxc/Makefile.inc.am index f69c1acff5..2fee607d3d 100644 --- a/src/lxc/Makefile.inc.am +++ b/src/lxc/Makefile.inc.am @@ -97,6 +97,7 @@ libvirt_driver_lxc_impl_la_CFLAGS = \ -I$(srcdir)/conf \ -I$(builddir)/lxc \ -I$(builddir)/rpc \ + -I$(srcdir)/hypervisor \ $(AM_CFLAGS) \ $(NULL) libvirt_driver_lxc_impl_la_LIBADD = \ @@ -221,6 +222,7 @@ libvirt_lxc_CFLAGS = \ -I$(srcdir)/conf \ -I$(builddir)/lxc \ -I$(builddir)/rpc \ + -I$(srcdir)/hypervisor \ $(AM_CFLAGS) \ $(PIE_CFLAGS) \ $(CAPNG_CFLAGS) \ diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 3c7e31c36b..96a89256a1 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -23,6 +23,7 @@ #include "lxc_cgroup.h" #include "lxc_container.h" +#include "domain_cgroup.h" #include "virfile.h" #include "virerror.h" #include "virlog.h" @@ -101,44 +102,7 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def, static int virLXCCgroupSetupBlkioTune(virDomainDefPtr def, virCgroupPtr cgroup) { - size_t i; - - if (def->blkio.weight && - virCgroupSetBlkioWeight(cgroup, def->blkio.weight) < 0) - return -1; - - if (def->blkio.ndevices) { - for (i = 0; i < def->blkio.ndevices; i++) { - virBlkioDevicePtr dev = &def->blkio.devices[i]; - - if (dev->weight && - virCgroupSetupBlkioDeviceWeight(cgroup, dev->path, - &dev->weight) < 0) - return -1; - - if (dev->riops && - virCgroupSetupBlkioDeviceReadIops(cgroup, dev->path, - &dev->riops) < 0) - return -1; - - if (dev->wiops && - virCgroupSetupBlkioDeviceWriteIops(cgroup, dev->path, - &dev->wiops) < 0) - return -1; - - if (dev->rbps && - virCgroupSetupBlkioDeviceReadBps(cgroup, dev->path, - &dev->rbps) < 0) - return -1; - - if (dev->wbps && - virCgroupSetupBlkioDeviceWriteBps(cgroup, dev->path, - &dev->wbps) < 0) - return -1; - } - } - - return 0; + return virDomainCgroupSetupBlkio(cgroup, def->blkio); } diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index b9c0c6ea9c..94a333f855 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -98,6 +98,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \ -I$(builddir)/access \ -I$(srcdir)/conf \ -I$(srcdir)/secret \ + -I$(srcdir)/hypervisor \ $(AM_CFLAGS) \ $(NULL) libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index da96a60a08..475c063823 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -30,6 +30,7 @@ #include "viralloc.h" #include "virerror.h" #include "domain_audit.h" +#include "domain_cgroup.h" #include "virscsi.h" #include "virstring.h" #include "virfile.h" @@ -591,7 +592,6 @@ static int qemuSetupBlkioCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - size_t i; if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { @@ -604,43 +604,7 @@ qemuSetupBlkioCgroup(virDomainObjPtr vm) } } - if (vm->def->blkio.weight != 0 && - virCgroupSetBlkioWeight(priv->cgroup, vm->def->blkio.weight) < 0) - return -1; - - if (vm->def->blkio.ndevices) { - for (i = 0; i < vm->def->blkio.ndevices; i++) { - virBlkioDevicePtr dev = &vm->def->blkio.devices[i]; - virCgroupPtr cgroup = priv->cgroup; - - if (dev->weight && - virCgroupSetupBlkioDeviceWeight(cgroup, dev->path, - &dev->weight) < 0) - return -1; - - if (dev->riops && - virCgroupSetupBlkioDeviceReadIops(cgroup, dev->path, - &dev->riops) < 0) - return -1; - - if (dev->wiops && - virCgroupSetupBlkioDeviceWriteIops(cgroup, dev->path, - &dev->wiops) < 0) - return -1; - - if (dev->rbps && - virCgroupSetupBlkioDeviceReadBps(cgroup, dev->path, - &dev->rbps) < 0) - return -1; - - if (dev->wbps && - virCgroupSetupBlkioDeviceWriteBps(cgroup, dev->path, - &dev->wbps) < 0) - return -1; - } - } - - return 0; + return virDomainCgroupSetupBlkio(priv->cgroup, vm->def->blkio); } -- 2.24.1

virLXCCgroupSetupMemTune() and qemuSetupMemoryCgroup() shares duplicated code that can be put in a new helper to avoid code repetition. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/hypervisor/domain_cgroup.c | 19 +++++++++++++++++++ src/hypervisor/domain_cgroup.h | 1 + src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 14 +------------- src/qemu/qemu_cgroup.c | 14 +------------- 5 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c index bef60f56c5..e60abd536c 100644 --- a/src/hypervisor/domain_cgroup.c +++ b/src/hypervisor/domain_cgroup.c @@ -65,3 +65,22 @@ virDomainCgroupSetupBlkio(virCgroupPtr cgroup, virDomainBlkiotune blkio) return 0; } + + +int +virDomainCgroupSetupMemtune(virCgroupPtr cgroup, virDomainMemtune mem) +{ + if (virMemoryLimitIsSet(mem.hard_limit)) + if (virCgroupSetMemoryHardLimit(cgroup, mem.hard_limit) < 0) + return -1; + + if (virMemoryLimitIsSet(mem.soft_limit)) + if (virCgroupSetMemorySoftLimit(cgroup, mem.soft_limit) < 0) + return -1; + + if (virMemoryLimitIsSet(mem.swap_hard_limit)) + if (virCgroupSetMemSwapHardLimit(cgroup, mem.swap_hard_limit) < 0) + return -1; + + return 0; +} diff --git a/src/hypervisor/domain_cgroup.h b/src/hypervisor/domain_cgroup.h index af244bd2d2..82ba47e85f 100644 --- a/src/hypervisor/domain_cgroup.h +++ b/src/hypervisor/domain_cgroup.h @@ -25,3 +25,4 @@ int virDomainCgroupSetupBlkio(virCgroupPtr cgroup, virDomainBlkiotune blkio); +int virDomainCgroupSetupMemtune(virCgroupPtr cgroup, virDomainMemtune mem); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dc06b1ae24..1bc5d17000 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1391,6 +1391,7 @@ virSetConnectStorage; # hypervisor/domain_cgroup.h virDomainCgroupSetupBlkio; +virDomainCgroupSetupMemtune; # libvirt_internal.h diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 96a89256a1..eac1ee1ee0 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -112,19 +112,7 @@ static int virLXCCgroupSetupMemTune(virDomainDefPtr def, if (virCgroupSetMemory(cgroup, virDomainDefGetMemoryInitial(def)) < 0) return -1; - if (virMemoryLimitIsSet(def->mem.hard_limit)) - if (virCgroupSetMemoryHardLimit(cgroup, def->mem.hard_limit) < 0) - return -1; - - if (virMemoryLimitIsSet(def->mem.soft_limit)) - if (virCgroupSetMemorySoftLimit(cgroup, def->mem.soft_limit) < 0) - return -1; - - if (virMemoryLimitIsSet(def->mem.swap_hard_limit)) - if (virCgroupSetMemSwapHardLimit(cgroup, def->mem.swap_hard_limit) < 0) - return -1; - - return 0; + return virDomainCgroupSetupMemtune(cgroup, def->mem); } diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 475c063823..0c2f5f1b25 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -625,19 +625,7 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm) } } - if (virMemoryLimitIsSet(vm->def->mem.hard_limit)) - if (virCgroupSetMemoryHardLimit(priv->cgroup, vm->def->mem.hard_limit) < 0) - return -1; - - if (virMemoryLimitIsSet(vm->def->mem.soft_limit)) - if (virCgroupSetMemorySoftLimit(priv->cgroup, vm->def->mem.soft_limit) < 0) - return -1; - - if (virMemoryLimitIsSet(vm->def->mem.swap_hard_limit)) - if (virCgroupSetMemSwapHardLimit(priv->cgroup, vm->def->mem.swap_hard_limit) < 0) - return -1; - - return 0; + return virDomainCgroupSetupMemtune(priv->cgroup, vm->def->mem); } -- 2.24.1

The code from qemuSetupCgroupCpusetCpus() and virLXCCgroupSetupCpusetTune() can be centralized in a new helper called virCgroupSetupCpusetCpus(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 11 +++-------- src/qemu/qemu_cgroup.c | 14 +------------- src/util/vircgroup.c | 15 +++++++++++++++ src/util/vircgroup.h | 1 + 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1bc5d17000..a58a5ed78b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1730,6 +1730,7 @@ virCgroupSetupBlkioDeviceReadIops; virCgroupSetupBlkioDeviceWeight; virCgroupSetupBlkioDeviceWriteBps; virCgroupSetupBlkioDeviceWriteIops; +virCgroupSetupCpusetCpus; virCgroupSupportsCpuBW; virCgroupTerminateMachine; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index eac1ee1ee0..618063bf43 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -69,14 +69,9 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def, virDomainNumatuneMemMode mode; if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO && - def->cpumask) { - if (!(mask = virBitmapFormat(def->cpumask))) - return -1; - - if (virCgroupSetCpusetCpus(cgroup, mask) < 0) - goto cleanup; - /* free mask to make sure we won't use it in a wrong way later */ - VIR_FREE(mask); + def->cpumask && + virCgroupSetupCpusetCpus(cgroup, def->cpumask) < 0) { + return -1; } if (virDomainNumatuneGetMode(def->numa, -1, &mode) < 0 || diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 0c2f5f1b25..ee08b99064 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1165,19 +1165,7 @@ int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask) { - int ret = -1; - char *new_cpus = NULL; - - if (!(new_cpus = virBitmapFormat(cpumask))) - goto cleanup; - - if (virCgroupSetCpusetCpus(cgroup, new_cpus) < 0) - goto cleanup; - - ret = 0; - cleanup: - VIR_FREE(new_cpus); - return ret; + return virCgroupSetupCpusetCpus(cgroup, cpumask); } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 261e63f6ec..dcbd55c0f1 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3666,3 +3666,18 @@ virCgroupSetupBlkioDeviceWriteBps(virCgroupPtr cgroup, const char *path, return 0; } + + +int +virCgroupSetupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask) +{ + g_autofree char *new_cpus = NULL; + + if (!(new_cpus = virBitmapFormat(cpumask))) + return -1; + + if (virCgroupSetCpusetCpus(cgroup, new_cpus) < 0) + return -1; + + return 0; +} diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index d50a8d69d4..55132dedb6 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -245,6 +245,7 @@ int virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate); int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus); int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus); +int virCgroupSetupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask); int virCgroupRemove(virCgroupPtr group); -- 2.24.1

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> --- 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

qemuSetupCgroupVcpuBW() and lxcSetVcpuBWLive() shares the same code to set CPU CFS period and quota. This code can be moved to a new virCgroupSetupCpuPeriodQuota() helper to avoid code repetition. A similar code is also executed in virLXCCgroupSetupCpuTune(), but without the rollback on error. Use the new helper in this function as well since the 'period' rollback, if not a straight improvement for virLXCCgroupSetupCpuTune(), is benign. And we end up cutting more code repetition. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 11 ++--------- src/lxc/lxc_driver.c | 32 +------------------------------- src/qemu/qemu_cgroup.c | 31 +------------------------------ src/util/vircgroup.c | 38 ++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 2 ++ 6 files changed, 45 insertions(+), 70 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8be5fe7d9f..2f958b1c6f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1730,6 +1730,7 @@ virCgroupSetupBlkioDeviceReadIops; virCgroupSetupBlkioDeviceWeight; virCgroupSetupBlkioDeviceWriteBps; virCgroupSetupBlkioDeviceWriteIops; +virCgroupSetupCpuPeriodQuota; virCgroupSetupCpusetCpus; virCgroupSetupCpuShares; virCgroupSupportsCpuBW; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 4c8464bd97..4ebe5ef467 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -45,15 +45,8 @@ static int virLXCCgroupSetupCpuTune(virDomainDefPtr def, def->cputune.shares = val; } - if (def->cputune.quota != 0 && - virCgroupSetCpuCfsQuota(cgroup, def->cputune.quota) < 0) - return -1; - - if (def->cputune.period != 0 && - virCgroupSetCpuCfsPeriod(cgroup, def->cputune.period) < 0) - return -1; - - return 0; + return virCgroupSetupCpuPeriodQuota(cgroup, def->cputune.period, + def->cputune.quota); } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e66aa7b8f5..d6d0f031a5 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1857,37 +1857,7 @@ lxcGetVcpuBWLive(virCgroupPtr cgroup, unsigned long long *period, static int lxcSetVcpuBWLive(virCgroupPtr cgroup, unsigned long long period, long long quota) { - unsigned long long old_period; - - if (period == 0 && quota == 0) - return 0; - - if (period) { - /* get old period, and we can rollback if set quota failed */ - if (virCgroupGetCpuCfsPeriod(cgroup, &old_period) < 0) - return -1; - - if (virCgroupSetCpuCfsPeriod(cgroup, period) < 0) - return -1; - } - - if (quota) { - if (virCgroupSetCpuCfsQuota(cgroup, quota) < 0) - goto error; - } - - return 0; - - error: - if (period) { - virErrorPtr saved; - - virErrorPreserveLast(&saved); - virCgroupSetCpuCfsPeriod(cgroup, old_period); - virErrorRestore(&saved); - } - - return -1; + return virCgroupSetupCpuPeriodQuota(cgroup, period, quota); } diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f969469931..548c5ec274 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1127,36 +1127,7 @@ qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, long long quota) { - unsigned long long old_period; - - if (period == 0 && quota == 0) - return 0; - - if (period) { - /* get old period, and we can rollback if set quota failed */ - if (virCgroupGetCpuCfsPeriod(cgroup, &old_period) < 0) - return -1; - - if (virCgroupSetCpuCfsPeriod(cgroup, period) < 0) - return -1; - } - - if (quota && - virCgroupSetCpuCfsQuota(cgroup, quota) < 0) - goto error; - - return 0; - - error: - if (period) { - virErrorPtr saved; - - virErrorPreserveLast(&saved); - ignore_value(virCgroupSetCpuCfsPeriod(cgroup, old_period)); - virErrorRestore(&saved); - } - - return -1; + return virCgroupSetupCpuPeriodQuota(cgroup, period, quota); } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index a1093e410d..1f853beb73 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3701,3 +3701,41 @@ virCgroupSetupCpuShares(virCgroupPtr cgroup, unsigned long long shares, return 0; } + + +int +virCgroupSetupCpuPeriodQuota(virCgroupPtr cgroup, + unsigned long long period, + long long quota) +{ + unsigned long long old_period; + + if (period == 0 && quota == 0) + return 0; + + if (period) { + /* get old period, and we can rollback if set quota failed */ + if (virCgroupGetCpuCfsPeriod(cgroup, &old_period) < 0) + return -1; + + if (virCgroupSetCpuCfsPeriod(cgroup, period) < 0) + return -1; + } + + if (quota && + virCgroupSetCpuCfsQuota(cgroup, quota) < 0) + goto error; + + return 0; + + error: + if (period) { + virErrorPtr saved; + + virErrorPreserveLast(&saved); + ignore_value(virCgroupSetCpuCfsPeriod(cgroup, old_period)); + virErrorRestore(&saved); + } + + return -1; +} diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 305d5c9853..cc53bae258 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -227,6 +227,8 @@ int virCgroupSetupCpuShares(virCgroupPtr cgroup, unsigned long long shares, int virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period); int virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period); +int virCgroupSetupCpuPeriodQuota(virCgroupPtr cgroup, unsigned long long period, + long long quota); int virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota); int virCgroupGetCpuCfsQuota(virCgroupPtr group, long long *cfs_quota); -- 2.24.1

lxcDomainMergeBlkioDevice() and qemuDomainMergeBlkioDevice() are the same functions. This duplicated code can't be put in the existing domain_cgroup.c since it's not cgroup related. This patch introduces a new src/hypervisor/domain_driver.c to host this more generic code that can be shared between virt drivers. This new file is then used to create a new helper called virDomainDeivceMergeBlkioDevice() to eliminate the code repetition mentioned above. Callers in LXC and QEMU files were updated. This change is a preliminary step for more code reduction of cgroup related code inside lxcDomainSetBlkioParameters() and qemuDomainSetBlkioParameters(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/hypervisor/Makefile.inc.am | 2 + src/hypervisor/domain_driver.c | 96 ++++++++++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 29 ++++++++++ src/libvirt_private.syms | 4 ++ src/lxc/lxc_driver.c | 84 ++++------------------------- src/qemu/qemu_driver.c | 83 ++++------------------------- 6 files changed, 149 insertions(+), 149 deletions(-) create mode 100644 src/hypervisor/domain_driver.c create mode 100644 src/hypervisor/domain_driver.h diff --git a/src/hypervisor/Makefile.inc.am b/src/hypervisor/Makefile.inc.am index 961b4e2b95..02cf2c7cb1 100644 --- a/src/hypervisor/Makefile.inc.am +++ b/src/hypervisor/Makefile.inc.am @@ -3,6 +3,8 @@ HYPERVISOR_SOURCES = \ hypervisor/domain_cgroup.h \ hypervisor/domain_cgroup.c \ + hypervisor/domain_driver.h \ + hypervisor/domain_driver.c \ $(NULL) noinst_LTLIBRARIES += libvirt_hypervisor.la diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c new file mode 100644 index 0000000000..c999458c25 --- /dev/null +++ b/src/hypervisor/domain_driver.c @@ -0,0 +1,96 @@ +/* + * domain_driver.c: general functions shared between hypervisor drivers + * + * Copyright IBM Corp. 2020 + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "domain_driver.h" +#include "viralloc.h" + +#define VIR_FROM_THIS VIR_FROM_DOMAIN + + +/* Modify dest_array to reflect all blkio device changes described in + * src_array. */ +int +virDomainDriverMergeBlkioDevice(virBlkioDevicePtr *dest_array, + size_t *dest_size, + virBlkioDevicePtr src_array, + size_t src_size, + const char *type) +{ + size_t i, j; + virBlkioDevicePtr dest, src; + + for (i = 0; i < src_size; i++) { + bool found = false; + + src = &src_array[i]; + for (j = 0; j < *dest_size; j++) { + dest = &(*dest_array)[j]; + if (STREQ(src->path, dest->path)) { + found = true; + + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { + dest->weight = src->weight; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { + dest->riops = src->riops; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { + dest->wiops = src->wiops; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { + dest->rbps = src->rbps; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { + dest->wbps = src->wbps; + } else { + virReportError(VIR_ERR_INVALID_ARG, _("Unknown parameter %s"), + type); + return -1; + } + break; + } + } + if (!found) { + if (!src->weight && !src->riops && !src->wiops && !src->rbps && !src->wbps) + continue; + if (VIR_EXPAND_N(*dest_array, *dest_size, 1) < 0) + return -1; + dest = &(*dest_array)[*dest_size - 1]; + + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { + dest->weight = src->weight; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { + dest->riops = src->riops; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { + dest->wiops = src->wiops; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { + dest->rbps = src->rbps; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { + dest->wbps = src->wbps; + } else { + *dest_size = *dest_size - 1; + return -1; + } + + dest->path = src->path; + src->path = NULL; + } + } + + return 0; +} diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h new file mode 100644 index 0000000000..2fb4148dff --- /dev/null +++ b/src/hypervisor/domain_driver.h @@ -0,0 +1,29 @@ +/* + * domain_driver.h: general functions shared between hypervisor drivers + * + * Copyright IBM Corp. 2020 + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "domain_conf.h" + +int virDomainDriverMergeBlkioDevice(virBlkioDevicePtr *dest_array, + size_t *dest_size, + virBlkioDevicePtr src_array, + size_t src_size, + const char *type); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2f958b1c6f..c6b9afe259 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1394,6 +1394,10 @@ virDomainCgroupSetupBlkio; virDomainCgroupSetupMemtune; +# hypervisor/domain_cgroup.h +virDomainDriverMergeBlkioDevice; + + # libvirt_internal.h virConnectSupportsFeature; virDomainMigrateBegin3; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index d6d0f031a5..08c744a69b 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -56,6 +56,7 @@ #include "virpidfile.h" #include "virfdstream.h" #include "domain_audit.h" +#include "domain_driver.h" #include "domain_nwfilter.h" #include "virinitctl.h" #include "virnetdev.h" @@ -2214,75 +2215,6 @@ lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, return -1; } -static int -lxcDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array, - size_t *dest_size, - virBlkioDevicePtr src_array, - size_t src_size, - const char *type) -{ - size_t i, j; - virBlkioDevicePtr dest, src; - - for (i = 0; i < src_size; i++) { - bool found = false; - - src = &src_array[i]; - for (j = 0; j < *dest_size; j++) { - dest = &(*dest_array)[j]; - if (STREQ(src->path, dest->path)) { - found = true; - - if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { - dest->weight = src->weight; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { - dest->riops = src->riops; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { - dest->wiops = src->wiops; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { - dest->rbps = src->rbps; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - dest->wbps = src->wbps; - } else { - virReportError(VIR_ERR_INVALID_ARG, _("Unknown parameter %s"), - type); - return -1; - } - - break; - } - } - if (!found) { - if (!src->weight && !src->riops && !src->wiops && !src->rbps && !src->wbps) - continue; - if (VIR_EXPAND_N(*dest_array, *dest_size, 1) < 0) - return -1; - dest = &(*dest_array)[*dest_size - 1]; - - if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { - dest->weight = src->weight; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { - dest->riops = src->riops; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { - dest->wiops = src->wiops; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { - dest->rbps = src->rbps; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - dest->wbps = src->wbps; - } else { - *dest_size = *dest_size - 1; - return -1; - } - - dest->path = src->path; - src->path = NULL; - } - } - - return 0; -} - - static int lxcDomainBlockStats(virDomainPtr dom, const char *path, @@ -2613,9 +2545,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, } if (j != ndevices || - lxcDomainMergeBlkioDevice(&def->blkio.devices, - &def->blkio.ndevices, - devices, ndevices, param->field) < 0) + virDomainDriverMergeBlkioDevice(&def->blkio.devices, + &def->blkio.ndevices, + devices, ndevices, + param->field) < 0) ret = -1; virBlkioDeviceArrayClear(devices, ndevices); VIR_FREE(devices); @@ -2645,9 +2578,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, ret = -1; continue; } - if (lxcDomainMergeBlkioDevice(&persistentDef->blkio.devices, - &persistentDef->blkio.ndevices, - devices, ndevices, param->field) < 0) + if (virDomainDriverMergeBlkioDevice(&persistentDef->blkio.devices, + &persistentDef->blkio.ndevices, + devices, ndevices, + param->field) < 0) ret = -1; virBlkioDeviceArrayClear(devices, ndevices); VIR_FREE(devices); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e9feb50220..3048e8a5c7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -65,6 +65,7 @@ #include "viruuid.h" #include "domain_conf.h" #include "domain_audit.h" +#include "domain_driver.h" #include "node_device_conf.h" #include "virpci.h" #include "virusb.h" @@ -9419,74 +9420,6 @@ qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, return -1; } -/* Modify dest_array to reflect all blkio device changes described in - * src_array. */ -static int -qemuDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array, - size_t *dest_size, - virBlkioDevicePtr src_array, - size_t src_size, - const char *type) -{ - size_t i, j; - virBlkioDevicePtr dest, src; - - for (i = 0; i < src_size; i++) { - bool found = false; - - src = &src_array[i]; - for (j = 0; j < *dest_size; j++) { - dest = &(*dest_array)[j]; - if (STREQ(src->path, dest->path)) { - found = true; - - if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { - dest->weight = src->weight; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { - dest->riops = src->riops; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { - dest->wiops = src->wiops; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { - dest->rbps = src->rbps; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - dest->wbps = src->wbps; - } else { - virReportError(VIR_ERR_INVALID_ARG, _("Unknown parameter %s"), - type); - return -1; - } - break; - } - } - if (!found) { - if (!src->weight && !src->riops && !src->wiops && !src->rbps && !src->wbps) - continue; - if (VIR_EXPAND_N(*dest_array, *dest_size, 1) < 0) - return -1; - dest = &(*dest_array)[*dest_size - 1]; - - if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { - dest->weight = src->weight; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { - dest->riops = src->riops; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { - dest->wiops = src->wiops; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { - dest->rbps = src->rbps; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - dest->wbps = src->wbps; - } else { - *dest_size = *dest_size - 1; - return -1; - } - - dest->path = src->path; - src->path = NULL; - } - } - - return 0; -} static int qemuDomainSetBlkioParameters(virDomainPtr dom, @@ -9628,9 +9561,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, } if (j != ndevices || - qemuDomainMergeBlkioDevice(&def->blkio.devices, - &def->blkio.ndevices, - devices, ndevices, param->field) < 0) + virDomainDriverMergeBlkioDevice(&def->blkio.devices, + &def->blkio.ndevices, + devices, ndevices, + param->field) < 0) ret = -1; virBlkioDeviceArrayClear(devices, ndevices); VIR_FREE(devices); @@ -9663,9 +9597,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, ret = -1; continue; } - if (qemuDomainMergeBlkioDevice(&persistentDef->blkio.devices, - &persistentDef->blkio.ndevices, - devices, ndevices, param->field) < 0) + if (virDomainDriverMergeBlkioDevice(&persistentDef->blkio.devices, + &persistentDef->blkio.ndevices, + devices, ndevices, + param->field) < 0) ret = -1; virBlkioDeviceArrayClear(devices, ndevices); VIR_FREE(devices); -- 2.24.1

lxcDomainParseBlkioDeviceStr() and qemuDomainParseBlkioDeviceStr() are the same function. Avoid code repetition by putting the code in a new helper. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- po/POTFILES.in | 1 + src/hypervisor/domain_driver.c | 112 +++++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 3 + src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 122 +++---------------------------- src/qemu/qemu_driver.c | 126 +++------------------------------ 6 files changed, 133 insertions(+), 232 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index dba0d3a12e..aa5c1fb6c7 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -78,6 +78,7 @@ @SRCDIR@/src/hyperv/hyperv_driver.c @SRCDIR@/src/hyperv/hyperv_util.c @SRCDIR@/src/hyperv/hyperv_wmi.c +@SRCDIR@/src/hypervisor/domain_driver.c @SRCDIR@/src/interface/interface_backend_netcf.c @SRCDIR@/src/interface/interface_backend_udev.c @SRCDIR@/src/internal.h diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index c999458c25..bbfadb3d9b 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -22,6 +22,7 @@ #include "domain_driver.h" #include "viralloc.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -94,3 +95,114 @@ virDomainDriverMergeBlkioDevice(virBlkioDevicePtr *dest_array, return 0; } + + +/* blkioDeviceStr in the form of /device/path,weight,/device/path,weight + * for example, /dev/disk/by-path/pci-0000:00:1f.2-scsi-0:0:0:0,800 + */ +int +virDomainDriverParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, + virBlkioDevicePtr *dev, size_t *size) +{ + char *temp; + int ndevices = 0; + int nsep = 0; + size_t i; + virBlkioDevicePtr result = NULL; + + *dev = NULL; + *size = 0; + + if (STREQ(blkioDeviceStr, "")) + return 0; + + temp = blkioDeviceStr; + while (temp) { + temp = strchr(temp, ','); + if (temp) { + temp++; + nsep++; + } + } + + /* A valid string must have even number of fields, hence an odd + * number of commas. */ + if (!(nsep & 1)) + goto parse_error; + + ndevices = (nsep + 1) / 2; + + if (VIR_ALLOC_N(result, ndevices) < 0) + return -1; + + i = 0; + temp = blkioDeviceStr; + while (temp) { + char *p = temp; + + /* device path */ + p = strchr(p, ','); + if (!p) + goto parse_error; + + result[i].path = g_strndup(temp, p - temp); + + /* value */ + temp = p + 1; + + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { + if (virStrToLong_uip(temp, &p, 10, &result[i].weight) < 0) + goto number_error; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { + if (virStrToLong_uip(temp, &p, 10, &result[i].riops) < 0) + goto number_error; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { + if (virStrToLong_uip(temp, &p, 10, &result[i].wiops) < 0) + goto number_error; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { + if (virStrToLong_ullp(temp, &p, 10, &result[i].rbps) < 0) + goto number_error; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { + if (virStrToLong_ullp(temp, &p, 10, &result[i].wbps) < 0) + goto number_error; + } else { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown parameter '%s'"), type); + goto cleanup; + } + + i++; + + if (*p == '\0') + break; + else if (*p != ',') + goto parse_error; + temp = p + 1; + } + + if (!i) + VIR_FREE(result); + + *dev = result; + *size = i; + + return 0; + + parse_error: + virReportError(VIR_ERR_INVALID_ARG, + _("unable to parse blkio device '%s' '%s'"), + type, blkioDeviceStr); + goto cleanup; + + number_error: + virReportError(VIR_ERR_INVALID_ARG, + _("invalid value '%s' for parameter '%s' of device '%s'"), + temp, type, result[i].path); + + cleanup: + if (result) { + virBlkioDeviceArrayClear(result, ndevices); + VIR_FREE(result); + } + return -1; +} diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index 2fb4148dff..b78401ea42 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -27,3 +27,6 @@ int virDomainDriverMergeBlkioDevice(virBlkioDevicePtr *dest_array, virBlkioDevicePtr src_array, size_t src_size, const char *type); + +int virDomainDriverParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, + virBlkioDevicePtr *dev, size_t *size); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c6b9afe259..88f9ca16d1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1396,6 +1396,7 @@ virDomainCgroupSetupMemtune; # hypervisor/domain_cgroup.h virDomainDriverMergeBlkioDevice; +virDomainDriverParseBlkioDeviceStr; # libvirt_internal.h diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 08c744a69b..78a0174277 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2108,112 +2108,6 @@ lxcDomainGetSchedulerParameters(virDomainPtr domain, return lxcDomainGetSchedulerParametersFlags(domain, params, nparams, 0); } -static int -lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, - virBlkioDevicePtr *dev, size_t *size) -{ - char *temp; - int ndevices = 0; - int nsep = 0; - size_t i; - virBlkioDevicePtr result = NULL; - - *dev = NULL; - *size = 0; - - if (STREQ(blkioDeviceStr, "")) - return 0; - - temp = blkioDeviceStr; - while (temp) { - temp = strchr(temp, ','); - if (temp) { - temp++; - nsep++; - } - } - - /* A valid string must have even number of fields, hence an odd - * number of commas. */ - if (!(nsep & 1)) - goto parse_error; - - ndevices = (nsep + 1) / 2; - - if (VIR_ALLOC_N(result, ndevices) < 0) - return -1; - - i = 0; - temp = blkioDeviceStr; - while (temp) { - char *p = temp; - - /* device path */ - p = strchr(p, ','); - if (!p) - goto parse_error; - - result[i].path = g_strndup(temp, p - temp); - - /* value */ - temp = p + 1; - - if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { - if (virStrToLong_uip(temp, &p, 10, &result[i].weight) < 0) - goto number_error; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { - if (virStrToLong_uip(temp, &p, 10, &result[i].riops) < 0) - goto number_error; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { - if (virStrToLong_uip(temp, &p, 10, &result[i].wiops) < 0) - goto number_error; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { - if (virStrToLong_ullp(temp, &p, 10, &result[i].rbps) < 0) - goto number_error; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - if (virStrToLong_ullp(temp, &p, 10, &result[i].wbps) < 0) - goto number_error; - } else { - virReportError(VIR_ERR_INVALID_ARG, - _("unknown parameter '%s'"), type); - goto cleanup; - } - - i++; - - if (*p == '\0') - break; - else if (*p != ',') - goto parse_error; - temp = p + 1; - } - - if (!i) - VIR_FREE(result); - - *dev = result; - *size = i; - - return 0; - - parse_error: - virReportError(VIR_ERR_INVALID_ARG, - _("unable to parse blkio device '%s' '%s'"), - type, blkioDeviceStr); - goto cleanup; - - number_error: - virReportError(VIR_ERR_INVALID_ARG, - _("invalid value '%s' for parameter '%s' of device '%s'"), - temp, type, result[i].path); - - cleanup: - if (result) { - virBlkioDeviceArrayClear(result, ndevices); - VIR_FREE(result); - } - return -1; -} static int lxcDomainBlockStats(virDomainPtr dom, @@ -2486,10 +2380,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, virBlkioDevicePtr devices = NULL; size_t j; - if (lxcDomainParseBlkioDeviceStr(params[i].value.s, - param->field, - &devices, - &ndevices) < 0) { + if (virDomainDriverParseBlkioDeviceStr(params[i].value.s, + param->field, + &devices, + &ndevices) < 0) { ret = -1; continue; } @@ -2571,10 +2465,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, virBlkioDevicePtr devices = NULL; size_t ndevices; - if (lxcDomainParseBlkioDeviceStr(params[i].value.s, - param->field, - &devices, - &ndevices) < 0) { + if (virDomainDriverParseBlkioDeviceStr(params[i].value.s, + param->field, + &devices, + &ndevices) < 0) { ret = -1; continue; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3048e8a5c7..2205dd1194 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9310,116 +9310,6 @@ static char *qemuDomainGetSchedulerType(virDomainPtr dom, return ret; } -/* blkioDeviceStr in the form of /device/path,weight,/device/path,weight - * for example, /dev/disk/by-path/pci-0000:00:1f.2-scsi-0:0:0:0,800 - */ -static int -qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, - virBlkioDevicePtr *dev, size_t *size) -{ - char *temp; - int ndevices = 0; - int nsep = 0; - size_t i; - virBlkioDevicePtr result = NULL; - - *dev = NULL; - *size = 0; - - if (STREQ(blkioDeviceStr, "")) - return 0; - - temp = blkioDeviceStr; - while (temp) { - temp = strchr(temp, ','); - if (temp) { - temp++; - nsep++; - } - } - - /* A valid string must have even number of fields, hence an odd - * number of commas. */ - if (!(nsep & 1)) - goto parse_error; - - ndevices = (nsep + 1) / 2; - - if (VIR_ALLOC_N(result, ndevices) < 0) - return -1; - - i = 0; - temp = blkioDeviceStr; - while (temp) { - char *p = temp; - - /* device path */ - p = strchr(p, ','); - if (!p) - goto parse_error; - - result[i].path = g_strndup(temp, p - temp); - - /* value */ - temp = p + 1; - - if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { - if (virStrToLong_uip(temp, &p, 10, &result[i].weight) < 0) - goto number_error; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { - if (virStrToLong_uip(temp, &p, 10, &result[i].riops) < 0) - goto number_error; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { - if (virStrToLong_uip(temp, &p, 10, &result[i].wiops) < 0) - goto number_error; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { - if (virStrToLong_ullp(temp, &p, 10, &result[i].rbps) < 0) - goto number_error; - } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - if (virStrToLong_ullp(temp, &p, 10, &result[i].wbps) < 0) - goto number_error; - } else { - virReportError(VIR_ERR_INVALID_ARG, - _("unknown parameter '%s'"), type); - goto cleanup; - } - - i++; - - if (*p == '\0') - break; - else if (*p != ',') - goto parse_error; - temp = p + 1; - } - - if (!i) - VIR_FREE(result); - - *dev = result; - *size = i; - - return 0; - - parse_error: - virReportError(VIR_ERR_INVALID_ARG, - _("unable to parse blkio device '%s' '%s'"), - type, blkioDeviceStr); - goto cleanup; - - number_error: - virReportError(VIR_ERR_INVALID_ARG, - _("invalid value '%s' for parameter '%s' of device '%s'"), - temp, type, result[i].path); - - cleanup: - if (result) { - virBlkioDeviceArrayClear(result, ndevices); - VIR_FREE(result); - } - return -1; -} - static int qemuDomainSetBlkioParameters(virDomainPtr dom, @@ -9502,10 +9392,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, virBlkioDevicePtr devices = NULL; size_t j; - if (qemuDomainParseBlkioDeviceStr(param->value.s, - param->field, - &devices, - &ndevices) < 0) { + if (virDomainDriverParseBlkioDeviceStr(param->value.s, + param->field, + &devices, + &ndevices) < 0) { ret = -1; continue; } @@ -9590,10 +9480,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, virBlkioDevicePtr devices = NULL; size_t ndevices; - if (qemuDomainParseBlkioDeviceStr(param->value.s, - param->field, - &devices, - &ndevices) < 0) { + if (virDomainDriverParseBlkioDeviceStr(param->value.s, + param->field, + &devices, + &ndevices) < 0) { ret = -1; continue; } -- 2.24.1

After the introduction of virDomainDriverMergeBlkioDevice() in a previous patch, it is now clear that lxcDomainSetBlkioParameters() and qemuDomainSetBlkioParameters() uses the same loop to set cgroup blkio parameter of a domain. Avoid the repetition by adding a new helper called virDomainCgroupSetupDomainBlkioParameters(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- po/POTFILES.in | 1 + src/hypervisor/domain_cgroup.c | 101 +++++++++++++++++++++++++++++++++ src/hypervisor/domain_cgroup.h | 4 ++ src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 87 +--------------------------- src/qemu/qemu_driver.c | 88 +--------------------------- 6 files changed, 113 insertions(+), 169 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index aa5c1fb6c7..1675c45206 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -78,6 +78,7 @@ @SRCDIR@/src/hyperv/hyperv_driver.c @SRCDIR@/src/hyperv/hyperv_util.c @SRCDIR@/src/hyperv/hyperv_wmi.c +@SRCDIR@/src/hypervisor/domain_cgroup.c @SRCDIR@/src/hypervisor/domain_driver.c @SRCDIR@/src/interface/interface_backend_netcf.c @SRCDIR@/src/interface/interface_backend_udev.c diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c index e60abd536c..32b07be3d6 100644 --- a/src/hypervisor/domain_cgroup.c +++ b/src/hypervisor/domain_cgroup.c @@ -21,6 +21,9 @@ #include <config.h> #include "domain_cgroup.h" +#include "domain_driver.h" + +#define VIR_FROM_THIS VIR_FROM_DOMAIN int @@ -84,3 +87,101 @@ virDomainCgroupSetupMemtune(virCgroupPtr cgroup, virDomainMemtune mem) return 0; } + + +int +virDomainCgroupSetupDomainBlkioParameters(virCgroupPtr cgroup, + virDomainDefPtr def, + virTypedParameterPtr params, + int nparams) +{ + size_t i; + int ret = 0; + + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + + if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { + if (virCgroupSetBlkioWeight(cgroup, params[i].value.ui) < 0) + ret = -1; + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { + size_t ndevices; + virBlkioDevicePtr devices = NULL; + size_t j; + + if (virDomainDriverParseBlkioDeviceStr(params[i].value.s, + param->field, + &devices, + &ndevices) < 0) { + ret = -1; + continue; + } + + if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetupBlkioDeviceWeight(cgroup, devices[j].path, + &devices[j].weight) < 0) { + ret = -1; + break; + } + } + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetupBlkioDeviceReadIops(cgroup, devices[j].path, + &devices[j].riops) < 0) { + ret = -1; + break; + } + } + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetupBlkioDeviceWriteIops(cgroup, devices[j].path, + &devices[j].wiops) < 0) { + ret = -1; + break; + } + } + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetupBlkioDeviceReadBps(cgroup, devices[j].path, + &devices[j].rbps) < 0) { + ret = -1; + break; + } + } + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetupBlkioDeviceWriteBps(cgroup, devices[j].path, + &devices[j].wbps) < 0) { + ret = -1; + break; + } + } + } else { + virReportError(VIR_ERR_INVALID_ARG, _("Unknown blkio parameter %s"), + param->field); + ret = -1; + virBlkioDeviceArrayClear(devices, ndevices); + g_free(devices); + + continue; + } + + if (j != ndevices || + virDomainDriverMergeBlkioDevice(&def->blkio.devices, + &def->blkio.ndevices, + devices, ndevices, + param->field) < 0) + ret = -1; + + virBlkioDeviceArrayClear(devices, ndevices); + g_free(devices); + } + } + + return ret; +} diff --git a/src/hypervisor/domain_cgroup.h b/src/hypervisor/domain_cgroup.h index 82ba47e85f..7f3a0d12f9 100644 --- a/src/hypervisor/domain_cgroup.h +++ b/src/hypervisor/domain_cgroup.h @@ -26,3 +26,7 @@ int virDomainCgroupSetupBlkio(virCgroupPtr cgroup, virDomainBlkiotune blkio); int virDomainCgroupSetupMemtune(virCgroupPtr cgroup, virDomainMemtune mem); +int virDomainCgroupSetupDomainBlkioParameters(virCgroupPtr cgroup, + virDomainDefPtr def, + virTypedParameterPtr params, + int nparams); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 88f9ca16d1..a01a0223f3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1391,6 +1391,7 @@ virSetConnectStorage; # hypervisor/domain_cgroup.h virDomainCgroupSetupBlkio; +virDomainCgroupSetupDomainBlkioParameters; virDomainCgroupSetupMemtune; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 78a0174277..c93dee37f8 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -56,6 +56,7 @@ #include "virpidfile.h" #include "virfdstream.h" #include "domain_audit.h" +#include "domain_cgroup.h" #include "domain_driver.h" #include "domain_nwfilter.h" #include "virinitctl.h" @@ -2364,90 +2365,8 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, ret = 0; if (def) { - for (i = 0; i < nparams; i++) { - virTypedParameterPtr param = ¶ms[i]; - - if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { - if (virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui) < 0) - ret = -1; - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - virCgroupPtr cgroup = priv->cgroup; - size_t ndevices; - virBlkioDevicePtr devices = NULL; - size_t j; - - if (virDomainDriverParseBlkioDeviceStr(params[i].value.s, - param->field, - &devices, - &ndevices) < 0) { - ret = -1; - continue; - } - - if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { - for (j = 0; j < ndevices; j++) { - if (virCgroupSetupBlkioDeviceWeight(cgroup, devices[j].path, - &devices[j].weight) < 0) { - ret = -1; - break; - } - } - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { - for (j = 0; j < ndevices; j++) { - if (virCgroupSetupBlkioDeviceReadIops(cgroup, devices[j].path, - &devices[j].riops) < 0) { - ret = -1; - break; - } - } - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { - for (j = 0; j < ndevices; j++) { - if (virCgroupSetupBlkioDeviceWriteIops(cgroup, devices[j].path, - &devices[j].wiops) < 0) { - ret = -1; - break; - } - } - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { - for (j = 0; j < ndevices; j++) { - if (virCgroupSetupBlkioDeviceReadBps(cgroup, devices[j].path, - &devices[j].rbps) < 0) { - ret = -1; - break; - } - } - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - for (j = 0; j < ndevices; j++) { - if (virCgroupSetupBlkioDeviceWriteBps(cgroup, devices[j].path, - &devices[j].wbps) < 0) { - ret = -1; - break; - } - } - } else { - virReportError(VIR_ERR_INVALID_ARG, _("Unknown blkio parameter %s"), - param->field); - ret = -1; - virBlkioDeviceArrayClear(devices, ndevices); - VIR_FREE(devices); - - continue; - } - - if (j != ndevices || - virDomainDriverMergeBlkioDevice(&def->blkio.devices, - &def->blkio.ndevices, - devices, ndevices, - param->field) < 0) - ret = -1; - virBlkioDeviceArrayClear(devices, ndevices); - VIR_FREE(devices); - } - } + ret = virDomainCgroupSetupDomainBlkioParameters(priv->cgroup, def, + params, nparams); } if (ret < 0) goto endjob; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2205dd1194..850d6699ce 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -65,6 +65,7 @@ #include "viruuid.h" #include "domain_conf.h" #include "domain_audit.h" +#include "domain_cgroup.h" #include "domain_driver.h" #include "node_device_conf.h" #include "virpci.h" @@ -9375,91 +9376,8 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, ret = 0; if (def) { - for (i = 0; i < nparams; i++) { - virTypedParameterPtr param = ¶ms[i]; - - if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { - if (virCgroupSetBlkioWeight(priv->cgroup, param->value.ui) < 0 || - virCgroupGetBlkioWeight(priv->cgroup, &def->blkio.weight) < 0) - ret = -1; - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - virCgroupPtr cgroup = priv->cgroup; - size_t ndevices; - virBlkioDevicePtr devices = NULL; - size_t j; - - if (virDomainDriverParseBlkioDeviceStr(param->value.s, - param->field, - &devices, - &ndevices) < 0) { - ret = -1; - continue; - } - - if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { - for (j = 0; j < ndevices; j++) { - if (virCgroupSetupBlkioDeviceWeight(cgroup, devices[j].path, - &devices[j].weight) < 0) { - ret = -1; - break; - } - } - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { - for (j = 0; j < ndevices; j++) { - if (virCgroupSetupBlkioDeviceReadIops(cgroup, devices[j].path, - &devices[j].riops) < 0) { - ret = -1; - break; - } - } - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { - for (j = 0; j < ndevices; j++) { - if (virCgroupSetupBlkioDeviceWriteIops(cgroup, devices[j].path, - &devices[j].wiops) < 0) { - ret = -1; - break; - } - } - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { - for (j = 0; j < ndevices; j++) { - if (virCgroupSetupBlkioDeviceReadBps(cgroup, devices[j].path, - &devices[j].rbps) < 0) { - ret = -1; - break; - } - } - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - for (j = 0; j < ndevices; j++) { - if (virCgroupSetupBlkioDeviceWriteBps(cgroup, devices[j].path, - &devices[j].wbps) < 0) { - ret = -1; - break; - } - } - } else { - virReportError(VIR_ERR_INVALID_ARG, _("Unknown blkio parameter %s"), - param->field); - ret = -1; - virBlkioDeviceArrayClear(devices, ndevices); - VIR_FREE(devices); - - continue; - } - - if (j != ndevices || - virDomainDriverMergeBlkioDevice(&def->blkio.devices, - &def->blkio.ndevices, - devices, ndevices, - param->field) < 0) - ret = -1; - virBlkioDeviceArrayClear(devices, ndevices); - VIR_FREE(devices); - } - } + ret = virDomainCgroupSetupDomainBlkioParameters(priv->cgroup, def, + params, nparams); if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) goto endjob; -- 2.24.1

This new helper avoids more code repetition inside lxcDomainSetBlkioParameters() and qemuDomainSetBlkioParameters(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/hypervisor/domain_driver.c | 44 ++++++++++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 4 ++++ src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 33 +++---------------------- src/qemu/qemu_driver.c | 33 +++---------------------- 5 files changed, 55 insertions(+), 60 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index bbfadb3d9b..0cf6c7604b 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -206,3 +206,47 @@ virDomainDriverParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, } return -1; } + + +int +virDomainDriverSetupPersistentDefBlkioParams(virDomainDefPtr persistentDef, + virTypedParameterPtr params, + int nparams) +{ + size_t i; + int ret = 0; + + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + + if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { + persistentDef->blkio.weight = param->value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { + virBlkioDevicePtr devices = NULL; + size_t ndevices; + + if (virDomainDriverParseBlkioDeviceStr(param->value.s, + param->field, + &devices, + &ndevices) < 0) { + ret = -1; + continue; + } + + if (virDomainDriverMergeBlkioDevice(&persistentDef->blkio.devices, + &persistentDef->blkio.ndevices, + devices, ndevices, + param->field) < 0) + ret = -1; + + virBlkioDeviceArrayClear(devices, ndevices); + g_free(devices); + } + } + + return ret; +} diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index b78401ea42..b6d5e66bba 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -30,3 +30,7 @@ int virDomainDriverMergeBlkioDevice(virBlkioDevicePtr *dest_array, int virDomainDriverParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, virBlkioDevicePtr *dev, size_t *size); + +int virDomainDriverSetupPersistentDefBlkioParams(virDomainDefPtr persistentDef, + virTypedParameterPtr params, + int nparams); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a01a0223f3..c157012707 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1398,6 +1398,7 @@ virDomainCgroupSetupMemtune; # hypervisor/domain_cgroup.h virDomainDriverMergeBlkioDevice; virDomainDriverParseBlkioDeviceStr; +virDomainDriverSetupPersistentDefBlkioParams; # libvirt_internal.h diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index c93dee37f8..0332b7668a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2314,7 +2314,6 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, unsigned int flags) { virLXCDriverPtr driver = dom->conn->privateData; - size_t i; virDomainObjPtr vm = NULL; virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; @@ -2371,35 +2370,9 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, if (ret < 0) goto endjob; if (persistentDef) { - for (i = 0; i < nparams; i++) { - virTypedParameterPtr param = ¶ms[i]; - - if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { - persistentDef->blkio.weight = params[i].value.ui; - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - virBlkioDevicePtr devices = NULL; - size_t ndevices; - - if (virDomainDriverParseBlkioDeviceStr(params[i].value.s, - param->field, - &devices, - &ndevices) < 0) { - ret = -1; - continue; - } - if (virDomainDriverMergeBlkioDevice(&persistentDef->blkio.devices, - &persistentDef->blkio.ndevices, - devices, ndevices, - param->field) < 0) - ret = -1; - virBlkioDeviceArrayClear(devices, ndevices); - VIR_FREE(devices); - } - } + ret = virDomainDriverSetupPersistentDefBlkioParams(persistentDef, + params, + nparams); if (virDomainDefSave(persistentDef, driver->xmlopt, cfg->configDir) < 0) ret = -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 850d6699ce..44145a5f60 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9319,7 +9319,6 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - size_t i; virDomainObjPtr vm = NULL; virDomainDefPtr def; virDomainDefPtr persistentDef; @@ -9385,35 +9384,9 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, if (ret < 0) goto endjob; if (persistentDef) { - for (i = 0; i < nparams; i++) { - virTypedParameterPtr param = ¶ms[i]; - - if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { - persistentDef->blkio.weight = param->value.ui; - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || - STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { - virBlkioDevicePtr devices = NULL; - size_t ndevices; - - if (virDomainDriverParseBlkioDeviceStr(param->value.s, - param->field, - &devices, - &ndevices) < 0) { - ret = -1; - continue; - } - if (virDomainDriverMergeBlkioDevice(&persistentDef->blkio.devices, - &persistentDef->blkio.ndevices, - devices, ndevices, - param->field) < 0) - ret = -1; - virBlkioDeviceArrayClear(devices, ndevices); - VIR_FREE(devices); - } - } + ret = virDomainDriverSetupPersistentDefBlkioParams(persistentDef, + params, + nparams); if (virDomainDefSave(persistentDef, driver->xmlopt, cfg->configDir) < 0) ret = -1; -- 2.24.1

lxcDomainSetMemoryParameters() and qemuDomainSetMemoryParameters() has duplicated chunks of code that can be put in a new helper. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/hypervisor/domain_cgroup.c | 81 ++++++++++++++++++++++++++++++++++ src/hypervisor/domain_cgroup.h | 6 +++ src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 71 ++--------------------------- src/qemu/qemu_driver.c | 71 ++--------------------------- 5 files changed, 96 insertions(+), 134 deletions(-) diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c index 32b07be3d6..2b3e1d6956 100644 --- a/src/hypervisor/domain_cgroup.c +++ b/src/hypervisor/domain_cgroup.c @@ -185,3 +185,84 @@ virDomainCgroupSetupDomainBlkioParameters(virCgroupPtr cgroup, return ret; } + + +int +virDomainCgroupSetMemoryLimitParameters(virCgroupPtr cgroup, + virDomainObjPtr vm, + virDomainDefPtr liveDef, + virDomainDefPtr persistentDef, + virTypedParameterPtr params, + int nparams) +{ + unsigned long long swap_hard_limit; + unsigned long long hard_limit = 0; + unsigned long long soft_limit = 0; + bool set_swap_hard_limit = false; + bool set_hard_limit = false; + bool set_soft_limit = false; + int rc; + +#define VIR_GET_LIMIT_PARAMETER(PARAM, VALUE) \ + if ((rc = virTypedParamsGetULLong(params, nparams, PARAM, &VALUE)) < 0) \ + return -1; \ + \ + if (rc == 1) \ + set_ ## VALUE = true; + + VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, swap_hard_limit) + VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_HARD_LIMIT, hard_limit) + VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SOFT_LIMIT, soft_limit) + +#undef VIR_GET_LIMIT_PARAMETER + + /* Swap hard limit must be greater than hard limit. */ + if (set_swap_hard_limit || set_hard_limit) { + unsigned long long mem_limit = vm->def->mem.hard_limit; + unsigned long long swap_limit = vm->def->mem.swap_hard_limit; + + if (set_swap_hard_limit) + swap_limit = swap_hard_limit; + + if (set_hard_limit) + mem_limit = hard_limit; + + if (mem_limit > swap_limit) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("memory hard_limit tunable value must be lower " + "than or equal to swap_hard_limit")); + return -1; + } + } + +#define VIR_SET_MEM_PARAMETER(FUNC, VALUE) \ + if (set_ ## VALUE) { \ + if (liveDef) { \ + if ((rc = FUNC(cgroup, VALUE)) < 0) \ + return -1; \ + liveDef->mem.VALUE = VALUE; \ + } \ + \ + if (persistentDef) \ + persistentDef->mem.VALUE = VALUE; \ + } + + /* Soft limit doesn't clash with the others */ + VIR_SET_MEM_PARAMETER(virCgroupSetMemorySoftLimit, soft_limit); + + /* set hard limit before swap hard limit if decreasing it */ + if (liveDef && liveDef->mem.hard_limit > hard_limit) { + VIR_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit); + /* inhibit changing the limit a second time */ + set_hard_limit = false; + } + + VIR_SET_MEM_PARAMETER(virCgroupSetMemSwapHardLimit, swap_hard_limit); + + /* otherwise increase it after swap hard limit */ + VIR_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit); + +#undef VIR_SET_MEM_PARAMETER + + return 0; +} diff --git a/src/hypervisor/domain_cgroup.h b/src/hypervisor/domain_cgroup.h index 7f3a0d12f9..5a6498b1ce 100644 --- a/src/hypervisor/domain_cgroup.h +++ b/src/hypervisor/domain_cgroup.h @@ -30,3 +30,9 @@ int virDomainCgroupSetupDomainBlkioParameters(virCgroupPtr cgroup, virDomainDefPtr def, virTypedParameterPtr params, int nparams); +int virDomainCgroupSetMemoryLimitParameters(virCgroupPtr cgroup, + virDomainObjPtr vm, + virDomainDefPtr liveDef, + virDomainDefPtr persistentDef, + virTypedParameterPtr params, + int nparams); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c157012707..679e32116c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1390,6 +1390,7 @@ virSetConnectStorage; # hypervisor/domain_cgroup.h +virDomainCgroupSetMemoryLimitParameters; virDomainCgroupSetupBlkio; virDomainCgroupSetupDomainBlkioParameters; virDomainCgroupSetupMemtune; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 0332b7668a..fc559d736a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -749,13 +749,6 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, virLXCDomainObjPrivatePtr priv = NULL; virLXCDriverConfigPtr cfg = NULL; virLXCDriverPtr driver = dom->conn->privateData; - unsigned long long hard_limit; - unsigned long long soft_limit; - unsigned long long swap_hard_limit; - bool set_hard_limit = false; - bool set_soft_limit = false; - bool set_swap_hard_limit = false; - int rc; int ret = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -794,66 +787,10 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, goto endjob; } -#define VIR_GET_LIMIT_PARAMETER(PARAM, VALUE) \ - if ((rc = virTypedParamsGetULLong(params, nparams, PARAM, &VALUE)) < 0) \ - goto endjob; \ - \ - if (rc == 1) \ - set_ ## VALUE = true; - - VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, swap_hard_limit) - VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_HARD_LIMIT, hard_limit) - VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SOFT_LIMIT, soft_limit) - -#undef VIR_GET_LIMIT_PARAMETER - - /* Swap hard limit must be greater than hard limit. */ - if (set_swap_hard_limit || set_hard_limit) { - unsigned long long mem_limit = vm->def->mem.hard_limit; - unsigned long long swap_limit = vm->def->mem.swap_hard_limit; - - if (set_swap_hard_limit) - swap_limit = swap_hard_limit; - - if (set_hard_limit) - mem_limit = hard_limit; - - if (mem_limit > swap_limit) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("memory hard_limit tunable value must be lower " - "than or equal to swap_hard_limit")); - goto endjob; - } - } - -#define VIR_SET_MEM_PARAMETER(FUNC, VALUE) \ - if (set_ ## VALUE) { \ - if (def) { \ - if ((rc = FUNC(priv->cgroup, VALUE)) < 0) \ - goto endjob; \ - def->mem.VALUE = VALUE; \ - } \ - \ - if (persistentDef) \ - persistentDef->mem.VALUE = VALUE; \ - } - - /* Soft limit doesn't clash with the others */ - VIR_SET_MEM_PARAMETER(virCgroupSetMemorySoftLimit, soft_limit); - - /* set hard limit before swap hard limit if decreasing it */ - if (def && def->mem.hard_limit > hard_limit) { - VIR_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit); - /* inhibit changing the limit a second time */ - set_hard_limit = false; - } - - VIR_SET_MEM_PARAMETER(virCgroupSetMemSwapHardLimit, swap_hard_limit); - - /* otherwise increase it after swap hard limit */ - VIR_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit); - -#undef VIR_SET_MEM_PARAMETER + if (virDomainCgroupSetMemoryLimitParameters(priv->cgroup, vm, def, + persistentDef, + params, nparams) < 0) + goto endjob; if (def && virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 44145a5f60..b65708385f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9502,14 +9502,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainObjPtr vm = NULL; - unsigned long long swap_hard_limit; - unsigned long long hard_limit = 0; - unsigned long long soft_limit = 0; - bool set_swap_hard_limit = false; - bool set_hard_limit = false; - bool set_soft_limit = false; g_autoptr(virQEMUDriverConfig) cfg = NULL; - int rc; int ret = -1; qemuDomainObjPrivatePtr priv; @@ -9556,66 +9549,10 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, goto endjob; } -#define VIR_GET_LIMIT_PARAMETER(PARAM, VALUE) \ - if ((rc = virTypedParamsGetULLong(params, nparams, PARAM, &VALUE)) < 0) \ - goto endjob; \ - \ - if (rc == 1) \ - set_ ## VALUE = true; - - VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, swap_hard_limit) - VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_HARD_LIMIT, hard_limit) - VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SOFT_LIMIT, soft_limit) - -#undef VIR_GET_LIMIT_PARAMETER - - /* Swap hard limit must be greater than hard limit. */ - if (set_swap_hard_limit || set_hard_limit) { - unsigned long long mem_limit = vm->def->mem.hard_limit; - unsigned long long swap_limit = vm->def->mem.swap_hard_limit; - - if (set_swap_hard_limit) - swap_limit = swap_hard_limit; - - if (set_hard_limit) - mem_limit = hard_limit; - - if (mem_limit > swap_limit) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("memory hard_limit tunable value must be lower " - "than or equal to swap_hard_limit")); - goto endjob; - } - } - -#define VIR_SET_MEM_PARAMETER(FUNC, VALUE) \ - if (set_ ## VALUE) { \ - if (def) { \ - if ((rc = FUNC(priv->cgroup, VALUE)) < 0) \ - goto endjob; \ - def->mem.VALUE = VALUE; \ - } \ - \ - if (persistentDef) \ - persistentDef->mem.VALUE = VALUE; \ - } - - /* Soft limit doesn't clash with the others */ - VIR_SET_MEM_PARAMETER(virCgroupSetMemorySoftLimit, soft_limit); - - /* set hard limit before swap hard limit if decreasing it */ - if (def && def->mem.hard_limit > hard_limit) { - VIR_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit); - /* inhibit changing the limit a second time */ - set_hard_limit = false; - } - - VIR_SET_MEM_PARAMETER(virCgroupSetMemSwapHardLimit, swap_hard_limit); - - /* otherwise increase it after swap hard limit */ - VIR_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit); - -#undef VIR_SET_MEM_PARAMETER + if (virDomainCgroupSetMemoryLimitParameters(priv->cgroup, vm, def, + persistentDef, + params, nparams) < 0) + goto endjob; if (def && virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) -- 2.24.1

Another vircgroup helper to avoid code repetition between the LXC and QEMU driver. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 8 +------- src/qemu/qemu_driver.c | 8 +------- src/util/vircgroup.c | 14 ++++++++++++++ src/util/vircgroup.h | 2 ++ 5 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 679e32116c..4c6ad89ecc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1692,6 +1692,7 @@ virCgroupGetCpuacctStat; virCgroupGetCpuacctUsage; virCgroupGetCpuCfsPeriod; virCgroupGetCpuCfsQuota; +virCgroupGetCpuPeriodQuota; virCgroupGetCpusetCpus; virCgroupGetCpusetMemoryMigrate; virCgroupGetCpusetMems; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index fc559d736a..b2e9bc3a46 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1783,13 +1783,7 @@ static int lxcGetVcpuBWLive(virCgroupPtr cgroup, unsigned long long *period, long long *quota) { - if (virCgroupGetCpuCfsPeriod(cgroup, period) < 0) - return -1; - - if (virCgroupGetCpuCfsQuota(cgroup, quota) < 0) - return -1; - - return 0; + return virCgroupGetCpuPeriodQuota(cgroup, period, quota); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b65708385f..dda4bded08 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10532,13 +10532,7 @@ static int qemuGetVcpuBWLive(virCgroupPtr cgroup, unsigned long long *period, long long *quota) { - if (virCgroupGetCpuCfsPeriod(cgroup, period) < 0) - return -1; - - if (virCgroupGetCpuCfsQuota(cgroup, quota) < 0) - return -1; - - return 0; + return virCgroupGetCpuPeriodQuota(cgroup, period, quota); } static int diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 1f853beb73..70d85200cb 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3739,3 +3739,17 @@ virCgroupSetupCpuPeriodQuota(virCgroupPtr cgroup, return -1; } + + +int +virCgroupGetCpuPeriodQuota(virCgroupPtr cgroup, unsigned long long *period, + long long *quota) +{ + if (virCgroupGetCpuCfsPeriod(cgroup, period) < 0) + return -1; + + if (virCgroupGetCpuCfsQuota(cgroup, quota) < 0) + return -1; + + return 0; +} diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index cc53bae258..a5bd35586d 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -227,6 +227,8 @@ int virCgroupSetupCpuShares(virCgroupPtr cgroup, unsigned long long shares, 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, + long long *quota); int virCgroupSetupCpuPeriodQuota(virCgroupPtr cgroup, unsigned long long period, long long quota); -- 2.24.1