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
s/introducing/introduce/ On Mon, Feb 17, 2020 at 04:29:11PM -0500, Daniel Henrique Barboza wrote: >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 missing space >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 > Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
© 2016 - 2024 Red Hat, Inc.