On 12/3/25 01:47, Aleksei Oladko wrote:
Add selftests for the printk virtualization feature. The new tests cover
ve_printk, ve_printk_ratelimited, net_ve_ratelimited, and log buffer
overflow handling.


[my previous comment]: i would also ask you to put more comments both to the commit message and to the test code.

Please enhance the commit message as well.
https://virtuozzo.atlassian.net/browse/VSTOR-114252

v2:
   - added checks for the return values of system() and write()
   - added comments to clarify the code

you have also dropped a hunk:

     7 │fd = open("/proc/self/gid_map", O_WRONLY);
     8 │if (fd < 0)
     9 │    return -1;
    10 │write(fd, "0 0 1\n", 6);

1. Why did you drop it? Why it's not needed?
2. Please always mention this in the changelog vX commit.


Signed-off-by: Aleksei Oladko <[email protected]>
---
...

+int ve_printk_test_ratelimit(void)
+{
+       FILE *pdmesg;
+       char buf[1024];
+        int ret;
+       int supressed = 1;

"supPressed" here and 5 times more below.

+       int i;
+
+       /*
+        * Attempting to add this rule results in an error message being written
+        * to the container's dmesg. The error is logged using a ratelimited 
printk:
+        * reject_tg_check -> ve_printk_ratelimited(VE_LOG, ...
+        */
+       for (i = 0; i < 2 * TEST_RATELIMIT_BURST; i++) {
+               /* The rule is addd inside the container */

addEd

+               ret = system("iptables -A INPUT -s 10.20.30.40 -j REJECT --reject-with 
tcp-reset &> /dev/null");
+               if (ret < 0)
+                       supressed++;
+       }
...



+int run_vzct(char *cgv2, char *cgve, int ctid, int testid)
+{
+       char *stack;
+       char *stack_top;
+       pid_t pid;
+       int flags = CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC |
+                   CLONE_NEWPID | CLONE_NEWUSER | CLONE_NEWNET |
+                   CLONE_NEWCGROUP | SIGCHLD;

You did not add here new flag CLONE_NEWVE.
Does it work on a new kernel (2.23.vz10+) without this flag added?
If it does work - that's strange, can you please understand why does it work and quite probably it's a bug.

If it does not work and you have just not tried on the latest kernel yet - please, add the flag CLONE_NEWVE.

+       struct fargs args = {
+               .cgve = cgve,
+               .ctid = ctid,
+               .test = testid,
+       };
+       int status;
+       int ret;
+
+       stack = malloc(__STACK_SIZE);
+       if (!stack)
+               return -1;
+
+       ret = enter_cgroup(cgv2, ctid);
+       if (ret < 0)
+               return -1;
+       ret = enter_cgroup(cgve, ctid);
+       if (ret < 0)
+               return -1;

OK, i've just got the idea that the test relies on the fact that we are running VE cgroup as v1 (legacy) and all other cgroups as v2. And will fail when we switch VE cgroup to v2 as well.

This is an essential thing for this test.
Please, add a notice about that in the commit message.


+       stack_top = stack + __STACK_SIZE;
+       pid = clone(child_func, stack_top, flags, &args);
+       if (pid < 0) {
+               return -1;
+       }
+
+       ret = waitpid(pid, &status, 0);
+       if (ret < 0)
+               return -1;
+       ret = -1;
+       if (WIFEXITED(status)) {
+               ret = WEXITSTATUS(status);
+       }
+       enter_cgroup(cgv2, 0);
+       enter_cgroup(cgve, 0);
+
+       free(stack);
+
+       return ret;
+}
+
...
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to