[libvirt] [PATCH 0/4] consolidate cgroup code that is duplicated between
Both QEMU and LXC drivers have some duplicate code when dealing with cgroups, this serials mainly consolidate the same chunk of src/qemu/qemu_cgroup.c and src/lxc/lxc_cgroup.c into src/util/cgroup.c. Mao Zhongyi (4): lxc: remove duplicate header files qemu: remove duplicate header files util: consolidate QEMU and LXC block io related duplicate code util: consolidate QEMU and LXC memory related duplicate code src/libvirt_private.syms | 2 + src/lxc/lxc_cgroup.c | 85 +------------------------------------ src/qemu/qemu_cgroup.c | 62 +-------------------------- src/util/vircgroup.c | 91 ++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 4 ++ 5 files changed, 101 insertions(+), 143 deletions(-) -- 2.17.1
"#include vircgroup.h" appears in both lxc_cgroup.h and lxc_cgroup.c, and lxc_cgroup.c contains lxc_cgroup.h, so remove the duplicate declarations. Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> --- src/lxc/lxc_cgroup.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 5efb495b56..0a019dc813 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -27,7 +27,6 @@ #include "virerror.h" #include "virlog.h" #include "viralloc.h" -#include "vircgroup.h" #include "virstring.h" #include "virsystemd.h" -- 2.17.1
On Wed, Oct 30, 2019 at 11:42:10AM +0800, Mao Zhongyi wrote:
"#include vircgroup.h" appears in both lxc_cgroup.h and lxc_cgroup.c, and lxc_cgroup.c contains lxc_cgroup.h, so remove the duplicate declarations.
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> --- src/lxc/lxc_cgroup.c | 1 - 1 file changed, 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
"#include vircgroup.h" appears in both qemu_cgroup.h and qemu_cgroup.c, and qemu_cgroup.c contains qemu_cgroup.h, so remove the duplicate declarations. Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> --- src/qemu/qemu_cgroup.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f8bb270117..131cdd1134 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -25,7 +25,6 @@ #include "qemu_domain.h" #include "qemu_process.h" #include "qemu_extdevice.h" -#include "vircgroup.h" #include "virlog.h" #include "viralloc.h" #include "virerror.h" -- 2.17.1
On Wed, Oct 30, 2019 at 11:42:11AM +0800, Mao Zhongyi wrote:
"#include vircgroup.h" appears in both qemu_cgroup.h and qemu_cgroup.c, and qemu_cgroup.c contains qemu_cgroup.h, so remove the duplicate declarations.
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> --- src/qemu/qemu_cgroup.c | 1 - 1 file changed, 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Both qemu and lxc drivers have some duplicate code when dealing with cgroups, such as virLXCCgroupSetupBlkioTune() and qemuSetupBlkioCgroup(), duplicate code over 80, so consolidate the same chunk into a separate routine. Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> --- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 56 +----------------------------------- src/qemu/qemu_cgroup.c | 47 +------------------------------ src/util/vircgroup.c | 61 ++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 3 ++ 5 files changed, 67 insertions(+), 101 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 94509d6f43..3df2af55bd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1685,6 +1685,7 @@ virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; virCgroupSetMemSwapHardLimit; virCgroupSetOwner; +virCgroupSetupBlkio; virCgroupSupportsCpuBW; virCgroupTerminateMachine; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 0a019dc813..fdf8df79aa 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -102,60 +102,6 @@ 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 && - (virCgroupSetBlkioDeviceWeight(cgroup, dev->path, - dev->weight) < 0 || - virCgroupGetBlkioDeviceWeight(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)) - return -1; - - if (dev->wiops && - (virCgroupSetBlkioDeviceWriteIops(cgroup, dev->path, - dev->wiops) < 0 || - virCgroupGetBlkioDeviceWriteIops(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)) - return -1; - - if (dev->wbps && - (virCgroupSetBlkioDeviceWriteBps(cgroup, dev->path, - dev->wbps) < 0 || - virCgroupGetBlkioDeviceWriteBps(cgroup, dev->path, - &dev->wbps) < 0)) - return -1; - } - } - - return 0; -} - - static int virLXCCgroupSetupMemTune(virDomainDefPtr def, virCgroupPtr cgroup) { @@ -489,7 +435,7 @@ int virLXCCgroupSetup(virDomainDefPtr def, if (virLXCCgroupSetupCpusetTune(def, cgroup, nodemask) < 0) goto cleanup; - if (virLXCCgroupSetupBlkioTune(def, cgroup) < 0) + if (virCgroupSetupBlkio(cgroup, def) < 0) goto cleanup; if (virLXCCgroupSetupMemTune(def, cgroup) < 0) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 131cdd1134..c9925ee5ca 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -521,7 +521,6 @@ static int qemuSetupBlkioCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - size_t i; if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { @@ -534,51 +533,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]; - if (dev->weight && - (virCgroupSetBlkioDeviceWeight(priv->cgroup, dev->path, - dev->weight) < 0 || - virCgroupGetBlkioDeviceWeight(priv->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)) - return -1; - - if (dev->wiops && - (virCgroupSetBlkioDeviceWriteIops(priv->cgroup, dev->path, - dev->wiops) < 0 || - virCgroupGetBlkioDeviceWriteIops(priv->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)) - return -1; - - if (dev->wbps && - (virCgroupSetBlkioDeviceWriteBps(priv->cgroup, dev->path, - dev->wbps) < 0 || - virCgroupGetBlkioDeviceWriteBps(priv->cgroup, dev->path, - &dev->wbps) < 0)) - return -1; - } - } - - return 0; + return virCgroupSetupBlkio(priv->cgroup, vm->def); } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index b46f20abfd..165e5f4854 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1419,6 +1419,67 @@ virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) getBlkioWeight, -1, weight); } +/** + * virCgroupSetupBlkio + * + * @group: The cgroup to change block io setting for + * @def: pointer to domain def + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupSetupBlkio(virCgroupPtr group, virDomainDefPtr def) +{ + size_t i; + + if (def->blkio.weight != 0 && + virCgroupSetBlkioWeight(group, 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 && + (virCgroupSetBlkioDeviceWeight(group, dev->path, + dev->weight) < 0 || + virCgroupGetBlkioDeviceWeight(group, dev->path, + &dev->weight) < 0)) + return -1; + + if (dev->riops && + (virCgroupSetBlkioDeviceReadIops(group, dev->path, + dev->riops) < 0 || + virCgroupGetBlkioDeviceReadIops(group, dev->path, + &dev->riops) < 0)) + return -1; + + if (dev->wiops && + (virCgroupSetBlkioDeviceWriteIops(group, dev->path, + dev->wiops) < 0 || + virCgroupGetBlkioDeviceWriteIops(group, dev->path, + &dev->wiops) < 0)) + return -1; + + if (dev->rbps && + (virCgroupSetBlkioDeviceReadBps(group, dev->path, + dev->rbps) < 0 || + virCgroupGetBlkioDeviceReadBps(group, dev->path, + &dev->rbps) < 0)) + return -1; + + if (dev->wbps && + (virCgroupSetBlkioDeviceWriteBps(group, dev->path, + dev->wbps) < 0 || + virCgroupGetBlkioDeviceWriteBps(group, dev->path, + &dev->wbps) < 0)) + return -1; + } + } + + return 0; +} + + /** * virCgroupSetBlkioDeviceReadIops: * @group: The cgroup to change block io setting for diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 3eefe78787..9815c51fda 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -24,6 +24,7 @@ #include "virutil.h" #include "virbitmap.h" #include "virenum.h" +#include "conf/domain_conf.h" struct _virCgroup; typedef struct _virCgroup virCgroup; @@ -123,6 +124,8 @@ int virCgroupAddThread(virCgroupPtr group, pid_t pid); int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight); +int virCgroupSetupBlkio(virCgroupPtr group, virDomainDefPtr def); + int virCgroupGetBlkioIoServiced(virCgroupPtr group, long long *bytes_read, long long *bytes_write, -- 2.17.1
On Wed, Oct 30, 2019 at 11:42:12AM +0800, Mao Zhongyi wrote:
Both qemu and lxc drivers have some duplicate code when dealing with cgroups, such as virLXCCgroupSetupBlkioTune() and qemuSetupBlkioCgroup(), duplicate code over 80, so consolidate the same chunk into a separate routine.
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> --- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 56 +----------------------------------- src/qemu/qemu_cgroup.c | 47 +------------------------------ src/util/vircgroup.c | 61 ++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 3 ++ 5 files changed, 67 insertions(+), 101 deletions(-)
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 3eefe78787..9815c51fda 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -24,6 +24,7 @@ #include "virutil.h" #include "virbitmap.h" #include "virenum.h" +#include "conf/domain_conf.h"
Code in src/util should not be including code from src/conf. src/util is for the lowest level of utilities src/conf can use code from src/util, but not the other way around and various hypervisor drivers use code from those I don't remember whether it was this particular function or something other shared between LXC and QEMU, but my attempt to introduce a layer between src/conf and the drivers was unsuccessful - probably due to too generic naming (I proposed src/common). So either the BlkioDevice data type needs to be defined in a lower layer than src/conf, or we can just give up and put these functions somewehere in src/conf/virblkio Introducing a dependency on domain_conf.h here could lead to the same recursive problems we have in src/util/virhostdev.h Jano
struct _virCgroup; typedef struct _virCgroup virCgroup; @@ -123,6 +124,8 @@ int virCgroupAddThread(virCgroupPtr group, pid_t pid); int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight);
+int virCgroupSetupBlkio(virCgroupPtr group, virDomainDefPtr def); + int virCgroupGetBlkioIoServiced(virCgroupPtr group, long long *bytes_read, long long *bytes_write, -- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Both qemu and lxc drivers have some duplicate code when dealing with cgroups, such as virLXCCgroupSetupMemTune() and qemuSetupMemoryCgroup(), duplicate code over 80, so consolidate the same chunk into a separate routine. Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> --- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 28 +--------------------------- src/qemu/qemu_cgroup.c | 14 +------------- src/util/vircgroup.c | 30 ++++++++++++++++++++++++++++++ src/util/vircgroup.h | 1 + 5 files changed, 34 insertions(+), 40 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3df2af55bd..ff002fc60b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1686,6 +1686,7 @@ virCgroupSetMemorySoftLimit; virCgroupSetMemSwapHardLimit; virCgroupSetOwner; virCgroupSetupBlkio; +virCgroupSetupMemory; virCgroupSupportsCpuBW; virCgroupTerminateMachine; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index fdf8df79aa..ea6c14dfef 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -102,32 +102,6 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def, } -static int virLXCCgroupSetupMemTune(virDomainDefPtr def, - virCgroupPtr cgroup) -{ - int ret = -1; - - if (virCgroupSetMemory(cgroup, virDomainDefGetMemoryInitial(def)) < 0) - goto cleanup; - - if (virMemoryLimitIsSet(def->mem.hard_limit)) - if (virCgroupSetMemoryHardLimit(cgroup, def->mem.hard_limit) < 0) - goto cleanup; - - if (virMemoryLimitIsSet(def->mem.soft_limit)) - if (virCgroupSetMemorySoftLimit(cgroup, def->mem.soft_limit) < 0) - goto cleanup; - - if (virMemoryLimitIsSet(def->mem.swap_hard_limit)) - if (virCgroupSetMemSwapHardLimit(cgroup, def->mem.swap_hard_limit) < 0) - goto cleanup; - - ret = 0; - cleanup: - return ret; -} - - static int virLXCCgroupGetMemSwapUsage(virCgroupPtr cgroup, virLXCMeminfoPtr meminfo) { @@ -438,7 +412,7 @@ int virLXCCgroupSetup(virDomainDefPtr def, if (virCgroupSetupBlkio(cgroup, def) < 0) goto cleanup; - if (virLXCCgroupSetupMemTune(def, cgroup) < 0) + if (virCgroupSetupMemory(cgroup, def) < 0) goto cleanup; if (virLXCCgroupSetupDeviceACL(def, cgroup) < 0) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index c9925ee5ca..22f77a03ad 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -554,19 +554,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 virCgroupSetupMemory(priv->cgroup, vm->def); } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 165e5f4854..12af8759f7 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1670,6 +1670,36 @@ virCgroupSetMemory(virCgroupPtr group, unsigned long long kb) setMemory, -1, kb); } +/** + * virCgroupSetupMemory + * + * @group: The cgroup to change memory for + * @def: pointer to domian def + * + * Returns: 0 on success, -1 on error. + */ +int +virCgroupSetupMemory(virCgroupPtr group, virDomainDefPtr def) +{ + int ret = -1; + + if (virMemoryLimitIsSet(def->mem.hard_limit)) + if (virCgroupSetMemoryHardLimit(group, def->mem.hard_limit) < 0) + goto cleanup; + + if (virMemoryLimitIsSet(def->mem.soft_limit)) + if (virCgroupSetMemorySoftLimit(group, def->mem.soft_limit) < 0) + goto cleanup; + + if (virMemoryLimitIsSet(def->mem.swap_hard_limit)) + if (virCgroupSetMemSwapHardLimit(group, def->mem.swap_hard_limit) < 0) + goto cleanup; + + ret = 0; + cleanup: + return ret; +} + /** * virCgroupGetMemoryStat: diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 9815c51fda..17f07ea53c 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -179,6 +179,7 @@ int virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, unsigned long long *wbps); int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb); +int virCgroupSetupMemory(virCgroupPtr group, virDomainDefPtr def); int virCgroupGetMemoryStat(virCgroupPtr group, unsigned long long *cache, unsigned long long *activeAnon, -- 2.17.1
participants (2)
-
Ján Tomko -
Mao Zhongyi