On 29/07/2024 18:30, Alexis Lothoré wrote: > Hello Alan, > > On 7/29/24 18:59, Alan Maguire wrote: >> On 29/07/2024 09:20, Alexis Lothoré (eBPF Foundation) wrote: >>> test_dev_cgroup currently loads a small bpf program allowing any access on >>> urandom and zero devices, disabling access to any other device. It makes >>> migrating this test to test_progs impossible, since this one manipulates >>> extensively /dev/null. >>> >>> Allow /dev/null manipulation in dev_cgroup program to make its usage in >>> test_progs framework possible. Update test_dev_cgroup.c as well to match >>> this change while it has not been removed. >>> >>> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@xxxxxxxxxxx> >>> --- >>> tools/testing/selftests/bpf/progs/dev_cgroup.c | 4 ++-- >>> tools/testing/selftests/bpf/test_dev_cgroup.c | 18 +++++++++--------- >> >> Not a big deal, but I found it a bit confusing that this file was >> modified then deleted in patch 2. Would it work having patch 1 stop >> building the standalone test/remove it and .gitignore entry, patch 2 >> updating progs/dev_cgroup.c to allow /dev/zero, /dev/urandom access, >> patch 3 add cgroup_dev.c test support, and patch 4 add the device type >> subtest? Or are there issues with doing things that way? Thanks! > > I've done this to make sure that at any point in the git history, there is one > working test for the targeted feature, either the old or the new one. I've done > it this way because the old test also helped me validate the new one while > developing it, but also because if at some point there is a (major) issue with > the new test, reverting only the relevant commit brings back the old test while > disabling the new one. > > But maybe this concern is not worth the trouble (especially since the old tests > are not run automatically) ? If that's indeed the case, I can do it the way you > are suggesting :) > If no-one complains, it seems fine to me to stick with the way you've constructed the series the next respin. Thanks! > Thanks, > > Alexis >
