On Thu, Sep 27, 2018 at 08:40:03AM -0400, John Ferlan wrote: If virCgroupAddProcess fails virCgroupNewMachineManual returns 0 and NULL, however, qemuExtTPMSetupCgroup returns -1 and NULL. The difference with qemuExtDevicesSetupCgroup is adding another process to existing cgroups that were created for the domain itself. The cleanup is done for the whole domain. If you follow the cleanup path: qemuSetupCgroupForExtDevices fails -> goto cleanup, there is no qemuRemoveCgroup or virCgroupRemove, this will result that qemuProcessLaunch fails -> goto stop, there we will call qemuProcessStop where we will call qemuRemoveCgroup which will remove cgroups. So that part of code is correct, we don't want to remove cgroups if the qemu process is running, we will do it later once we stop the qemu process. Before this patch: - when virCgroupEnableMissingControllers failed we returned -1 and cgroup ptr or NULL - when virCgroupAddTask failed we returned 0 and NULL After this patch: - when virCgroupEnableMissingControllers fails we return -1 and NULL - when virCgroupAddTask fails we return -1 and NULL The change for virCgroupEnableMissingControllers is not an issue because we always return -1 therefore starting guest always fails and we always call virCgroupRemove. The change for virCgroupAddTask changed the behavior, previously we continued with starting the guest but no cgroups were used for that guest, after this patch starting guest fails. So there are two issues after this patch, we need to return 0 if virCgroupAddTask fails and we need to modify virCgroupRemove to accept NULL as well. Marc's patch fixes the second issue, but is incorrect since we tend to have check for NULL inside the function. I'll post new patches to address both issues. Pavel

