1:09 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.
Fabiano Fidêncio (2):
vircgroup: Add virCgroupSetupBlkiotune()
vircgroup: Add virCgroupSetupMemtune()
src/libvirt_private.syms | 2 ++
src/lxc/lxc_cgroup.c | 69 ++------------------------------------------
src/qemu/qemu_cgroup.c | 61 ++-------------------------------------
src/util/vircgroup.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/vircgroup.h | 4 +++
5 files changed, 85 insertions(+), 125 deletions(-)
--
2.14.3
1:09 p.m.
New subject: [libvirt] [PATCH 1/2] 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 <fabiano(a)fidencio.org>
---
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 6001635916..4bec93c786 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1545,6 +1545,7 @@ virCgroupSetMemoryHardLimit;
virCgroupSetMemorySoftLimit;
virCgroupSetMemSwapHardLimit;
virCgroupSetOwner;
+virCgroupSetupBlkiotune;
virCgroupSupportsCpuBW;
virCgroupTerminateMachine;
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 8e937ec389..2158c1c12b 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 546a4c8e63..47ee23796c 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -510,7 +510,6 @@ static int
qemuSetupBlkioCgroup(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- size_t i;
if (!virCgroupHasController(priv->cgroup,
VIR_CGROUP_CONTROLLER_BLKIO)) {
@@ -523,51 +522,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 0a31947b0d..4e57f1322f 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -4920,3 +4920,57 @@ virCgroupDelThread(virCgroupPtr cgroup,
return 0;
}
+
+
+int
+virCgroupSetupBlkiotune(virCgroupPtr cgroup,
+ virDomainBlkiotunePtr 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 d833927678..bd7e7c6d70 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -27,6 +27,7 @@
# include "virutil.h"
# include "virbitmap.h"
+# include "conf/domain_conf.h"
struct virCgroup;
typedef struct virCgroup *virCgroupPtr;
@@ -297,4 +298,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
int virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller);
bool virCgroupControllerAvailable(int controller);
+
+int virCgroupSetupBlkiotune(virCgroupPtr cgroup, virDomainBlkiotunePtr blkio);
#endif /* __VIR_CGROUP_H__ */
--
2.14.3
9:16 a.m.
New subject: [libvirt] [PATCH 1/2] vircgroup: Add virCgroupSetupBlkiotune()
On 06/03/2018 08:09 PM, Fabiano Fidêncio wrote:
virCgroupSetupBlkiotune() has been introduced in order to remove the
code duplication present between virLXCCgroupSetupBlkioTune() and
qemuSetupBlkioCgroup().
Signed-off-by: Fabiano Fidêncio <fabiano(a)fidencio.org>
---
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/util/vircgroup.h b/src/util/vircgroup.h
index d833927678..bd7e7c6d70 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -27,6 +27,7 @@
# include "virutil.h"
# include "virbitmap.h"
+# include "conf/domain_conf.h"
I don't think we want to do this. The point of this code separation is
to use src/util/ independently of parsing code (even though we break
this rule in two places: src/util/virhostdev.h and
src/util/virclosecallbacks.h).
What we can do is moving virDomainBlkiotunePtr into vircgroup.h (in
which case it will need renaming too).
Michal
10:23 a.m.
New subject: [libvirt] [PATCH 1/2] vircgroup: Add virCgroupSetupBlkiotune()
On Tue, Jun 5, 2018 at 4:16 PM, Michal Privoznik <mprivozn(a)redhat.com> wrote:
On 06/03/2018 08:09 PM, Fabiano Fidêncio wrote:
> virCgroupSetupBlkiotune() has been introduced in order to remove the
> code duplication present between virLXCCgroupSetupBlkioTune() and
> qemuSetupBlkioCgroup().
>
> Signed-off-by: Fabiano Fidêncio <fabiano(a)fidencio.org>
> ---
> 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/util/vircgroup.h b/src/util/vircgroup.h
> index d833927678..bd7e7c6d70 100644
> --- a/src/util/vircgroup.h
> +++ b/src/util/vircgroup.h
> @@ -27,6 +27,7 @@
>
> # include "virutil.h"
> # include "virbitmap.h"
> +# include "conf/domain_conf.h"
I don't think we want to do this. The point of this code separation is
to use src/util/ independently of parsing code (even though we break
this rule in two places: src/util/virhostdev.h and
src/util/virclosecallbacks.h).
Hmm. Makes sense.
What we can do is moving virDomainBlkiotunePtr into vircgroup.h (in
which case it will need renaming too).
Right, I'll re-spin the patches soon and submit a v2.
Thanks for the review!
Michal
Best Regards,
--
Fabiano Fidêncio
9:09 a.m.
New subject: [libvirt] [PATCH 1/2] vircgroup: Add virCgroupSetupBlkiotune()
On Tue, Jun 05, 2018 at 05:23:18PM +0200, Fabiano Fidêncio wrote:
On Tue, Jun 5, 2018 at 4:16 PM, Michal Privoznik
<mprivozn(a)redhat.com> wrote:
> On 06/03/2018 08:09 PM, Fabiano Fidêncio wrote:
>> virCgroupSetupBlkiotune() has been introduced in order to remove the
>> code duplication present between virLXCCgroupSetupBlkioTune() and
>> qemuSetupBlkioCgroup().
>>
>> Signed-off-by: Fabiano Fidêncio <fabiano(a)fidencio.org>
>> ---
>> 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/util/vircgroup.h b/src/util/vircgroup.h
>> index d833927678..bd7e7c6d70 100644
>> --- a/src/util/vircgroup.h
>> +++ b/src/util/vircgroup.h
>> @@ -27,6 +27,7 @@
>>
>> # include "virutil.h"
>> # include "virbitmap.h"
>> +# include "conf/domain_conf.h"
>
> I don't think we want to do this. The point of this code separation is
> to use src/util/ independently of parsing code (even though we break
> this rule in two places: src/util/virhostdev.h and
> src/util/virclosecallbacks.h).
Hmm. Makes sense.
This is also pointed out by 'make syntax-check':
src/util/vircgroup.h:30:# include "conf/domain_conf.h"
maint.mk: unsafe cross-directory include
make: *** [cfg.mk:786: sc_prohibit_cross_inclusion] Error 1
Perhaps our HACKING page [0] could use a rework to mention the important
stuff (running make check and make syntax-check with cppi installed) and
we can have the detailed coding style rules somewhere else (many of
those are caught by syntax-check anyway).
Jano
[0] https://libvirt.org/hacking.html
1:10 p.m.
New subject: [libvirt] [PATCH 2/2] 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 <fabiano(a)fidencio.org>
---
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 | 1 +
5 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4bec93c786..39fce2e169 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1546,6 +1546,7 @@ virCgroupSetMemorySoftLimit;
virCgroupSetMemSwapHardLimit;
virCgroupSetOwner;
virCgroupSetupBlkiotune;
+virCgroupSetupMemtune;
virCgroupSupportsCpuBW;
virCgroupTerminateMachine;
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 2158c1c12b..a5e71aee53 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 47ee23796c..7b6a4ec796 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -543,19 +543,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 4e57f1322f..88f213e6da 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -4974,3 +4974,23 @@ virCgroupSetupBlkiotune(virCgroupPtr cgroup,
return 0;
}
+
+
+int
+virCgroupSetupMemtune(virCgroupPtr cgroup,
+ virDomainMemtunePtr 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 bd7e7c6d70..5d7973b272 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -300,4 +300,5 @@ int virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller);
bool virCgroupControllerAvailable(int controller);
int virCgroupSetupBlkiotune(virCgroupPtr cgroup, virDomainBlkiotunePtr blkio);
+int virCgroupSetupMemtune(virCgroupPtr cgroup, virDomainMemtunePtr mem);
#endif /* __VIR_CGROUP_H__ */
--
2.14.3
2356
days inactive
2364
days old
5 comments
3 participants
participants (3)
-
Fabiano Fidêncio -
Ján Tomko -
Michal Privoznik