| Message ID | 39c482de2c75e393fa53c31d06620995dabc5284.1641376050.git.luke.nowakowskikrijger@canonical.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Expand Cgroup shell library | expand |
Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote: > +static int parse_root_config(char *config_entry) > +{ > + char *key; > + char *value; > + struct cgroup_root *root; > + > + for (key = strtok(config_entry, " "); key; key = strtok(NULL, " ")) { > + if (!(value = strchr(key, '='))) { > + if (!(root = cgroup_find_root(key))) > + tst_brk(TBROK, "Could not find root from config. Roots changing between calls?"); > + > + continue; > + } > + > + *value = '\0'; > + value = value + 1; > + > + if (!strcmp(key, CONFIG_MOUNTROOT_KEY) && !strcmp(value, "yes")) { > + root->we_mounted_it = 1; > + > + } else if (!strcmp(key, CONFIG_LTPDIR_KEY) && !root->ltp_dir.dir_name) { > + cgroup_dir_mk(&root->mnt_dir, cgroup_ltp_dir, &root->ltp_dir); > + if (!strcmp(value, "yes")) > + root->ltp_dir.we_created_it = 1; > + > + } else if (!strcmp(key, CONFIG_DRAINDIR_KEY) && !root->drain_dir.dir_name) { > + cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir); > + if (!strcmp(value, "yes")) > + root->ltp_dir.we_created_it = 1; I think that parsing the CONFIG_DRAINDIR_KEY from '$_cgroup_state' is superfluous. Because from the tst_cgroup_cleanup, if root->ltp_dir.we_created_it is 1 then both of the two directories will be removed, so just using CONFIG_LTPDIR_KE to track the status is enough. And maybe it is not necessary to print "Created_Drain_Dir=xx" in tst_cgroup_print_config at all. Then, the code snippet could be as: } else if (!strcmp(key, CONFIG_LTPDIR_KEY) && !root->ltp_dir.dir_name) { cgroup_dir_mk(&root->mnt_dir, cgroup_ltp_dir, &root->ltp_dir); cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir); if (!strcmp(value, "yes")) root->ltp_dir.we_created_it = 1; > + > + } else if (!strcmp(key, CONFIG_TESTID_KEY) && strcmp(value, "NULL") && > + !root->test_dir.dir_name) { > + cgroup_dir_mk(&root->ltp_dir, value, &root->test_dir); > + root->test_dir.we_created_it = 1; > + } > + } > + > + return 0; > +} -- Regards, Li Wang
Hi Li, On Thu, Jan 13, 2022 at 10:57 PM Li Wang <liwang@redhat.com> wrote: > Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote: > > > +static int parse_root_config(char *config_entry) > > +{ > > + char *key; > > + char *value; > > + struct cgroup_root *root; > > + > > + for (key = strtok(config_entry, " "); key; key = strtok(NULL, " > ")) { > > + if (!(value = strchr(key, '='))) { > > + if (!(root = cgroup_find_root(key))) > > + tst_brk(TBROK, "Could not find root from > config. Roots changing between calls?"); > > + > > + continue; > > + } > > + > > + *value = '\0'; > > + value = value + 1; > > + > > + if (!strcmp(key, CONFIG_MOUNTROOT_KEY) && !strcmp(value, > "yes")) { > > + root->we_mounted_it = 1; > > + > > > + } else if (!strcmp(key, CONFIG_LTPDIR_KEY) && > !root->ltp_dir.dir_name) { > > + cgroup_dir_mk(&root->mnt_dir, cgroup_ltp_dir, > &root->ltp_dir); > > + if (!strcmp(value, "yes")) > > + root->ltp_dir.we_created_it = 1; > > + > > + } else if (!strcmp(key, CONFIG_DRAINDIR_KEY) && > !root->drain_dir.dir_name) { > > + cgroup_dir_mk(&root->ltp_dir, > cgroup_ltp_drain_dir, &root->drain_dir); > > + if (!strcmp(value, "yes")) > > + root->ltp_dir.we_created_it = 1; > > I think that parsing the CONFIG_DRAINDIR_KEY from '$_cgroup_state' > is superfluous. Because from the tst_cgroup_cleanup, if > root->ltp_dir.we_created_it > is 1 then both of the two directories will be removed, so just using > CONFIG_LTPDIR_KE > to track the status is enough. > > This is definitely true and I think it makes it a lot simpler. I will add this in the next version. And maybe it is not necessary to print "Created_Drain_Dir=xx" in > tst_cgroup_print_config at all. > > Then, the code snippet could be as: > > } else if (!strcmp(key, CONFIG_LTPDIR_KEY) && > !root->ltp_dir.dir_name) { > cgroup_dir_mk(&root->mnt_dir, cgroup_ltp_dir, > &root->ltp_dir); > cgroup_dir_mk(&root->ltp_dir, > cgroup_ltp_drain_dir, &root->drain_dir); > if (!strcmp(value, "yes")) > root->ltp_dir.we_created_it = 1; > > > > + > > + } else if (!strcmp(key, CONFIG_TESTID_KEY) && > strcmp(value, "NULL") && > > + !root->test_dir.dir_name) { > > + cgroup_dir_mk(&root->ltp_dir, value, > &root->test_dir); > > + root->test_dir.we_created_it = 1; > > + } > > + } > > + > > + return 0; > > +} > > > > I'll be posting the changes along with all the modified tests probably later today or early next week, and I will definitely need help reviewing that :) > > -- > Regards, > Li Wang > > Thanks as always, - Luke
diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h index cfcc189ee..9785bf7b9 100644 --- a/include/tst_cgroup.h +++ b/include/tst_cgroup.h @@ -112,7 +112,9 @@ struct tst_cgroup_group; void tst_cgroup_scan(void); /* Print the config detected by tst_cgroup_scan */ void tst_cgroup_print_config(void); - +/* Load the config printed by tst_cgroup_print_config() to update the + * the internal state of the test to the given config */ +void tst_cgroup_load_config(const char *const config); /* Ensure the specified controller is available in the test's default * CGroup, mounting/enabling it if necessary */ void tst_cgroup_require(const char *const ctrl_name, diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c index 1cec3e722..9b3c639fc 100644 --- a/lib/tst_cgroup.c +++ b/lib/tst_cgroup.c @@ -378,6 +378,114 @@ static struct cgroup_root *cgroup_find_root(const char *const mnt_path) return root; } +static int parse_ctrl_config(const char *const config_entry) +{ + char *token; + struct cgroup_ctrl *ctrl = NULL; + int we_require_it = 0; + + if (!strcmp(CONFIG_CTRL_HEADER, config_entry)) + return 0; + + if (!strcmp(CONFIG_ROOT_HEADER, config_entry)) + return 1; + + for (token = strtok(config_entry, " "); token; token = strtok(NULL, " ")) { + if (!ctrl && !(ctrl = cgroup_find_ctrl(token))) + tst_brk(TBROK, "Could not find ctrl from config. Ctrls changing between calls?"); + + if (ctrl && !strcmp(token, CONFIG_CTRL_REQUIRED)) + ctrl->we_require_it = 1; + } + + return 0; +} + +static int parse_root_config(char *config_entry) +{ + char *key; + char *value; + struct cgroup_root *root; + + for (key = strtok(config_entry, " "); key; key = strtok(NULL, " ")) { + if (!(value = strchr(key, '='))) { + if (!(root = cgroup_find_root(key))) + tst_brk(TBROK, "Could not find root from config. Roots changing between calls?"); + + continue; + } + + *value = '\0'; + value = value + 1; + + if (!strcmp(key, CONFIG_MOUNTROOT_KEY) && !strcmp(value, "yes")) { + root->we_mounted_it = 1; + + } else if (!strcmp(key, CONFIG_LTPDIR_KEY) && !root->ltp_dir.dir_name) { + cgroup_dir_mk(&root->mnt_dir, cgroup_ltp_dir, &root->ltp_dir); + if (!strcmp(value, "yes")) + root->ltp_dir.we_created_it = 1; + + } else if (!strcmp(key, CONFIG_DRAINDIR_KEY) && !root->drain_dir.dir_name) { + cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir); + if (!strcmp(value, "yes")) + root->ltp_dir.we_created_it = 1; + + } else if (!strcmp(key, CONFIG_TESTID_KEY) && strcmp(value, "NULL") && + !root->test_dir.dir_name) { + cgroup_dir_mk(&root->ltp_dir, value, &root->test_dir); + root->test_dir.we_created_it = 1; + } + } + + return 0; +} + +/* Load the test state configuration provided by tst_cgroup_print_config() + * + * This will reload some internal tst_cgroup state given by the config + * that might otherwise have been lost between calls or between different + * processes. In particular this is used by tools/cgroup/tst_cgctl to + * provide access to this C api to shell scripts. + * + * The config keeps track of the minimal state needed for tst_cgroup_cleanup + * to cleanup after a test and does not keep track of the creation of + * test cgroups that might be created through tst_cgroup_group_mk(). + */ +void tst_cgroup_load_config(const char *const config) +{ + char temp_config[BUFSIZ]; + char *curr_line; + char *next_line; + + if (strlen(config) >= BUFSIZ) + tst_brk(TBROK, "Config has exceeded buffer size?"); + + strncpy(temp_config, config, BUFSIZ); + + for (curr_line = &temp_config[0]; curr_line; curr_line = next_line + 1) { + next_line = strchr(curr_line, '\n'); + if (next_line) + *next_line = '\0'; + + if (parse_ctrl_config(curr_line)) + break; + } + + for (curr_line = next_line + 1; curr_line; curr_line = next_line + 1) { + next_line = strchr(curr_line, '\n'); + if (next_line) + *next_line = '\0'; + + parse_root_config(curr_line); + /* Bash will truncate the last newline that we are using to deliminate + * the start and end of valid entries, so if we could not detect any more + * newlines we assume that was our last entry. */ + if (!next_line) + break; + } +} + /* Determine if a mounted cgroup hierarchy is unique and record it if so. * * For CGroups V2 this is very simple as there is only one
Implement tst_cgroup_load_config which consumes the state given by tst_cgroup_print_config() to update the internal test state to reflect the given config. This allows for programs using the cgroup C API to load and reload state, allowing functionality such as calling tst_cgroup_require and tst_cgroup_cleanup to function properly between programs or between invocations of a binary using the C API. Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> --- include/tst_cgroup.h | 4 +- lib/tst_cgroup.c | 108 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-)