On Thu, Sep 20, 2018 at 10:54:33AM +0200, Pavel Hrdina wrote:
>Use flags in virCgroupAddTaskInternal instead of boolean parameter.
>Following patch will add new flag to indicate thread instead of process.
>
>Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>---
>
>Notes:
> changes in v2:
> - added comments for new flags
>
> src/util/vircgroup.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
>diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
>index 30ffb658d9..490cbc03a0 100644
>--- a/src/util/vircgroup.c
>+++ b/src/util/vircgroup.c
>@@ -1152,8 +1152,21 @@ virCgroupNew(pid_t pid,
> }
>
>
>+typedef enum {
>+ /* Adds a whole process with all threads to specific cgroup except
>+ * to systemd named controller. */
>+ VIR_CGROUP_TASK_PROCESS = 0,
>+
Having an OR-able flag with a value of 0 is odd.
>+ /* Same as VIR_CGROUP_TASK_PROCESS but it also adds the task to systemd
>+ * named controller. */
>+ VIR_CGROUP_TASK_SYSTEMD = 1 << 0,
>+} virCgroupTaskFlags;
>+
>+
> static int
>-virCgroupAddTaskInternal(virCgroupPtr group, pid_t pid, bool withSystemd)
>+virCgroupAddTaskInternal(virCgroupPtr group,
>+ pid_t pid,
>+ unsigned int flags)
> {
> int ret = -1;
> size_t i;
>@@ -1166,7 +1179,8 @@ virCgroupAddTaskInternal(virCgroupPtr group, pid_t pid, bool withSystemd)
> /* We must never add tasks in systemd's hierarchy
> * unless we're intentionally trying to move a
> * task into a systemd machine scope */
>- if (i == VIR_CGROUP_CONTROLLER_SYSTEMD && !withSystemd)
>+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD &&
>+ !(flags & VIR_CGROUP_TASK_SYSTEMD))
> continue;
>
> if (virCgroupSetValueI64(group, i, "tasks", pid) < 0)
>@@ -1192,7 +1206,7 @@ virCgroupAddTaskInternal(virCgroupPtr group, pid_t pid, bool withSystemd)
> int
> virCgroupAddProcess(virCgroupPtr group, pid_t pid)
> {
>- return virCgroupAddTaskInternal(group, pid, false);
>+ return virCgroupAddTaskInternal(group, pid, VIR_CGROUP_TASK_PROCESS);
> }
>
> /**
>@@ -1209,7 +1223,9 @@ virCgroupAddProcess(virCgroupPtr group, pid_t pid)
> int
> virCgroupAddMachineProcess(virCgroupPtr group, pid_t pid)
> {
>- return virCgroupAddTaskInternal(group, pid, true);
>+ return virCgroupAddTaskInternal(group, pid,
>+ VIR_CGROUP_TASK_PROCESS |
>+ VIR_CGROUP_TASK_SYSTEMD);
> }
Actually OR-ing it with something is odder.
If you either:
* Give TASK_PROCESS a value
* only use TASK_PROCESS as an alias for 0 and not OR it with anything:
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list