If we are on host with systemd we need to build cgroup hierarchy ourselves for controllers that are not managed by systemd. As a starting parent we need to force root group because virCgroupMakeGroup() takes that parent in order to inherit values for cpuset controller. By default cpuset controller is managed by systemd so we will never hit the issue but for v2 cgroups we need to use parent cgroup every time. Reviewed-by: Fabiano Fidêncio <fidencio(a)redhat.com&gt; Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index f9e387c86d..13f5e0d83a 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1491,7 +1491,7 @@ virCgroupEnableMissingControllers(char *path, int ret = -1; if (virCgroupNew(pidleader, - "", + "/", NULL, controllers, &parent) < 0) -- 2.17.1

In cgroup v2 we need to handle processes and threads differently, following patch will introduce virCgroupAddThread. Reviewed-by: Fabiano Fidêncio <fidencio(a)redhat.com&gt; Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/libvirt-lxc.c | 2 +- src/libvirt_private.syms | 4 ++-- src/lxc/lxc_controller.c | 4 ++-- src/qemu/qemu_process.c | 4 ++-- src/qemu/qemu_tpm.c | 2 +- src/util/vircgroup.c | 32 ++++++++++++++++---------------- src/util/vircgroup.h | 4 ++-- 7 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c index c9f2146487..9bf0174b95 100644 --- a/src/libvirt-lxc.c +++ b/src/libvirt-lxc.c @@ -306,7 +306,7 @@ int virDomainLxcEnterCGroup(virDomainPtr domain, if (virCgroupNewDetect(domain->id, -1, &cgroup) < 0) goto error; - if (virCgroupAddTask(cgroup, getpid()) < 0) + if (virCgroupAddProcess(cgroup, getpid()) < 0) goto error; virCgroupFree(&cgroup); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b9dabfef1b..eac66b0174 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1498,8 +1498,8 @@ virBufferVasprintf; # util/vircgroup.h -virCgroupAddMachineTask; -virCgroupAddTask; +virCgroupAddMachineProcess; +virCgroupAddProcess; virCgroupAllowAllDevices; virCgroupAllowDevice; virCgroupAllowDevicePath; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index d81c8e946d..62dfd09473 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -874,12 +874,12 @@ static int virLXCControllerSetupCgroupLimits(virLXCControllerPtr ctrl) ctrl->nicindexes))) goto cleanup; - if (virCgroupAddMachineTask(ctrl->cgroup, getpid()) < 0) + if (virCgroupAddMachineProcess(ctrl->cgroup, getpid()) < 0) goto cleanup; /* Add all qemu-nbd tasks to the cgroup */ for (i = 0; i < ctrl->nnbdpids; i++) { - if (virCgroupAddMachineTask(ctrl->cgroup, ctrl->nbdpids[i]) < 0) + if (virCgroupAddMachineProcess(ctrl->cgroup, ctrl->nbdpids[i]) < 0) goto cleanup; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 72a59dec55..249dac39f2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2549,7 +2549,7 @@ qemuProcessSetupPid(virDomainObjPtr vm, goto cleanup; /* Move the thread to the sub dir */ - if (virCgroupAddTask(cgroup, pid) < 0) + if (virCgroupAddProcess(cgroup, pid) < 0) goto cleanup; } @@ -2787,7 +2787,7 @@ qemuProcessStartManagedPRDaemon(virDomainObjPtr vm) } if (priv->cgroup && - virCgroupAddMachineTask(priv->cgroup, cpid) < 0) + virCgroupAddMachineProcess(priv->cgroup, cpid) < 0) goto cleanup; if (qemuSecurityDomainSetPathLabel(driver, vm, socketPath, true) < 0) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 278b262c48..c64114feac 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -905,7 +905,7 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver, _("Could not get process id of swtpm")); goto cleanup; } - if (virCgroupAddTask(cgroup, pid) < 0) + if (virCgroupAddProcess(cgroup, pid) < 0) goto cleanup; break; case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 13f5e0d83a..30ffb658d9 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1179,35 +1179,35 @@ virCgroupAddTaskInternal(virCgroupPtr group, pid_t pid, bool withSystemd) } /** - * virCgroupAddTask: + * virCgroupAddProcess: * - * @group: The cgroup to add a task to - * @pid: The pid of the task to add + * @group: The cgroup to add a process to + * @pid: The pid of the process to add * - * Will add the task to all controllers, except the + * Will add the process to all controllers, except the * systemd unit controller. * * Returns: 0 on success, -1 on error */ int -virCgroupAddTask(virCgroupPtr group, pid_t pid) +virCgroupAddProcess(virCgroupPtr group, pid_t pid) { return virCgroupAddTaskInternal(group, pid, false); } /** - * virCgroupAddMachineTask: + * virCgroupAddMachineProcess: * - * @group: The cgroup to add a task to - * @pid: The pid of the task to add + * @group: The cgroup to add a process to + * @pid: The pid of the process to add * - * Will add the task to all controllers, including the + * Will add the process to all controllers, including the * systemd unit controller. * * Returns: 0 on success, -1 on error */ int -virCgroupAddMachineTask(virCgroupPtr group, pid_t pid) +virCgroupAddMachineProcess(virCgroupPtr group, pid_t pid) { return virCgroupAddTaskInternal(group, pid, true); } @@ -1588,7 +1588,7 @@ virCgroupNewMachineSystemd(const char *name, goto error; } - if (virCgroupAddTask(*group, pidleader) < 0) + if (virCgroupAddProcess(*group, pidleader) < 0) goto error; return 0; @@ -1644,7 +1644,7 @@ virCgroupNewMachineManual(const char *name, group) < 0) goto cleanup; - if (virCgroupAddTask(*group, pidleader) < 0) { + if (virCgroupAddProcess(*group, pidleader) < 0) { virErrorPtr saved = virSaveLastError(); virCgroupRemove(*group); virCgroupFree(group); @@ -4194,8 +4194,8 @@ virCgroupPathOfController(virCgroupPtr group ATTRIBUTE_UNUSED, int -virCgroupAddTask(virCgroupPtr group ATTRIBUTE_UNUSED, - pid_t pid ATTRIBUTE_UNUSED) +virCgroupAddProcess(virCgroupPtr group ATTRIBUTE_UNUSED, + pid_t pid ATTRIBUTE_UNUSED) { virReportSystemError(ENXIO, "%s", _("Control groups not supported on this platform")); @@ -4204,8 +4204,8 @@ virCgroupAddTask(virCgroupPtr group ATTRIBUTE_UNUSED, int -virCgroupAddMachineTask(virCgroupPtr group ATTRIBUTE_UNUSED, - pid_t pid ATTRIBUTE_UNUSED) +virCgroupAddMachineProcess(virCgroupPtr group ATTRIBUTE_UNUSED, + pid_t pid ATTRIBUTE_UNUSED) { virReportSystemError(ENXIO, "%s", _("Control groups not supported on this platform")); diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index ee3b7c7222..bbd4c2ed57 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -118,8 +118,8 @@ int virCgroupPathOfController(virCgroupPtr group, const char *key, char **path); -int virCgroupAddTask(virCgroupPtr group, pid_t pid); -int virCgroupAddMachineTask(virCgroupPtr group, pid_t pid); +int virCgroupAddProcess(virCgroupPtr group, pid_t pid); +int virCgroupAddMachineProcess(virCgroupPtr group, pid_t pid); int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight); -- 2.17.1

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(a)redhat.com&gt; --- 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, + + /* 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); } -- 2.17.1

Once we introduce cgroup v2 support we need to handle processes and threads differently. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: changes in v2: - added comment for new flag src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 2 +- src/util/vircgroup.c | 32 ++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 1 + 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index eac66b0174..ad7ce57b65 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1500,6 +1500,7 @@ virBufferVasprintf; # util/vircgroup.h virCgroupAddMachineProcess; virCgroupAddProcess; +virCgroupAddThread; virCgroupAllowAllDevices; virCgroupAllowDevice; virCgroupAllowDevicePath; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 249dac39f2..00dcd5b580 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2549,7 +2549,7 @@ qemuProcessSetupPid(virDomainObjPtr vm, goto cleanup; /* Move the thread to the sub dir */ - if (virCgroupAddProcess(cgroup, pid) < 0) + if (virCgroupAddThread(cgroup, pid) < 0) goto cleanup; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 490cbc03a0..ea83c5f5b2 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1160,6 +1160,10 @@ typedef enum { /* Same as VIR_CGROUP_TASK_PROCESS but it also adds the task to systemd * named controller. */ VIR_CGROUP_TASK_SYSTEMD = 1 << 0, + + /* Moves only specific thread into cgroup except to systemd + * named controller. */ + VIR_CGROUP_TASK_THREAD = 1 << 1, } virCgroupTaskFlags; @@ -1228,6 +1232,24 @@ virCgroupAddMachineProcess(virCgroupPtr group, pid_t pid) VIR_CGROUP_TASK_SYSTEMD); } +/** + * virCgroupAddThread: + * + * @group: The cgroup to add a thread to + * @pid: The pid of the thread to add + * + * Will add the thread to all controllers, except the + * systemd unit controller. + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupAddThread(virCgroupPtr group, + pid_t pid) +{ + return virCgroupAddTaskInternal(group, pid, VIR_CGROUP_TASK_THREAD); +} + static int virCgroupSetPartitionSuffix(const char *path, char **res) @@ -4229,6 +4251,16 @@ virCgroupAddMachineProcess(virCgroupPtr group ATTRIBUTE_UNUSED, } +int +virCgroupAddThread(virCgroupPtr group ATTRIBUTE_UNUSED, + pid_t pid ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; +} + + int virCgroupGetBlkioIoServiced(virCgroupPtr group ATTRIBUTE_UNUSED, long long *bytes_read ATTRIBUTE_UNUSED, diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index bbd4c2ed57..1f676f21c3 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -120,6 +120,7 @@ int virCgroupPathOfController(virCgroupPtr group, int virCgroupAddProcess(virCgroupPtr group, pid_t pid); int virCgroupAddMachineProcess(virCgroupPtr group, pid_t pid); +int virCgroupAddThread(virCgroupPtr group, pid_t pid); int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight); -- 2.17.1

Reviewed-by: Fabiano Fidêncio <fidencio(a)redhat.com&gt; Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgroupmock.c | 73 ------------------------------------------- 1 file changed, 73 deletions(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index d512417789..73cf876645 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -212,13 +212,7 @@ static int make_controller(const char *path, mode_t mode) if (STRPREFIX(controller, "cpu,cpuacct")) { MAKE_FILE("cpu.cfs_period_us", "100000\n"); MAKE_FILE("cpu.cfs_quota_us", "-1\n"); - MAKE_FILE("cpu.rt_period_us", "1000000\n"); - MAKE_FILE("cpu.rt_runtime_us", "950000\n"); MAKE_FILE("cpu.shares", "1024\n"); - MAKE_FILE("cpu.stat", - "nr_periods 0\n" - "nr_throttled 0\n" - "throttled_time 0\n"); MAKE_FILE("cpuacct.stat", "user 216687025\n" "system 43421396\n"); @@ -233,46 +227,19 @@ static int make_controller(const char *path, mode_t mode) "709566900 0 0 0 0 0 0 0 444777342 0 0 0 0 0 0 0 " "5683512916 0 0 0 0 0 0 0 635751356 0 0 0 0 0 0 0\n"); } else if (STRPREFIX(controller, "cpuset")) { - MAKE_FILE("cpuset.cpu_exclusive", "1\n"); if (STREQ(controller, "cpuset")) MAKE_FILE("cpuset.cpus", "0-1"); else MAKE_FILE("cpuset.cpus", ""); /* Values don't inherit */ - MAKE_FILE("cpuset.mem_exclusive", "1\n"); - MAKE_FILE("cpuset.mem_hardwall", "0\n"); MAKE_FILE("cpuset.memory_migrate", "0\n"); - MAKE_FILE("cpuset.memory_pressure", "0\n"); - MAKE_FILE("cpuset.memory_pressure_enabled", "0\n"); - MAKE_FILE("cpuset.memory_spread_page", "0\n"); - MAKE_FILE("cpuset.memory_spread_slab", "0\n"); if (STREQ(controller, "cpuset")) MAKE_FILE("cpuset.mems", "0"); else MAKE_FILE("cpuset.mems", ""); /* Values don't inherit */ - MAKE_FILE("cpuset.sched_load_balance", "1\n"); - MAKE_FILE("cpuset.sched_relax_domain_level", "-1\n"); } else if (STRPREFIX(controller, "memory")) { - MAKE_FILE("memory.failcnt", "0\n"); - MAKE_FILE("memory.force_empty", ""); /* Write only */ - MAKE_FILE("memory.kmem.tcp.failcnt", "0\n"); - MAKE_FILE("memory.kmem.tcp.limit_in_bytes", "9223372036854775807\n"); - MAKE_FILE("memory.kmem.tcp.max_usage_in_bytes", "0\n"); - MAKE_FILE("memory.kmem.tcp.usage_in_bytes", "16384\n"); MAKE_FILE("memory.limit_in_bytes", "9223372036854775807\n"); - MAKE_FILE("memory.max_usage_in_bytes", "0\n"); - MAKE_FILE("memory.memsw.failcnt", ""); /* Not supported */ MAKE_FILE("memory.memsw.limit_in_bytes", ""); /* Not supported */ - MAKE_FILE("memory.memsw.max_usage_in_bytes", ""); /* Not supported */ MAKE_FILE("memory.memsw.usage_in_bytes", ""); /* Not supported */ - MAKE_FILE("memory.move_charge_at_immigrate", "0\n"); - MAKE_FILE("memory.numa_stat", - "total=367664 N0=367664\n" - "file=314764 N0=314764\n" - "anon=51999 N0=51999\n" - "unevictable=901 N0=901\n"); - MAKE_FILE("memory.oom_control", - "oom_kill_disable 0\n" - "under_oom 0\n"); MAKE_FILE("memory.soft_limit_in_bytes", "9223372036854775807\n"); MAKE_FILE("memory.stat", "cache 1336619008\n" @@ -304,50 +271,11 @@ static int make_controller(const char *path, mode_t mode) "recent_rotated_file 2547948\n" "recent_scanned_anon 113796164\n" "recent_scanned_file 8199863\n"); - MAKE_FILE("memory.swappiness", "60\n"); MAKE_FILE("memory.usage_in_bytes", "1455321088\n"); MAKE_FILE("memory.use_hierarchy", "0\n"); } else if (STRPREFIX(controller, "freezer")) { MAKE_FILE("freezer.state", "THAWED"); } else if (STRPREFIX(controller, "blkio")) { - MAKE_FILE("blkio.io_merged", - "8:0 Read 1100949\n" - "8:0 Write 2248076\n" - "8:0 Sync 63063\n" - "8:0 Async 3285962\n" - "8:0 Total 3349025\n"); - MAKE_FILE("blkio.io_queued", - "8:0 Read 0\n" - "8:0 Write 0\n" - "8:0 Sync 0\n" - "8:0 Async 0\n" - "8:0 Total 0\n"); - MAKE_FILE("blkio.io_service_bytes", - "8:0 Read 59542078464\n" - "8:0 Write 397369182208\n" - "8:0 Sync 234080922624\n" - "8:0 Async 222830338048\n" - "8:0 Total 456911260672\n"); - MAKE_FILE("blkio.io_serviced", - "8:0 Read 3402504\n" - "8:0 Write 14966516\n" - "8:0 Sync 12064031\n" - "8:0 Async 6304989\n" - "8:0 Total 18369020\n"); - MAKE_FILE("blkio.io_service_time", - "8:0 Read 10747537542349\n" - "8:0 Write 9200028590575\n" - "8:0 Sync 6449319855381\n" - "8:0 Async 13498246277543\n" - "8:0 Total 19947566132924\n"); - MAKE_FILE("blkio.io_wait_time", - "8:0 Read 14687514824889\n" - "8:0 Write 357748452187691\n" - "8:0 Sync 55296974349413\n" - "8:0 Async 317138992663167\n" - "8:0 Total 372435967012580\n"); - MAKE_FILE("blkio.reset_stats", ""); /* Write only */ - MAKE_FILE("blkio.sectors", "8:0 892404806\n"); MAKE_FILE("blkio.throttle.io_service_bytes", "8:0 Read 59542107136\n" "8:0 Write 411440480256\n" @@ -374,7 +302,6 @@ static int make_controller(const char *path, mode_t mode) MAKE_FILE("blkio.throttle.read_iops_device", ""); MAKE_FILE("blkio.throttle.write_bps_device", ""); MAKE_FILE("blkio.throttle.write_iops_device", ""); - MAKE_FILE("blkio.time", "8:0 61019089\n"); MAKE_FILE("blkio.weight", "1000\n"); MAKE_FILE("blkio.weight_device", ""); -- 2.17.1

Move all the cgroup data into separate files out of vircgroupmock.c and rework the fopen function to load data from files. This will make it easier to add more test cases. Reviewed-by: Fabiano Fidêncio <fidencio(a)redhat.com&gt; Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgroupdata/all-in-one.cgroups | 7 ++ tests/vircgroupdata/all-in-one.mounts | 2 +- tests/vircgroupdata/all-in-one.parsed | 12 +- tests/vircgroupdata/all-in-one.self.cgroup | 1 + tests/vircgroupdata/logind.cgroups | 10 ++ tests/vircgroupdata/logind.mounts | 2 + tests/vircgroupdata/logind.self.cgroup | 1 + tests/vircgroupdata/systemd.cgroups | 8 ++ tests/vircgroupdata/systemd.mounts | 11 ++ tests/vircgroupdata/systemd.self.cgroup | 6 + tests/vircgroupmock.c | 133 ++++----------------- tests/vircgrouptest.c | 10 +- 12 files changed, 79 insertions(+), 124 deletions(-) create mode 100644 tests/vircgroupdata/all-in-one.cgroups create mode 100644 tests/vircgroupdata/all-in-one.self.cgroup create mode 100644 tests/vircgroupdata/logind.cgroups create mode 100644 tests/vircgroupdata/logind.mounts create mode 100644 tests/vircgroupdata/logind.self.cgroup create mode 100644 tests/vircgroupdata/systemd.cgroups create mode 100644 tests/vircgroupdata/systemd.mounts create mode 100644 tests/vircgroupdata/systemd.self.cgroup diff --git a/tests/vircgroupdata/all-in-one.cgroups b/tests/vircgroupdata/all-in-one.cgroups new file mode 100644 index 0000000000..7208e5a0b6 --- /dev/null +++ b/tests/vircgroupdata/all-in-one.cgroups @@ -0,0 +1,7 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 6 1 1 +cpu 6 1 1 +cpuacct 6 1 1 +memory 6 1 1 +devices 6 1 1 +blkio 6 1 1 diff --git a/tests/vircgroupdata/all-in-one.mounts b/tests/vircgroupdata/all-in-one.mounts index 14093b961c..76c579ff69 100644 --- a/tests/vircgroupdata/all-in-one.mounts +++ b/tests/vircgroupdata/all-in-one.mounts @@ -4,4 +4,4 @@ proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0 udev /dev devtmpfs rw,relatime,size=16458560k,nr_inodes=4114640,mode=755 0 0 devpts /dev/pts devpts rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0 nfsd /proc/fs/nfsd nfsd rw,relatime 0 0 -cgroup /sys/fs/cgroup cgroup rw,relatime,blkio,devices,memory,cpuacct,cpu,cpuset 0 0 +cgroup /not/really/sys/fs/cgroup cgroup rw,relatime,blkio,devices,memory,cpuacct,cpu,cpuset 0 0 diff --git a/tests/vircgroupdata/all-in-one.parsed b/tests/vircgroupdata/all-in-one.parsed index 2701778fea..d703d08fb9 100644 --- a/tests/vircgroupdata/all-in-one.parsed +++ b/tests/vircgroupdata/all-in-one.parsed @@ -1,10 +1,10 @@ -cpu /sys/fs/cgroup -cpuacct /sys/fs/cgroup -cpuset /sys/fs/cgroup -memory /sys/fs/cgroup -devices /sys/fs/cgroup +cpu /not/really/sys/fs/cgroup +cpuacct /not/really/sys/fs/cgroup +cpuset /not/really/sys/fs/cgroup +memory /not/really/sys/fs/cgroup +devices /not/really/sys/fs/cgroup freezer <null> -blkio /sys/fs/cgroup +blkio /not/really/sys/fs/cgroup net_cls <null> perf_event <null> name=systemd <null> diff --git a/tests/vircgroupdata/all-in-one.self.cgroup b/tests/vircgroupdata/all-in-one.self.cgroup new file mode 100644 index 0000000000..cf237502e9 --- /dev/null +++ b/tests/vircgroupdata/all-in-one.self.cgroup @@ -0,0 +1 @@ +6:blkio,devices,memory,cpuacct,cpu,cpuset:/ diff --git a/tests/vircgroupdata/logind.cgroups b/tests/vircgroupdata/logind.cgroups new file mode 100644 index 0000000000..9d46f130e0 --- /dev/null +++ b/tests/vircgroupdata/logind.cgroups @@ -0,0 +1,10 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 0 1 1 +cpu 0 1 1 +cpuacct 0 1 1 +memory 0 1 0 +devices 0 1 1 +freezer 0 1 1 +net_cls 0 1 1 +blkio 0 1 1 +perf_event 0 1 1 diff --git a/tests/vircgroupdata/logind.mounts b/tests/vircgroupdata/logind.mounts new file mode 100644 index 0000000000..3ab908aee9 --- /dev/null +++ b/tests/vircgroupdata/logind.mounts @@ -0,0 +1,2 @@ +none /not/really/sys/fs/cgroup tmpfs rw,rootcontext=system_u:object_r:sysfs_t:s0,seclabel,relatime,size=4k,mode=755 0 0 +systemd /not/really/sys/fs/cgroup/systemd cgroup rw,nosuid,nodev,noexec,relatime,name=systemd 0 0 diff --git a/tests/vircgroupdata/logind.self.cgroup b/tests/vircgroupdata/logind.self.cgroup new file mode 100644 index 0000000000..31e0cfe8eb --- /dev/null +++ b/tests/vircgroupdata/logind.self.cgroup @@ -0,0 +1 @@ +0:name=systemd:/ diff --git a/tests/vircgroupdata/systemd.cgroups b/tests/vircgroupdata/systemd.cgroups new file mode 100644 index 0000000000..d32dfab222 --- /dev/null +++ b/tests/vircgroupdata/systemd.cgroups @@ -0,0 +1,8 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 2 4 1 +cpu 3 48 1 +cpuacct 3 48 1 +memory 4 4 1 +devices 5 4 1 +freezer 6 4 1 +blkio 8 4 1 diff --git a/tests/vircgroupdata/systemd.mounts b/tests/vircgroupdata/systemd.mounts new file mode 100644 index 0000000000..75572c86f7 --- /dev/null +++ b/tests/vircgroupdata/systemd.mounts @@ -0,0 +1,11 @@ +rootfs / rootfs rw 0 0 +tmpfs /run tmpfs rw,seclabel,nosuid,nodev,mode=755 0 0 +tmpfs /not/really/sys/fs/cgroup tmpfs rw,seclabel,nosuid,nodev,noexec,mode=755 0 0 +cgroup /not/really/sys/fs/cgroup/systemd cgroup rw,nosuid,nodev,noexec,relatime,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd 0 0 +cgroup /not/really/sys/fs/cgroup/cpuset cgroup rw,nosuid,nodev,noexec,relatime,cpuset 0 0 +cgroup /not/really/sys/fs/cgroup/cpu,cpuacct cgroup rw,nosuid,nodev,noexec,relatime,cpuacct,cpu 0 0 +cgroup /not/really/sys/fs/cgroup/freezer cgroup rw,nosuid,nodev,noexec,relatime,freezer 0 0 +cgroup /not/really/sys/fs/cgroup/blkio cgroup rw,nosuid,nodev,noexec,relatime,blkio 0 0 +cgroup /not/really/sys/fs/cgroup/memory cgroup rw,nosuid,nodev,noexec,relatime,memory 0 0 +/dev/sda1 /boot ext4 rw,seclabel,relatime,data=ordered 0 0 +tmpfs /tmp tmpfs rw,seclabel,relatime,size=1024000k 0 0 diff --git a/tests/vircgroupdata/systemd.self.cgroup b/tests/vircgroupdata/systemd.self.cgroup new file mode 100644 index 0000000000..2b95af79d2 --- /dev/null +++ b/tests/vircgroupdata/systemd.self.cgroup @@ -0,0 +1,6 @@ +115:memory:/ +8:blkio:/ +6:freezer:/ +3:cpuacct,cpu:/system +2:cpuset:/ +1:name=systemd:/user/berrange/123 diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 73cf876645..cfff1f0b7a 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -79,85 +79,6 @@ const char *fakedevicedir1 = FAKEDEVDIR1; * of files beneath it */ -/* - * Intentionally missing the 'devices' mount. - * Co-mounting cpu & cpuacct controllers - * An anonymous controller for systemd - */ -const char *procmounts = - "rootfs / rootfs rw 0 0\n" - "tmpfs /run tmpfs rw,seclabel,nosuid,nodev,mode=755 0 0\n" - "tmpfs /not/really/sys/fs/cgroup tmpfs rw,seclabel,nosuid,nodev,noexec,mode=755 0 0\n" - "cgroup /not/really/sys/fs/cgroup/systemd cgroup rw,nosuid,nodev,noexec,relatime,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd 0 0\n" - "cgroup /not/really/sys/fs/cgroup/cpuset cgroup rw,nosuid,nodev,noexec,relatime,cpuset 0 0\n" - "cgroup /not/really/sys/fs/cgroup/cpu,cpuacct cgroup rw,nosuid,nodev,noexec,relatime,cpuacct,cpu 0 0\n" - "cgroup /not/really/sys/fs/cgroup/freezer cgroup rw,nosuid,nodev,noexec,relatime,freezer 0 0\n" - "cgroup /not/really/sys/fs/cgroup/blkio cgroup rw,nosuid,nodev,noexec,relatime,blkio 0 0\n" - "cgroup /not/really/sys/fs/cgroup/memory cgroup rw,nosuid,nodev,noexec,relatime,memory 0 0\n" - "/dev/sda1 /boot ext4 rw,seclabel,relatime,data=ordered 0 0\n" - "tmpfs /tmp tmpfs rw,seclabel,relatime,size=1024000k 0 0\n"; - -const char *procselfcgroups = - "115:memory:/\n" - "8:blkio:/\n" - "6:freezer:/\n" - "3:cpuacct,cpu:/system\n" - "2:cpuset:/\n" - "1:name=systemd:/user/berrange/123\n"; - -const char *proccgroups = - "#subsys_name hierarchy num_cgroups enabled\n" - "cpuset 2 4 1\n" - "cpu 3 48 1\n" - "cpuacct 3 48 1\n" - "memory 4 4 1\n" - "devices 5 4 1\n" - "freezer 6 4 1\n" - "blkio 8 4 1\n"; - - -const char *procmountsallinone = - "rootfs / rootfs rw 0 0\n" - "sysfs /sys sysfs rw,nosuid,nodev,noexec,relatime 0 0\n" - "proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0\n" - "udev /dev devtmpfs rw,relatime,size=16458560k,nr_inodes=4114640,mode=755 0 0\n" - "devpts /dev/pts devpts rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0\n" - "nfsd /proc/fs/nfsd nfsd rw,relatime 0 0\n" - "cgroup /not/really/sys/fs/cgroup cgroup rw,relatime,blkio,devices,memory,cpuacct,cpu,cpuset 0 0\n"; - -const char *procselfcgroupsallinone = - "6:blkio,devices,memory,cpuacct,cpu,cpuset:/"; - -const char *proccgroupsallinone = - "#subsys_name hierarchy num_cgroups enabled\n" - "cpuset 6 1 1\n" - "cpu 6 1 1\n" - "cpuacct 6 1 1\n" - "memory 6 1 1\n" - "devices 6 1 1\n" - "blkio 6 1 1\n"; - -const char *procmountslogind = - "none /not/really/sys/fs/cgroup tmpfs rw,rootcontext=system_u:object_r:sysfs_t:s0,seclabel,relatime,size=4k,mode=755 0 0\n" - "systemd /not/really/sys/fs/cgroup/systemd cgroup rw,nosuid,nodev,noexec,relatime,name=systemd 0 0\n"; - -const char *procselfcgroupslogind = - "1:name=systemd:/\n"; - -const char *proccgroupslogind = - "#subsys_name hierarchy num_cgroups enabled\n" - "cpuset 0 1 1\n" - "cpu 0 1 1\n" - "cpuacct 0 1 1\n" - "memory 0 1 0\n" - "devices 0 1 1\n" - "freezer 0 1 1\n" - "net_cls 0 1 1\n" - "blkio 0 1 1\n" - "perf_event 0 1 1\n"; - - - static int make_file(const char *path, const char *name, const char *value) @@ -377,28 +298,16 @@ static void init_sysfs(void) FILE *fopen(const char *path, const char *mode) { - const char *mock; - bool allinone = false, logind = false; - init_syms(); + char *filepath = NULL; + const char *type = NULL; + FILE *rc = NULL; + const char *filename = getenv("VIR_CGROUP_MOCK_FILENAME"); - mock = getenv("VIR_CGROUP_MOCK_MODE"); - if (mock) { - if (STREQ(mock, "allinone")) - allinone = true; - else if (STREQ(mock, "logind")) - logind = true; - } + init_syms(); if (STREQ(path, "/proc/mounts")) { if (STREQ(mode, "r")) { - if (allinone) - return fmemopen((void *)procmountsallinone, - strlen(procmountsallinone), mode); - else if (logind) - return fmemopen((void *)procmountslogind, - strlen(procmountslogind), mode); - else - return fmemopen((void *)procmounts, strlen(procmounts), mode); + type = "mounts"; } else { errno = EACCES; return NULL; @@ -406,14 +315,7 @@ FILE *fopen(const char *path, const char *mode) } if (STREQ(path, "/proc/cgroups")) { if (STREQ(mode, "r")) { - if (allinone) - return fmemopen((void *)proccgroupsallinone, - strlen(proccgroupsallinone), mode); - else if (logind) - return fmemopen((void *)proccgroupslogind, - strlen(proccgroupslogind), mode); - else - return fmemopen((void *)proccgroups, strlen(proccgroups), mode); + type = "cgroups"; } else { errno = EACCES; return NULL; @@ -421,20 +323,25 @@ FILE *fopen(const char *path, const char *mode) } if (STREQ(path, "/proc/self/cgroup")) { if (STREQ(mode, "r")) { - if (allinone) - return fmemopen((void *)procselfcgroupsallinone, - strlen(procselfcgroupsallinone), mode); - else if (logind) - return fmemopen((void *)procselfcgroupslogind, - strlen(procselfcgroupslogind), mode); - else - return fmemopen((void *)procselfcgroups, strlen(procselfcgroups), mode); + type = "self.cgroup"; } else { errno = EACCES; return NULL; } } + if (type) { + if (!filename) + abort(); + if (virAsprintfQuiet(&filepath, "%s/vircgroupdata/%s.%s", + abs_srcdir, filename, type) < 0) { + abort(); + } + rc = real_fopen(filepath, mode); + free(filepath); + return rc; + } + return real_fopen(path, mode); } diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 6ab67dca78..d23ce2155b 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -889,6 +889,7 @@ mymain(void) DETECT_MOUNTS("no-cgroups"); DETECT_MOUNTS("kubevirt"); + setenv("VIR_CGROUP_MOCK_FILENAME", "systemd", 1); if (virTestRun("New cgroup for self", testCgroupNewForSelf, NULL) < 0) ret = -1; @@ -924,20 +925,21 @@ mymain(void) if (virTestRun("virCgroupGetPercpuStats works", testCgroupGetPercpuStats, NULL) < 0) ret = -1; + unsetenv("VIR_CGROUP_MOCK_FILENAME"); - setenv("VIR_CGROUP_MOCK_MODE", "allinone", 1); + setenv("VIR_CGROUP_MOCK_FILENAME", "all-in-one", 1); if (virTestRun("New cgroup for self (allinone)", testCgroupNewForSelfAllInOne, NULL) < 0) ret = -1; if (virTestRun("Cgroup available", testCgroupAvailable, (void*)0x1) < 0) ret = -1; - unsetenv("VIR_CGROUP_MOCK_MODE"); + unsetenv("VIR_CGROUP_MOCK_FILENAME"); - setenv("VIR_CGROUP_MOCK_MODE", "logind", 1); + setenv("VIR_CGROUP_MOCK_FILENAME", "logind", 1); if (virTestRun("New cgroup for self (logind)", testCgroupNewForSelfLogind, NULL) < 0) ret = -1; if (virTestRun("Cgroup available", testCgroupAvailable, (void*)0x0) < 0) ret = -1; - unsetenv("VIR_CGROUP_MOCK_MODE"); + unsetenv("VIR_CGROUP_MOCK_FILENAME"); if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); -- 2.17.1

Because we can set which files to return for cgroup tests there is no need to have special function tailored to run tests. Reviewed-by: Fabiano Fidêncio <fidencio(a)redhat.com&gt; Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/libvirt_private.syms | 2 +- src/util/vircgroup.c | 21 +++++---------------- src/util/vircgrouppriv.h | 4 +--- tests/vircgrouptest.c | 16 ++++++++-------- 4 files changed, 15 insertions(+), 28 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ad7ce57b65..7f3b5738c4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1513,7 +1513,7 @@ virCgroupDelThread; virCgroupDenyAllDevices; virCgroupDenyDevice; virCgroupDenyDevicePath; -virCgroupDetectMountsFromFile; +virCgroupDetectMounts; virCgroupFree; virCgroupGetBlkioDeviceReadBps; virCgroupGetBlkioDeviceReadIops; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index ea83c5f5b2..cc4c45b4fb 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -426,9 +426,7 @@ virCgroupMountOptsMatchController(const char *mntOpts, * mounted and where */ int -virCgroupDetectMountsFromFile(virCgroupPtr group, - const char *path, - bool checkLinks) +virCgroupDetectMounts(virCgroupPtr group) { size_t i; FILE *mounts = NULL; @@ -436,9 +434,9 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, char buf[CGROUP_MAX_VAL]; int ret = -1; - mounts = fopen(path, "r"); + mounts = fopen("/proc/mounts", "r"); if (mounts == NULL) { - virReportSystemError(errno, _("Unable to open %s"), path); + virReportSystemError(errno, "%s", _("Unable to open /proc/mounts")); return -1; } @@ -466,8 +464,7 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, /* If it is a co-mount it has a filename like "cpu,cpuacct" * and we must identify the symlink path */ - if (checkLinks && - virCgroupResolveMountLink(entry.mnt_dir, typestr, + if (virCgroupResolveMountLink(entry.mnt_dir, typestr, controller) < 0) { goto cleanup; } @@ -481,12 +478,6 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, return ret; } -static int -virCgroupDetectMounts(virCgroupPtr group) -{ - return virCgroupDetectMountsFromFile(group, "/proc/mounts", true); -} - static int virCgroupCopyPlacement(virCgroupPtr group, @@ -4086,9 +4077,7 @@ virCgroupAvailable(void) int -virCgroupDetectMountsFromFile(virCgroupPtr group ATTRIBUTE_UNUSED, - const char *path ATTRIBUTE_UNUSED, - bool checkLinks ATTRIBUTE_UNUSED) +virCgroupDetectMounts(virCgroupPtr group ATTRIBUTE_UNUSED) { virReportSystemError(ENXIO, "%s", _("Control groups not supported on this platform")); diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index a0034f3889..f78fe8bb9c 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -50,9 +50,7 @@ struct _virCgroup { virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST]; }; -int virCgroupDetectMountsFromFile(virCgroupPtr group, - const char *path, - bool checkLinks); +int virCgroupDetectMounts(virCgroupPtr group); int virCgroupNewPartition(const char *path, bool create, diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index d23ce2155b..54945eea2d 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -163,21 +163,21 @@ testCgroupDetectMounts(const void *args) { int result = -1; const char *file = args; - char *mounts = NULL; char *parsed = NULL; const char *actual; virCgroupPtr group = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i; - if (virAsprintf(&mounts, "%s/vircgroupdata/%s.mounts", - abs_srcdir, file) < 0 || - virAsprintf(&parsed, "%s/vircgroupdata/%s.parsed", - abs_srcdir, file) < 0 || - VIR_ALLOC(group) < 0) + setenv("VIR_CGROUP_MOCK_FILENAME", file, 1); + + if (virAsprintf(&parsed, "%s/vircgroupdata/%s.parsed", abs_srcdir, file) < 0) + goto cleanup; + + if (VIR_ALLOC(group) < 0) goto cleanup; - if (virCgroupDetectMountsFromFile(group, mounts, false) < 0) + if (virCgroupDetectMounts(group) < 0) goto cleanup; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { @@ -195,7 +195,7 @@ testCgroupDetectMounts(const void *args) result = 0; cleanup: - VIR_FREE(mounts); + unsetenv("VIR_CGROUP_MOCK_FILENAME"); VIR_FREE(parsed); virCgroupFree(&group); virBufferFreeAndReset(&buf); -- 2.17.1

This will be required once cgroup v2 is introduced. The cgroup detection is not simple and we will have multiple backends so we should not just jump into the middle of the detection code. In order to use virCgroupNewSelf we need to create all the remaining data files: - {name}.cgroups represents /proc/cgroups, it is a list of cgroup controllers compiled into kernel - {name}.self.cgroup represents /proc/self/cgroup, it describes cgroups to which the process belongs For "no-cgroups" we need to modify the expected behavior because virCgroupNewSelf() will fail if there are no controllers available. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: changes in v2: - no-cgroups detect test is now expected to fail src/libvirt_private.syms | 1 - src/util/vircgroup.c | 11 +------ src/util/vircgrouppriv.h | 2 -- tests/vircgroupdata/cgroups1.cgroups | 11 +++++++ tests/vircgroupdata/cgroups1.self.cgroup | 11 +++++++ tests/vircgroupdata/cgroups2.cgroups | 10 +++++++ tests/vircgroupdata/cgroups2.self.cgroup | 10 +++++++ tests/vircgroupdata/cgroups3.cgroups | 12 ++++++++ tests/vircgroupdata/cgroups3.self.cgroup | 12 ++++++++ tests/vircgroupdata/fedora-18.cgroups | 10 +++++++ tests/vircgroupdata/fedora-18.self.cgroup | 9 ++++++ tests/vircgroupdata/fedora-21.cgroups | 12 ++++++++ tests/vircgroupdata/fedora-21.self.cgroup | 10 +++++++ tests/vircgroupdata/kubevirt.cgroups | 10 +++++++ tests/vircgroupdata/kubevirt.self.cgroup | 10 +++++++ tests/vircgroupdata/no-cgroups.cgroups | 8 +++++ tests/vircgroupdata/no-cgroups.parsed | 10 ------- tests/vircgroupdata/no-cgroups.self.cgroup | 0 tests/vircgroupdata/ovirt-node-6.6.cgroups | 9 ++++++ .../vircgroupdata/ovirt-node-6.6.self.cgroup | 8 +++++ tests/vircgroupdata/ovirt-node-7.1.cgroups | 11 +++++++ .../vircgroupdata/ovirt-node-7.1.self.cgroup | 10 +++++++ tests/vircgroupdata/rhel-7.1.cgroups | 11 +++++++ tests/vircgroupdata/rhel-7.1.self.cgroup | 10 +++++++ tests/vircgrouptest.c | 30 ++++++++++++++----- 25 files changed, 217 insertions(+), 31 deletions(-) create mode 100644 tests/vircgroupdata/cgroups1.cgroups create mode 100644 tests/vircgroupdata/cgroups1.self.cgroup create mode 100644 tests/vircgroupdata/cgroups2.cgroups create mode 100644 tests/vircgroupdata/cgroups2.self.cgroup create mode 100644 tests/vircgroupdata/cgroups3.cgroups create mode 100644 tests/vircgroupdata/cgroups3.self.cgroup create mode 100644 tests/vircgroupdata/fedora-18.cgroups create mode 100644 tests/vircgroupdata/fedora-18.self.cgroup create mode 100644 tests/vircgroupdata/fedora-21.cgroups create mode 100644 tests/vircgroupdata/fedora-21.self.cgroup create mode 100644 tests/vircgroupdata/kubevirt.cgroups create mode 100644 tests/vircgroupdata/kubevirt.self.cgroup create mode 100644 tests/vircgroupdata/no-cgroups.cgroups delete mode 100644 tests/vircgroupdata/no-cgroups.parsed create mode 100644 tests/vircgroupdata/no-cgroups.self.cgroup create mode 100644 tests/vircgroupdata/ovirt-node-6.6.cgroups create mode 100644 tests/vircgroupdata/ovirt-node-6.6.self.cgroup create mode 100644 tests/vircgroupdata/ovirt-node-7.1.cgroups create mode 100644 tests/vircgroupdata/ovirt-node-7.1.self.cgroup create mode 100644 tests/vircgroupdata/rhel-7.1.cgroups create mode 100644 tests/vircgroupdata/rhel-7.1.self.cgroup diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7f3b5738c4..75c59fbf89 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1513,7 +1513,6 @@ virCgroupDelThread; virCgroupDenyAllDevices; virCgroupDenyDevice; virCgroupDenyDevicePath; -virCgroupDetectMounts; virCgroupFree; virCgroupGetBlkioDeviceReadBps; virCgroupGetBlkioDeviceReadIops; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index cc4c45b4fb..38458de0a2 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -425,7 +425,7 @@ virCgroupMountOptsMatchController(const char *mntOpts, * Process /proc/mounts figuring out what controllers are * mounted and where */ -int +static int virCgroupDetectMounts(virCgroupPtr group) { size_t i; @@ -4076,15 +4076,6 @@ virCgroupAvailable(void) } -int -virCgroupDetectMounts(virCgroupPtr group ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENXIO, "%s", - _("Control groups not supported on this platform")); - return -1; -} - - int virCgroupNewPartition(const char *path ATTRIBUTE_UNUSED, bool create ATTRIBUTE_UNUSED, diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index f78fe8bb9c..046c96c52c 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -50,8 +50,6 @@ struct _virCgroup { virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST]; }; -int virCgroupDetectMounts(virCgroupPtr group); - int virCgroupNewPartition(const char *path, bool create, int controllers, diff --git a/tests/vircgroupdata/cgroups1.cgroups b/tests/vircgroupdata/cgroups1.cgroups new file mode 100644 index 0000000000..a03c290a98 --- /dev/null +++ b/tests/vircgroupdata/cgroups1.cgroups @@ -0,0 +1,11 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 1 1 1 +cpu 2 1 1 +cpuacct 3 1 1 +blkio 8 1 1 +memory 4 1 1 +devices 5 1 1 +freezer 6 1 1 +net_cls 7 1 1 +net_prio 9 1 1 +hugetlb 10 1 1 diff --git a/tests/vircgroupdata/cgroups1.self.cgroup b/tests/vircgroupdata/cgroups1.self.cgroup new file mode 100644 index 0000000000..181f0c22f8 --- /dev/null +++ b/tests/vircgroupdata/cgroups1.self.cgroup @@ -0,0 +1,11 @@ +10:hugetlb:/ +9:net_prio:/ +8:blkio:/ +7:net_cls:/ +6:freezer:/ +5:devices:/ +4:memory:/ +3:cpuacct:/ +2:cpu:/ +1:cpuset:/ +0:name=openrc:/ diff --git a/tests/vircgroupdata/cgroups2.cgroups b/tests/vircgroupdata/cgroups2.cgroups new file mode 100644 index 0000000000..f0a7699559 --- /dev/null +++ b/tests/vircgroupdata/cgroups2.cgroups @@ -0,0 +1,10 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 1 1 1 +cpu 2 1 1 +cpuacct 3 1 1 +blkio 7 1 1 +memory 4 1 1 +devices 5 1 1 +freezer 6 1 1 +perf_event 8 1 1 +hugetlb 9 1 1 diff --git a/tests/vircgroupdata/cgroups2.self.cgroup b/tests/vircgroupdata/cgroups2.self.cgroup new file mode 100644 index 0000000000..3d0e793e5a --- /dev/null +++ b/tests/vircgroupdata/cgroups2.self.cgroup @@ -0,0 +1,10 @@ +9:hugetlb:/ +8:perf_event:/ +7:blkio:/ +6:freezer:/ +5:devices:/ +4:memory:/ +3:cpuacct:/ +2:cpu:/ +1:cpuset:/ +0:name=openrc:/ diff --git a/tests/vircgroupdata/cgroups3.cgroups b/tests/vircgroupdata/cgroups3.cgroups new file mode 100644 index 0000000000..294d95dedf --- /dev/null +++ b/tests/vircgroupdata/cgroups3.cgroups @@ -0,0 +1,12 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 1 1 1 +cpu 2 1 1 +cpuacct 3 1 1 +blkio 8 1 1 +memory 4 1 1 +devices 5 1 1 +freezer 6 1 1 +net_cls 7 1 1 +perf_event 9 1 1 +net_prio 10 1 1 +hugetlb 11 1 1 diff --git a/tests/vircgroupdata/cgroups3.self.cgroup b/tests/vircgroupdata/cgroups3.self.cgroup new file mode 100644 index 0000000000..bf346abdf9 --- /dev/null +++ b/tests/vircgroupdata/cgroups3.self.cgroup @@ -0,0 +1,12 @@ +11:hugetlb:/ +10:net_prio:/ +9:perf_event:/ +8:blkio:/ +7:net_cls:/ +6:freezer:/ +5:devices:/ +4:memory:/ +3:cpuacct:/ +2:cpu:/ +1:cpuset:/ +0:name=openrc:/ diff --git a/tests/vircgroupdata/fedora-18.cgroups b/tests/vircgroupdata/fedora-18.cgroups new file mode 100644 index 0000000000..8eb41087f3 --- /dev/null +++ b/tests/vircgroupdata/fedora-18.cgroups @@ -0,0 +1,10 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 1 1 1 +cpu 2 1 1 +cpuacct 2 1 1 +blkio 7 1 1 +memory 3 1 1 +devices 4 1 1 +freezer 5 1 1 +net_cls 6 1 1 +perf_event 8 1 1 diff --git a/tests/vircgroupdata/fedora-18.self.cgroup b/tests/vircgroupdata/fedora-18.self.cgroup new file mode 100644 index 0000000000..da9ad8ad4d --- /dev/null +++ b/tests/vircgroupdata/fedora-18.self.cgroup @@ -0,0 +1,9 @@ +8:perf_event:/ +7:blkio:/ +6:net_cls:/ +5:freezer:/ +4:devices:/ +3:memory:/ +2:cpu,cpuacct:/ +1:cpuset:/ +0:name=systemd:/ diff --git a/tests/vircgroupdata/fedora-21.cgroups b/tests/vircgroupdata/fedora-21.cgroups new file mode 100644 index 0000000000..3e1401ee98 --- /dev/null +++ b/tests/vircgroupdata/fedora-21.cgroups @@ -0,0 +1,12 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 1 1 1 +cpu 2 1 1 +cpuacct 2 1 1 +blkio 7 1 1 +memory 3 1 1 +devices 4 1 1 +freezer 5 1 1 +net_cls 6 1 1 +perf_event 8 1 1 +net_prio 6 1 1 +hugetlb 9 1 1 diff --git a/tests/vircgroupdata/fedora-21.self.cgroup b/tests/vircgroupdata/fedora-21.self.cgroup new file mode 100644 index 0000000000..4c666bd59e --- /dev/null +++ b/tests/vircgroupdata/fedora-21.self.cgroup @@ -0,0 +1,10 @@ +9:hugetlb:/ +8:perf_event:/ +7:blkio:/ +6:net_cls,net_prio:/ +5:freezer:/ +4:devices:/ +3:memory:/ +2:cpu,cpuacct:/ +1:cpuset:/ +0:name=systemd:/ diff --git a/tests/vircgroupdata/kubevirt.cgroups b/tests/vircgroupdata/kubevirt.cgroups new file mode 100644 index 0000000000..f0a7699559 --- /dev/null +++ b/tests/vircgroupdata/kubevirt.cgroups @@ -0,0 +1,10 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 1 1 1 +cpu 2 1 1 +cpuacct 3 1 1 +blkio 7 1 1 +memory 4 1 1 +devices 5 1 1 +freezer 6 1 1 +perf_event 8 1 1 +hugetlb 9 1 1 diff --git a/tests/vircgroupdata/kubevirt.self.cgroup b/tests/vircgroupdata/kubevirt.self.cgroup new file mode 100644 index 0000000000..3d0e793e5a --- /dev/null +++ b/tests/vircgroupdata/kubevirt.self.cgroup @@ -0,0 +1,10 @@ +9:hugetlb:/ +8:perf_event:/ +7:blkio:/ +6:freezer:/ +5:devices:/ +4:memory:/ +3:cpuacct:/ +2:cpu:/ +1:cpuset:/ +0:name=openrc:/ diff --git a/tests/vircgroupdata/no-cgroups.cgroups b/tests/vircgroupdata/no-cgroups.cgroups new file mode 100644 index 0000000000..3ed1d4e45e --- /dev/null +++ b/tests/vircgroupdata/no-cgroups.cgroups @@ -0,0 +1,8 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 0 1 1 +cpu 0 1 1 +cpuacct 0 1 1 +memory 0 1 1 +devices 0 1 1 +freezer 0 1 1 +blkio 0 1 1 diff --git a/tests/vircgroupdata/no-cgroups.parsed b/tests/vircgroupdata/no-cgroups.parsed deleted file mode 100644 index bf4eea085f..0000000000 --- a/tests/vircgroupdata/no-cgroups.parsed +++ /dev/null @@ -1,10 +0,0 @@ -cpu <null> -cpuacct <null> -cpuset <null> -memory <null> -devices <null> -freezer <null> -blkio <null> -net_cls <null> -perf_event <null> -name=systemd <null> diff --git a/tests/vircgroupdata/no-cgroups.self.cgroup b/tests/vircgroupdata/no-cgroups.self.cgroup new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/vircgroupdata/ovirt-node-6.6.cgroups b/tests/vircgroupdata/ovirt-node-6.6.cgroups new file mode 100644 index 0000000000..aaabf11a44 --- /dev/null +++ b/tests/vircgroupdata/ovirt-node-6.6.cgroups @@ -0,0 +1,9 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 0 1 1 +cpu 1 1 1 +cpuacct 2 1 1 +blkio 7 1 1 +memory 3 1 1 +devices 4 1 1 +freezer 5 1 1 +net_cls 6 1 1 diff --git a/tests/vircgroupdata/ovirt-node-6.6.self.cgroup b/tests/vircgroupdata/ovirt-node-6.6.self.cgroup new file mode 100644 index 0000000000..dadc8155fa --- /dev/null +++ b/tests/vircgroupdata/ovirt-node-6.6.self.cgroup @@ -0,0 +1,8 @@ +7:blkio:/ +6:net_cls:/ +5:freezer:/ +4:devices:/ +3:memory:/ +2:cpuacct:/ +1:cpu:/ +0:cpuset:/ diff --git a/tests/vircgroupdata/ovirt-node-7.1.cgroups b/tests/vircgroupdata/ovirt-node-7.1.cgroups new file mode 100644 index 0000000000..687297ad4a --- /dev/null +++ b/tests/vircgroupdata/ovirt-node-7.1.cgroups @@ -0,0 +1,11 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 1 1 1 +cpu 2 1 1 +cpuacct 2 1 1 +blkio 7 1 1 +memory 3 1 1 +devices 4 1 1 +freezer 5 1 1 +net_cls 6 1 1 +perf_event 8 1 1 +hugetlb 9 1 1 diff --git a/tests/vircgroupdata/ovirt-node-7.1.self.cgroup b/tests/vircgroupdata/ovirt-node-7.1.self.cgroup new file mode 100644 index 0000000000..f07e8e20f5 --- /dev/null +++ b/tests/vircgroupdata/ovirt-node-7.1.self.cgroup @@ -0,0 +1,10 @@ +9:hugetlb:/ +8:perf_event:/ +7:blkio:/ +6:net_cls:/ +5:freezer:/ +4:devices:/ +3:memory:/ +2:cpu,cpuacct:/ +1:cpuset:/ +0:name=systemd:/ diff --git a/tests/vircgroupdata/rhel-7.1.cgroups b/tests/vircgroupdata/rhel-7.1.cgroups new file mode 100644 index 0000000000..687297ad4a --- /dev/null +++ b/tests/vircgroupdata/rhel-7.1.cgroups @@ -0,0 +1,11 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 1 1 1 +cpu 2 1 1 +cpuacct 2 1 1 +blkio 7 1 1 +memory 3 1 1 +devices 4 1 1 +freezer 5 1 1 +net_cls 6 1 1 +perf_event 8 1 1 +hugetlb 9 1 1 diff --git a/tests/vircgroupdata/rhel-7.1.self.cgroup b/tests/vircgroupdata/rhel-7.1.self.cgroup new file mode 100644 index 0000000000..f07e8e20f5 --- /dev/null +++ b/tests/vircgroupdata/rhel-7.1.self.cgroup @@ -0,0 +1,10 @@ +9:hugetlb:/ +8:perf_event:/ +7:blkio:/ +6:net_cls:/ +5:freezer:/ +4:devices:/ +3:memory:/ +2:cpu,cpuacct:/ +1:cpuset:/ +0:name=systemd:/ diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 54945eea2d..b6564bdd45 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -158,26 +158,37 @@ const char *linksLogind[VIR_CGROUP_CONTROLLER_LAST] = { }; +struct _detectMountsData { + const char *file; + bool fail; +}; + + static int testCgroupDetectMounts(const void *args) { int result = -1; - const char *file = args; + const struct _detectMountsData *data = args; char *parsed = NULL; const char *actual; virCgroupPtr group = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i; - setenv("VIR_CGROUP_MOCK_FILENAME", file, 1); + setenv("VIR_CGROUP_MOCK_FILENAME", data->file, 1); - if (virAsprintf(&parsed, "%s/vircgroupdata/%s.parsed", abs_srcdir, file) < 0) + if (virAsprintf(&parsed, "%s/vircgroupdata/%s.parsed", + abs_srcdir, data->file) < 0) { goto cleanup; + } - if (VIR_ALLOC(group) < 0) + if (virCgroupNewSelf(&group) < 0) { + if (data->fail) + result = 0; goto cleanup; + } - if (virCgroupDetectMounts(group) < 0) + if (data->fail) goto cleanup; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { @@ -869,13 +880,16 @@ mymain(void) setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1); -# define DETECT_MOUNTS(file) \ +# define DETECT_MOUNTS_FULL(file, fail) \ do { \ + struct _detectMountsData data = { file, fail }; \ if (virTestRun("Detect cgroup mounts for " file, \ testCgroupDetectMounts, \ - file) < 0) \ + &data) < 0) \ ret = -1; \ } while (0) +# define DETECT_MOUNTS(file) DETECT_MOUNTS_FULL(file, false); +# define DETECT_MOUNTS_FAIL(file) DETECT_MOUNTS_FULL(file, true); DETECT_MOUNTS("ovirt-node-6.6"); DETECT_MOUNTS("ovirt-node-7.1"); @@ -886,7 +900,7 @@ mymain(void) DETECT_MOUNTS("cgroups2"); DETECT_MOUNTS("cgroups3"); DETECT_MOUNTS("all-in-one"); - DETECT_MOUNTS("no-cgroups"); + DETECT_MOUNTS_FAIL("no-cgroups"); DETECT_MOUNTS("kubevirt"); setenv("VIR_CGROUP_MOCK_FILENAME", "systemd", 1); -- 2.17.1