4:57 p.m.
virLXCCgroupSetupBlkioTune() and qemuSetupBlkioCgroup() and
virLXCCgroupSetupCpuTune() and qemuSetupCpuCgroup() are the most similar
functions between QEMU and LXC code.
Let's move their common code to virCgroup.
Mind that the first two patches are basically preparing the ground for
the changes introduced in the last two patches.
changes since v1:
- Michal Privoznik pointed out (as did the `make syntax-check` :-)) that
we do want to keep src/util independently of the parsing code (thus,
including "conf/domain_conf.h" in vircgroup.h is not the way to go).
This has been solved now by partially following Michal's suggestion
and splitting the structs and functions that would be use in the
common code to new different files.
Fabiano Fidêncio (4):
domain_conf: split out virBlkioDevice and virDomainBlkiotune
definitions
domain_conf: split out virDomainMemtune and virDomainHugePage
definitions
vircgroup: Add virCgroupSetupBlkioTune()
vircgroup: Add virCgroupSetupMemTune()
src/Makefile.am | 1 +
src/conf/domain_conf.c | 22 ++++--------
src/conf/domain_conf.h | 70 +++----------------------------------
src/libvirt_private.syms | 2 ++
src/lxc/lxc_cgroup.c | 69 ++-----------------------------------
src/qemu/qemu_cgroup.c | 61 ++-------------------------------
src/qemu/qemu_command.c | 4 +--
src/util/Makefile.inc.am | 2 ++
src/util/virblkio.c | 37 ++++++++++++++++++++
src/util/virblkio.h | 52 ++++++++++++++++++++++++++++
src/util/vircgroup.c | 74 ++++++++++++++++++++++++++++++++++++++++
src/util/vircgroup.h | 7 ++++
src/util/virmem.h | 66 +++++++++++++++++++++++++++++++++++
13 files changed, 259 insertions(+), 208 deletions(-)
create mode 100644 src/util/virblkio.c
create mode 100644 src/util/virblkio.h
create mode 100644 src/util/virmem.h
--
2.17.1
4:57 p.m.
New subject: [libvirt] [libvirt PATCH v2 1/4] domain_conf: split out virBlkioDevice and virDomainBlkiotune definitions
Let's move those to their own newly created files
(src/util/virblkio.{c,h}) as this will help us to easily start sharing
the cgroup code that's duplicated between QEMU and LXC.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/Makefile.am | 1 +
src/conf/domain_conf.c | 11 +--------
src/conf/domain_conf.h | 27 ++-------------------
src/util/Makefile.inc.am | 2 ++
src/util/virblkio.c | 37 ++++++++++++++++++++++++++++
src/util/virblkio.h | 52 ++++++++++++++++++++++++++++++++++++++++
6 files changed, 95 insertions(+), 35 deletions(-)
create mode 100644 src/util/virblkio.c
create mode 100644 src/util/virblkio.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 2a3ed0d42d..926085ff2d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -671,6 +671,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \
util/viratomic.c \
util/viratomic.h \
util/virbitmap.c \
+ util/virblkio.c \
util/virbuffer.c \
util/vircgroup.c \
util/vircommand.c \
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7e14cea128..6ce50f712a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -59,6 +59,7 @@
#include "virnetdevmacvlan.h"
#include "virhostdev.h"
#include "virmdev.h"
+#include "virblkio.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -1205,16 +1206,6 @@ virDomainXMLOptionGetSaveCookie(virDomainXMLOptionPtr xmlopt)
}
-void
-virBlkioDeviceArrayClear(virBlkioDevicePtr devices,
- int ndevices)
-{
- size_t i;
-
- for (i = 0; i < ndevices; i++)
- VIR_FREE(devices[i].path);
-}
-
/**
* virDomainBlkioDeviceParseXML
*
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e30a4b2fe7..e9e6b6d6c4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -57,6 +57,7 @@
# include "virtypedparam.h"
# include "virsavecookie.h"
# include "virresctrl.h"
+# include "virblkio.h"
/* forward declarations of all device types, required by
* virDomainDeviceDef
@@ -2084,17 +2085,6 @@ struct _virDomainClockDef {
};
-typedef struct _virBlkioDevice virBlkioDevice;
-typedef virBlkioDevice *virBlkioDevicePtr;
-struct _virBlkioDevice {
- char *path;
- unsigned int weight;
- unsigned int riops;
- unsigned int wiops;
- unsigned long long rbps;
- unsigned long long wbps;
-};
-
typedef enum {
VIR_DOMAIN_RNG_MODEL_VIRTIO,
@@ -2184,9 +2174,6 @@ struct _virDomainPanicDef {
};
-void virBlkioDeviceArrayClear(virBlkioDevicePtr deviceWeights,
- int ndevices);
-
typedef struct _virDomainResourceDef virDomainResourceDef;
typedef virDomainResourceDef *virDomainResourceDefPtr;
struct _virDomainResourceDef {
@@ -2260,16 +2247,6 @@ struct _virDomainVcpuDef {
virObjectPtr privateData;
};
-typedef struct _virDomainBlkiotune virDomainBlkiotune;
-typedef virDomainBlkiotune *virDomainBlkiotunePtr;
-
-struct _virDomainBlkiotune {
- unsigned int weight;
-
- size_t ndevices;
- virBlkioDevicePtr devices;
-};
-
typedef struct _virDomainMemtune virDomainMemtune;
typedef virDomainMemtune *virDomainMemtunePtr;
@@ -2402,7 +2379,7 @@ struct _virDomainDef {
char *title;
char *description;
- virDomainBlkiotune blkio;
+ virBlkioTune blkio;
virDomainMemtune mem;
virDomainVcpuDefPtr *vcpus;
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index a22265606c..13f415b23c 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -17,6 +17,8 @@ UTIL_SOURCES = \
util/virauthconfig.h \
util/virbitmap.c \
util/virbitmap.h \
+ util/virblkio.c \
+ util/virblkio.h \
util/virbuffer.c \
util/virbuffer.h \
util/virperf.c \
diff --git a/src/util/virblkio.c b/src/util/virblkio.c
new file mode 100644
index 0000000000..9711077ee8
--- /dev/null
+++ b/src/util/virblkio.c
@@ -0,0 +1,37 @@
+/*
+ * virblkio.c: Block IO helpers
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * 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/>;.
+ *
+ * Authors:
+ * Fabiano Fidêncio <fidencio(a)redhat.com>
+ */
+
+#include <config.h>
+
+#include "viralloc.h"
+#include "virblkio.h"
+
+void
+virBlkioDeviceArrayClear(virBlkioDevicePtr devices,
+ int ndevices)
+{
+ size_t i;
+
+ for (i = 0; i < ndevices; i++)
+ VIR_FREE(devices[i].path);
+}
diff --git a/src/util/virblkio.h b/src/util/virblkio.h
new file mode 100644
index 0000000000..dcaeaaf1f0
--- /dev/null
+++ b/src/util/virblkio.h
@@ -0,0 +1,52 @@
+/*
+ * virblkio.h: Block IO definitions and helpers
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * 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/>;.
+ *
+ * Author: Fabiano Fidêncio <fidencio(a)redhat.com>
+ */
+
+#ifndef __VIR_BLKIO_H__
+# define __VIR_BLKIO_H__
+
+#include "virutil.h"
+
+typedef struct _virBlkioDevice virBlkioDevice;
+typedef virBlkioDevice *virBlkioDevicePtr;
+struct _virBlkioDevice {
+ char *path;
+ unsigned int weight;
+ unsigned int riops;
+ unsigned int wiops;
+ unsigned long long rbps;
+ unsigned long long wbps;
+};
+
+
+typedef struct _virBlkioTune virBlkioTune;
+typedef virBlkioTune *virBlkioTunePtr;
+struct _virBlkioTune {
+ unsigned int weight;
+
+ size_t ndevices;
+ virBlkioDevicePtr devices;
+};
+
+void virBlkioDeviceArrayClear(virBlkioDevicePtr deviceWeights,
+ int ndevices);
+
+#endif /* __VIR_BLKIO_H__ */
--
2.17.1
9:20 p.m.
New subject: [libvirt] [libvirt PATCH v2 1/4] domain_conf: split out virBlkioDevice and virDomainBlkiotune definitions
On 09/12/2018 10:57 AM, Fabiano Fidêncio wrote:
Let's move those to their own newly created files
(src/util/virblkio.{c,h}) as this will help us to easily start sharing
the cgroup code that's duplicated between QEMU and LXC.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/Makefile.am | 1 +
src/conf/domain_conf.c | 11 +--------
src/conf/domain_conf.h | 27 ++-------------------
src/util/Makefile.inc.am | 2 ++
src/util/virblkio.c | 37 ++++++++++++++++++++++++++++
src/util/virblkio.h | 52 ++++++++++++++++++++++++++++++++++++++++
6 files changed, 95 insertions(+), 35 deletions(-)
create mode 100644 src/util/virblkio.c
create mode 100644 src/util/virblkio.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 2a3ed0d42d..926085ff2d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -671,6 +671,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \
util/viratomic.c \
util/viratomic.h \
util/virbitmap.c \
+ util/virblkio.c \
util/virbuffer.c \
util/vircgroup.c \
util/vircommand.c \
This is not needed. setuid_rpc_client is trying to be very minimalistic
library which is then statically linked to virt-login-shell. The aim is
to have something small, thus auditable and yet capable of talking to
libvirtd through RPC.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7e14cea128..6ce50f712a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -59,6 +59,7 @@
#include "virnetdevmacvlan.h"
#include "virhostdev.h"
#include "virmdev.h"
+#include "virblkio.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -1205,16 +1206,6 @@ virDomainXMLOptionGetSaveCookie(virDomainXMLOptionPtr xmlopt)
}
-void
-virBlkioDeviceArrayClear(virBlkioDevicePtr devices,
- int ndevices)
-{
- size_t i;
-
- for (i = 0; i < ndevices; i++)
- VIR_FREE(devices[i].path);
-}
-
/**
* virDomainBlkioDeviceParseXML
*
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e30a4b2fe7..e9e6b6d6c4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -57,6 +57,7 @@
# include "virtypedparam.h"
# include "virsavecookie.h"
# include "virresctrl.h"
+# include "virblkio.h"
/* forward declarations of all device types, required by
* virDomainDeviceDef
@@ -2084,17 +2085,6 @@ struct _virDomainClockDef {
};
-typedef struct _virBlkioDevice virBlkioDevice;
-typedef virBlkioDevice *virBlkioDevicePtr;
-struct _virBlkioDevice {
- char *path;
- unsigned int weight;
- unsigned int riops;
- unsigned int wiops;
- unsigned long long rbps;
- unsigned long long wbps;
-};
-
typedef enum {
VIR_DOMAIN_RNG_MODEL_VIRTIO,
@@ -2184,9 +2174,6 @@ struct _virDomainPanicDef {
};
-void virBlkioDeviceArrayClear(virBlkioDevicePtr deviceWeights,
- int ndevices);
-
typedef struct _virDomainResourceDef virDomainResourceDef;
typedef virDomainResourceDef *virDomainResourceDefPtr;
struct _virDomainResourceDef {
@@ -2260,16 +2247,6 @@ struct _virDomainVcpuDef {
virObjectPtr privateData;
};
-typedef struct _virDomainBlkiotune virDomainBlkiotune;
-typedef virDomainBlkiotune *virDomainBlkiotunePtr;
-
-struct _virDomainBlkiotune {
- unsigned int weight;
-
- size_t ndevices;
- virBlkioDevicePtr devices;
-};
-
typedef struct _virDomainMemtune virDomainMemtune;
typedef virDomainMemtune *virDomainMemtunePtr;
@@ -2402,7 +2379,7 @@ struct _virDomainDef {
char *title;
char *description;
- virDomainBlkiotune blkio;
+ virBlkioTune blkio;
virDomainMemtune mem;
virDomainVcpuDefPtr *vcpus;
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index a22265606c..13f415b23c 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -17,6 +17,8 @@ UTIL_SOURCES = \
util/virauthconfig.h \
util/virbitmap.c \
util/virbitmap.h \
+ util/virblkio.c \
+ util/virblkio.h \
util/virbuffer.c \
util/virbuffer.h \
util/virperf.c \
diff --git a/src/util/virblkio.c b/src/util/virblkio.c
new file mode 100644
index 0000000000..9711077ee8
--- /dev/null
+++ b/src/util/virblkio.c
@@ -0,0 +1,37 @@
+/*
+ * virblkio.c: Block IO helpers
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * 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/>;.
+ *
+ * Authors:
+ * Fabiano Fidêncio <fidencio(a)redhat.com>
+ */
+
+#include <config.h>
+
+#include "viralloc.h"
+#include "virblkio.h"
+
+void
+virBlkioDeviceArrayClear(virBlkioDevicePtr devices,
+ int ndevices)
+{
+ size_t i;
+
+ for (i = 0; i < ndevices; i++)
+ VIR_FREE(devices[i].path);
+}
diff --git a/src/util/virblkio.h b/src/util/virblkio.h
new file mode 100644
index 0000000000..dcaeaaf1f0
--- /dev/null
+++ b/src/util/virblkio.h
@@ -0,0 +1,52 @@
+/*
+ * virblkio.h: Block IO definitions and helpers
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * 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/>;.
+ *
+ * Author: Fabiano Fidêncio <fidencio(a)redhat.com>
+ */
+
+#ifndef __VIR_BLKIO_H__
+# define __VIR_BLKIO_H__
+
+#include "virutil.h"
Oops. You need to install cppi ;-)
The same problem is in 2/4.
+
+typedef struct _virBlkioDevice virBlkioDevice;
+typedef virBlkioDevice *virBlkioDevicePtr;
+struct _virBlkioDevice {
+ char *path;
+ unsigned int weight;
+ unsigned int riops;
+ unsigned int wiops;
+ unsigned long long rbps;
+ unsigned long long wbps;
+};
+
+
+typedef struct _virBlkioTune virBlkioTune;
+typedef virBlkioTune *virBlkioTunePtr;
+struct _virBlkioTune {
+ unsigned int weight;
+
+ size_t ndevices;
+ virBlkioDevicePtr devices;
+};
+
+void virBlkioDeviceArrayClear(virBlkioDevicePtr deviceWeights,
+ int ndevices);
+
+#endif /* __VIR_BLKIO_H__ */
You need to invent a new section to src/libvirt_private.syms so that
this symbol is correctly exported.
The rest of the patches looks okay.
Michal
4:57 p.m.
New subject: [libvirt] [libvirt PATCH v2 2/4] domain_conf: split out virDomainMemtune and virDomainHugePage definitions
Let's move those to their own newly created header (src/util/virmem.h)
as this will help us to easily start sharing the cgroup code that's
duplicated between QEMU and LXC.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/conf/domain_conf.c | 11 +++----
src/conf/domain_conf.h | 43 ++-------------------------
src/qemu/qemu_command.c | 4 +--
src/util/virmem.h | 66 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 76 insertions(+), 48 deletions(-)
create mode 100644 src/util/virmem.h
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6ce50f712a..bd22ce6e7d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -60,6 +60,7 @@
#include "virhostdev.h"
#include "virmdev.h"
#include "virblkio.h"
+#include "virmem.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -6166,7 +6167,7 @@ virDomainDefLifecycleActionValidate(const virDomainDef *def)
static int
virDomainDefMemtuneValidate(const virDomainDef *def)
{
- const virDomainMemtune *mem = &(def->mem);
+ const virMemTune *mem = &(def->mem);
size_t i;
ssize_t pos = virDomainNumaGetNodeCount(def->numa) - 1;
@@ -18197,7 +18198,7 @@ virDomainDefMaybeAddInput(virDomainDefPtr def,
static int
virDomainHugepagesParseXML(xmlNodePtr node,
xmlXPathContextPtr ctxt,
- virDomainHugePagePtr hugepage)
+ virMemHugePagePtr hugepage)
{
int ret = -1;
xmlNodePtr oldnode = ctxt->node;
@@ -26841,7 +26842,7 @@ virDomainResourceDefFormat(virBufferPtr buf,
static int
virDomainHugepagesFormatBuf(virBufferPtr buf,
- virDomainHugePagePtr hugepage)
+ virMemHugePagePtr hugepage)
{
int ret = -1;
@@ -26865,7 +26866,7 @@ virDomainHugepagesFormatBuf(virBufferPtr buf,
static void
virDomainHugepagesFormat(virBufferPtr buf,
- virDomainHugePagePtr hugepages,
+ virMemHugePagePtr hugepages,
size_t nhugepages)
{
size_t i;
@@ -27403,7 +27404,7 @@ virDomainIOMMUDefFormat(virBufferPtr buf,
static int
virDomainMemtuneFormat(virBufferPtr buf,
- const virDomainMemtune *mem)
+ const virMemTune *mem)
{
virBuffer childBuf = VIR_BUFFER_INITIALIZER;
int ret = -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e9e6b6d6c4..10acc39861 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -58,6 +58,7 @@
# include "virsavecookie.h"
# include "virresctrl.h"
# include "virblkio.h"
+# include "virmem.h"
/* forward declarations of all device types, required by
* virDomainDeviceDef
@@ -2180,14 +2181,6 @@ struct _virDomainResourceDef {
char *partition;
};
-typedef struct _virDomainHugePage virDomainHugePage;
-typedef virDomainHugePage *virDomainHugePagePtr;
-
-struct _virDomainHugePage {
- virBitmapPtr nodemask; /* guest's NUMA node mask */
- unsigned long long size; /* hugepage size in KiB */
-};
-
# define VIR_DOMAIN_CPUMASK_LEN 1024
typedef struct _virDomainIOThreadIDDef virDomainIOThreadIDDef;
@@ -2247,38 +2240,6 @@ struct _virDomainVcpuDef {
virObjectPtr privateData;
};
-typedef struct _virDomainMemtune virDomainMemtune;
-typedef virDomainMemtune *virDomainMemtunePtr;
-
-struct _virDomainMemtune {
- /* total memory size including memory modules in kibibytes, this field
- * should be accessed only via accessors */
- unsigned long long total_memory;
- unsigned long long cur_balloon; /* in kibibytes, capped at ulong thanks
- to virDomainGetInfo */
-
- virDomainHugePagePtr hugepages;
- size_t nhugepages;
-
- /* maximum supported memory for a guest, for hotplugging */
- unsigned long long max_memory; /* in kibibytes */
- unsigned int memory_slots; /* maximum count of RAM memory slots */
-
- bool nosharepages;
- bool locked;
- int dump_core; /* enum virTristateSwitch */
- unsigned long long hard_limit; /* in kibibytes, limit at off_t bytes */
- unsigned long long soft_limit; /* in kibibytes, limit at off_t bytes */
- unsigned long long min_guarantee; /* in kibibytes, limit at off_t bytes */
- unsigned long long swap_hard_limit; /* in kibibytes, limit at off_t bytes */
-
- int source; /* enum virDomainMemorySource */
- int access; /* enum virDomainMemoryAccess */
- int allocation; /* enum virDomainMemoryAllocation */
-
- virTristateBool discard;
-};
-
typedef struct _virDomainPowerManagement virDomainPowerManagement;
typedef virDomainPowerManagement *virDomainPowerManagementPtr;
@@ -2380,7 +2341,7 @@ struct _virDomainDef {
char *description;
virBlkioTune blkio;
- virDomainMemtune mem;
+ virMemTune mem;
virDomainVcpuDefPtr *vcpus;
size_t maxvcpus;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ff9589f593..3511505389 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3202,8 +3202,8 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
if (pagesize == 0) {
- virDomainHugePagePtr master_hugepage = NULL;
- virDomainHugePagePtr hugepage = NULL;
+ virMemHugePagePtr master_hugepage = NULL;
+ virMemHugePagePtr hugepage = NULL;
bool thisHugepage = false;
/* Find the huge page size we want to use */
diff --git a/src/util/virmem.h b/src/util/virmem.h
new file mode 100644
index 0000000000..8f73218ac8
--- /dev/null
+++ b/src/util/virmem.h
@@ -0,0 +1,66 @@
+/*
+ * virmem.h: Memory definitions and helpers
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * 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/>;.
+ *
+ * Author: Fabiano Fidêncio <fidencio(a)redhat.com>
+ */
+
+#ifndef __VIR_MEM_H__
+# define __VIR_MEM_H__
+
+#include "virutil.h"
+
+typedef struct _virMemHugePage virMemHugePage;
+typedef virMemHugePage *virMemHugePagePtr;
+struct _virMemHugePage {
+ virBitmapPtr nodemask; /* guest's NUMA node mask */
+ unsigned long long size; /* hugepage size in KiB */
+};
+
+typedef struct _virMemTune virMemTune;
+typedef virMemTune *virMemTunePtr;
+struct _virMemTune {
+ /* total memory size including memory modules in kibibytes, this field
+ * should be accessed only via accessors */
+ unsigned long long total_memory;
+ unsigned long long cur_balloon; /* in kibibytes, capped at ulong thanks
+ to virDomainGetInfo */
+
+ virMemHugePagePtr hugepages;
+ size_t nhugepages;
+
+ /* maximum supported memory for a guest, for hotplugging */
+ unsigned long long max_memory; /* in kibibytes */
+ unsigned int memory_slots; /* maximum count of RAM memory slots */
+
+ bool nosharepages;
+ bool locked;
+ int dump_core; /* enum virTristateSwitch */
+ unsigned long long hard_limit; /* in kibibytes, limit at off_t bytes */
+ unsigned long long soft_limit; /* in kibibytes, limit at off_t bytes */
+ unsigned long long min_guarantee; /* in kibibytes, limit at off_t bytes */
+ unsigned long long swap_hard_limit; /* in kibibytes, limit at off_t bytes */
+
+ int source; /* enum virDomainMemorySource */
+ int access; /* enum virDomainMemoryAccess */
+ int allocation; /* enum virDomainMemoryAllocation */
+
+ virTristateBool discard;
+};
+
+#endif /* __VIR_MEM_H__ */
--
2.17.1
4:57 p.m.
New subject: [libvirt] [libvirt PATCH v2 3/4] vircgroup: Add virCgroupSetupBlkioTune()
virCgroupSetupBlkioTune() has been introduced in order to remove the
code duplication present between virLXCCgroupSetupBlkioTune() and
qemuSetupBlkioCgroup().
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/lxc/lxc_cgroup.c | 49 +-----------------------------------
src/qemu/qemu_cgroup.c | 47 +---------------------------------
src/util/vircgroup.c | 54 ++++++++++++++++++++++++++++++++++++++++
src/util/vircgroup.h | 3 +++
5 files changed, 60 insertions(+), 94 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0fc5314b02..9a85575cce 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1576,6 +1576,7 @@ virCgroupSetMemoryHardLimit;
virCgroupSetMemorySoftLimit;
virCgroupSetMemSwapHardLimit;
virCgroupSetOwner;
+virCgroupSetupBlkioTune;
virCgroupSupportsCpuBW;
virCgroupTerminateMachine;
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index d93a19d684..4a95f2a8b0 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -106,54 +106,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 &&
- (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;
+ return virCgroupSetupBlkioTune(cgroup, &def->blkio);
}
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 43e17d786e..0e53678faa 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -514,7 +514,6 @@ static int
qemuSetupBlkioCgroup(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- size_t i;
if (!virCgroupHasController(priv->cgroup,
VIR_CGROUP_CONTROLLER_BLKIO)) {
@@ -527,51 +526,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 virCgroupSetupBlkioTune(priv->cgroup, &vm->def->blkio);
}
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 64507bf8aa..a08b6f4869 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -4820,3 +4820,57 @@ virCgroupDelThread(virCgroupPtr cgroup,
return 0;
}
+
+
+int
+virCgroupSetupBlkioTune(virCgroupPtr cgroup,
+ virBlkioTunePtr 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 &&
+ (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;
+}
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index ee3b7c7222..2908f70372 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -27,6 +27,7 @@
# include "virutil.h"
# include "virbitmap.h"
+# include "virblkio.h"
struct _virCgroup;
typedef struct _virCgroup virCgroup;
@@ -286,4 +287,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
int virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller);
bool virCgroupControllerAvailable(int controller);
+
+int virCgroupSetupBlkioTune(virCgroupPtr cgroup, virBlkioTunePtr blkio);
#endif /* __VIR_CGROUP_H__ */
--
2.17.1
4:57 p.m.
New subject: [libvirt] [libvirt PATCH v2 4/4] vircgroup: Add virCgroupSetupMemTune()
virCgroupSetupMemTune() has been introduced in order to remove the code
duplication present between virLXCCgroupSetupMemTune() and
qemuSetupMemoryCgroup().
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/lxc/lxc_cgroup.c | 20 ++------------------
src/qemu/qemu_cgroup.c | 14 +-------------
src/util/vircgroup.c | 20 ++++++++++++++++++++
src/util/vircgroup.h | 4 ++++
5 files changed, 28 insertions(+), 31 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9a85575cce..ffee878e34 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1577,6 +1577,7 @@ virCgroupSetMemorySoftLimit;
virCgroupSetMemSwapHardLimit;
virCgroupSetOwner;
virCgroupSetupBlkioTune;
+virCgroupSetupMemTune;
virCgroupSupportsCpuBW;
virCgroupTerminateMachine;
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 4a95f2a8b0..95311fde9e 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -113,26 +113,10 @@ static int virLXCCgroupSetupBlkioTune(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;
+ return -1;
- ret = 0;
- cleanup:
- return ret;
+ return virCgroupSetupMemTune(cgroup, &def->mem);
}
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 0e53678faa..205098ec30 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -547,19 +547,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 virCgroupSetupMemTune(priv->cgroup, &vm->def->mem);
}
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index a08b6f4869..3973f6da61 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -4874,3 +4874,23 @@ virCgroupSetupBlkioTune(virCgroupPtr cgroup,
return 0;
}
+
+
+int
+virCgroupSetupMemTune(virCgroupPtr cgroup,
+ virMemTunePtr 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/util/vircgroup.h b/src/util/vircgroup.h
index 2908f70372..ad88da62d4 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -28,6 +28,7 @@
# include "virutil.h"
# include "virbitmap.h"
# include "virblkio.h"
+# include "virmem.h"
struct _virCgroup;
typedef struct _virCgroup virCgroup;
@@ -289,4 +290,7 @@ int virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller);
bool virCgroupControllerAvailable(int controller);
int virCgroupSetupBlkioTune(virCgroupPtr cgroup, virBlkioTunePtr blkio);
+int virCgroupSetupMemTune(virCgroupPtr cgroup, virMemTunePtr mem);
+
+
#endif /* __VIR_CGROUP_H__ */
--
2.17.1
6:46 p.m.
On Wed, Sep 12, 2018 at 10:57:32AM +0200, Fabiano Fidêncio wrote:
virLXCCgroupSetupBlkioTune() and qemuSetupBlkioCgroup() and
virLXCCgroupSetupCpuTune() and qemuSetupCpuCgroup() are the most similar
functions between QEMU and LXC code.
Let's move their common code to virCgroup.
Mind that the first two patches are basically preparing the ground for
the changes introduced in the last two patches.
Hi, definitely good idea to remove code duplication!
We have similar issue with the virDomainSetBlkioParameters for QEMU and
LXC drivers. The code to set cgroup values is the same.
Since the common object is domain how about introducing
virDomainSetupBlkioTune and virDomainSetupMemTune and move it into
domain_conf.c, that way we don't have to extract domain specific data
into src/util.
Another benefit is that it will not cause me merge conflicts because I'm
rewriting cgroup code and adding support for cgroup v2.
Pavel
9:18 p.m.
On 09/12/2018 12:46 PM, Pavel Hrdina wrote:
On Wed, Sep 12, 2018 at 10:57:32AM +0200, Fabiano Fidêncio wrote:
> virLXCCgroupSetupBlkioTune() and qemuSetupBlkioCgroup() and
> virLXCCgroupSetupCpuTune() and qemuSetupCpuCgroup() are the most similar
> functions between QEMU and LXC code.
>
> Let's move their common code to virCgroup.
>
> Mind that the first two patches are basically preparing the ground for
> the changes introduced in the last two patches.
Hi, definitely good idea to remove code duplication!
We have similar issue with the virDomainSetBlkioParameters for QEMU and
LXC drivers. The code to set cgroup values is the same.
Since the common object is domain how about introducing
virDomainSetupBlkioTune and virDomainSetupMemTune and move it into
domain_conf.c, that way we don't have to extract domain specific data
into src/util.
I'd rather remove stuff from doman_conf.c than add something there. It's
already the biggest file by far margin:
libvirt.git $ find . -type f -iname \*.c -exec du -h {} \; | sort -h
416K ./src/qemu/qemu_domain.c
696K ./src/qemu/qemu_driver.c
956K ./src/conf/domain_conf.c
And moving code from domain_conf is something we do from time to time.
Just look at all those virnet* includes at the beginning of domain_conf.h.
Another reason for moving the code out is that blkio tune is not domain
specific. In the future, we might want to limit say iohelper and thus
move it into its specific cgroup.
I'm not that convinced about 2/4 to be honest. Memory specification is
pretty much domain centric. But it follows my first point - moving code
out from domain_conf.c. And it de-duplicates the code.
Another benefit is that it will not cause me merge conflicts because I'm
rewriting cgroup code and adding support for cgroup v2.
I don't think that code movement should cause that big of a trouble.
You'll get some context conflicts but that's always the case with 30+
unmerged patches.
Michal
6:59 a.m.
On Wed, Sep 12, 2018 at 03:18:26PM +0200, Michal Privoznik wrote:
On 09/12/2018 12:46 PM, Pavel Hrdina wrote:
> On Wed, Sep 12, 2018 at 10:57:32AM +0200, Fabiano Fidêncio wrote:
>> virLXCCgroupSetupBlkioTune() and qemuSetupBlkioCgroup() and
>> virLXCCgroupSetupCpuTune() and qemuSetupCpuCgroup() are the most similar
>> functions between QEMU and LXC code.
>>
>> Let's move their common code to virCgroup.
>>
>> Mind that the first two patches are basically preparing the ground for
>> the changes introduced in the last two patches.
>
> Hi, definitely good idea to remove code duplication!
>
> We have similar issue with the virDomainSetBlkioParameters for QEMU and
> LXC drivers. The code to set cgroup values is the same.
>
> Since the common object is domain how about introducing
> virDomainSetupBlkioTune and virDomainSetupMemTune and move it into
> domain_conf.c, that way we don't have to extract domain specific data
> into src/util.
I'd rather remove stuff from doman_conf.c than add something there. It's
already the biggest file by far margin:
libvirt.git $ find . -type f -iname \*.c -exec du -h {} \; | sort -h
416K ./src/qemu/qemu_domain.c
696K ./src/qemu/qemu_driver.c
956K ./src/conf/domain_conf.c
And moving code from domain_conf is something we do from time to time.
Just look at all those virnet* includes at the beginning of domain_conf.h.
Another reason for moving the code out is that blkio tune is not domain
specific. In the future, we might want to limit say iohelper and thus
move it into its specific cgroup.
I'm not that convinced about 2/4 to be honest. Memory specification is
pretty much domain centric.
Both seem to follow cgroups to a point, so they're process-centric, not
domain-centric. So if they're needed on lower (src/util) layers,
separating them makes sense.
(Not that domain_conf does not deserve to get both the parser and the
formatter to be split out at this line count)
But it follows my first point - moving code
out from domain_conf.c. And it de-duplicates the code.
>
> Another benefit is that it will not cause me merge conflicts because I'm
> rewriting cgroup code and adding support for cgroup v2.
Such downstream branches should not interfere with upstream libvirt
development. IOW: it's your opportunity as a maintainer to offer to
merge this patch on top of your changes if merging them now would cause
you too much trouble. ;)
Jano
I don't think that code movement should cause that big of a trouble.
You'll get some context conflicts but that's always the case with 30+
unmerged patches.
Michal
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
7:12 p.m.
On Thu, Sep 13, 2018 at 12:59:48AM +0200, Ján Tomko wrote:
On Wed, Sep 12, 2018 at 03:18:26PM +0200, Michal Privoznik wrote:
> On 09/12/2018 12:46 PM, Pavel Hrdina wrote:
> > On Wed, Sep 12, 2018 at 10:57:32AM +0200, Fabiano Fidêncio wrote:
> > > virLXCCgroupSetupBlkioTune() and qemuSetupBlkioCgroup() and
> > > virLXCCgroupSetupCpuTune() and qemuSetupCpuCgroup() are the most similar
> > > functions between QEMU and LXC code.
> > >
> > > Let's move their common code to virCgroup.
> > >
> > > Mind that the first two patches are basically preparing the ground for
> > > the changes introduced in the last two patches.
> >
> > Hi, definitely good idea to remove code duplication!
> >
> > We have similar issue with the virDomainSetBlkioParameters for QEMU and
> > LXC drivers. The code to set cgroup values is the same.
> >
> > Since the common object is domain how about introducing
> > virDomainSetupBlkioTune and virDomainSetupMemTune and move it into
> > domain_conf.c, that way we don't have to extract domain specific data
> > into src/util.
>
> I'd rather remove stuff from doman_conf.c than add something there. It's
> already the biggest file by far margin:
>
> libvirt.git $ find . -type f -iname \*.c -exec du -h {} \; | sort -h
> 416K ./src/qemu/qemu_domain.c
> 696K ./src/qemu/qemu_driver.c
> 956K ./src/conf/domain_conf.c
>
> And moving code from domain_conf is something we do from time to time.
> Just look at all those virnet* includes at the beginning of domain_conf.h.
>
> Another reason for moving the code out is that blkio tune is not domain
> specific. In the future, we might want to limit say iohelper and thus
> move it into its specific cgroup.
>
> I'm not that convinced about 2/4 to be honest. Memory specification is
> pretty much domain centric.
Both seem to follow cgroups to a point, so they're process-centric, not
domain-centric. So if they're needed on lower (src/util) layers,
separating them makes sense.
(Not that domain_conf does not deserve to get both the parser and the
formatter to be split out at this line count)
So I definitely agree that we should move a lot of code out of
domain_conf.c, the main reason why I actually suggested to move it
there in the fist place is because there is virDomainObj.
The ideal thing would be to separate all the virDomainObj code out
of domain_conf.c like we have for a lot of other objects.
> But it follows my first point - moving code
> out from domain_conf.c. And it de-duplicates the code.
>
> >
> > Another benefit is that it will not cause me merge conflicts because I'm
> > rewriting cgroup code and adding support for cgroup v2.
Such downstream branches should not interfere with upstream libvirt
development. IOW: it's your opportunity as a maintainer to offer to
merge this patch on top of your changes if merging them now would cause
you too much trouble. ;)
I was just mentioning it to inform upstream that there is such work
going on, it was not an argument against this change :).
Pavel
2240
days inactive
2241
days old
9 comments
4 participants
participants (4)
-
Fabiano Fidêncio -
Ján Tomko -
Michal Privoznik -
Pavel Hrdina