[LTP] [PATCH 4/6] API/cgroup: Implement tst_cgroup_load_config()

[Luke Nowakowski-Krijger]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20220114/d6856213/attachment.htm>