[libvirt] [PATCH v2 4/9] vircgroup: introduce virCgroupTaskFlags

[Ján Tomko]

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 at 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 at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180924/25c68b72/attachment-0001.sig>