diff mbox series

[06/16] API/cgroup: Change to TWARN when v2 controllers change

Message ID 2a49929d166ac88d4c7900c5961be77cbd601fa4.1642601554.git.luke.nowakowskikrijger@canonical.com
State Superseded
Headers show
Series Expand Cgroup lib and modify controller tests | expand

Commit Message

Luke Nowakowski-Krijger Jan. 19, 2022, 2:44 p.m. UTC
When the v1 blkio controller is mounted it unmounts the v2 io controller
effectively bringing the amount of cgroupv2 controllers down and
triggering the tst_brk. Because this is exected behaivor it should be
changed to TWARN in case there is something funny still going on and
should be logged.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 lib/tst_cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Richard Palethorpe Jan. 24, 2022, 10:44 a.m. UTC | #1
Hello Luke,

Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:

> When the v1 blkio controller is mounted it unmounts the v2 io controller
> effectively bringing the amount of cgroupv2 controllers down and
> triggering the tst_brk. Because this is exected behaivor it should be
> changed to TWARN in case there is something funny still going on and
> should be logged.

Ah, we are in new territory here, the library assumes this won't happen.

What happens if a test author requires the V2 io controller then
requires the blkio controller?

At best I think we will get TBROK saying some file doesn't exist. Not
too bad, but nothing helpful either.

Also what happens if the io controller was in use, disappears then
returns? (that is, if it returns, because unmounting V1 controllers
doesn't guarantee the V1 root is destroyed and it can block V2 from
being used afterwards).

The basic assumption we originally made was that we can't mess with the
current CGroup setup beyond adding to it. LTP doesn't guarantee it won't
mess up your system, but it at least tries not to. Pulling controllers
out from underneath the system manager seems likely to cause wierd
errors for people.

Why not treat the blkio controller and io controller as the same thing?
Or if they are not remotely compatible, then do tst_brk(TCONF... when we
need blkio, but io is on V2.

BTW, this is a moot point, but TWARN is really only used when we think
an error was just caused by previous errors. Otherwise, if something is
expected, then it should be TINFO.

>
> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> ---
>  lib/tst_cgroup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 3d56a3364..c53b88ed2 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -677,7 +677,7 @@ static void cgroup_root_scan(const char *const mnt_type,
>  		goto discard;
>  
>  	if (root->ctrl_field)
> -		tst_brk(TBROK, "Available V2 controllers are changing between scans?");
> +		tst_res(TWARN, "Available V2 controllers are changing between scans?");
>  
>  	root->ver = TST_CGROUP_V2;
diff mbox series

Patch

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 3d56a3364..c53b88ed2 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -677,7 +677,7 @@  static void cgroup_root_scan(const char *const mnt_type,
 		goto discard;
 
 	if (root->ctrl_field)
-		tst_brk(TBROK, "Available V2 controllers are changing between scans?");
+		tst_res(TWARN, "Available V2 controllers are changing between scans?");
 
 	root->ver = TST_CGROUP_V2;