<div dir="ltr"><div>Hi Li, <br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 13, 2022 at 10:57 PM Li Wang <<a href="mailto:liwang@redhat.com">liwang@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Luke Nowakowski-Krijger <<a href="mailto:luke.nowakowskikrijger@canonical.com" target="_blank">luke.nowakowskikrijger@canonical.com</a>> wrote:<br>
<br>
> +static int parse_root_config(char *config_entry)<br>
> +{<br>
> +       char *key;<br>
> +       char *value;<br>
> +       struct cgroup_root *root;<br>
> +<br>
> +       for (key = strtok(config_entry, " "); key; key = strtok(NULL, " ")) {<br>
> +               if (!(value = strchr(key, '='))) {<br>
> +                       if (!(root = cgroup_find_root(key)))<br>
> +                               tst_brk(TBROK, "Could not find root from config. Roots changing between calls?");<br>
> +<br>
> +                       continue;<br>
> +               }<br>
> +<br>
> +               *value = '\0';<br>
> +               value = value + 1;<br>
> +<br>
> +               if (!strcmp(key, CONFIG_MOUNTROOT_KEY) && !strcmp(value, "yes")) {<br>
> +                       root->we_mounted_it = 1;<br>
> +<br>
<br>
> +               } else if (!strcmp(key, CONFIG_LTPDIR_KEY) && !root->ltp_dir.dir_name) {<br>
> +                       cgroup_dir_mk(&root->mnt_dir, cgroup_ltp_dir, &root->ltp_dir);<br>
> +                       if (!strcmp(value, "yes"))<br>
> +                               root->ltp_dir.we_created_it = 1;<br>
> +<br>
> +               } else if (!strcmp(key, CONFIG_DRAINDIR_KEY) && !root->drain_dir.dir_name) {<br>
> +                       cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);<br>
> +                       if (!strcmp(value, "yes"))<br>
> +                               root->ltp_dir.we_created_it = 1;<br>
<br>
I think that parsing the  CONFIG_DRAINDIR_KEY from '$_cgroup_state'<br>
is superfluous. Because from the tst_cgroup_cleanup, if<br>
root->ltp_dir.we_created_it<br>
is 1 then both of the two directories will be removed, so just using<br>
CONFIG_LTPDIR_KE<br>
to track the status is enough.<br>
<br></blockquote><div><br></div><div>This is definitely true and I think it makes it a lot simpler. I will add this in the next version.  <br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
And maybe it is not necessary to print "Created_Drain_Dir=xx" in<br>
tst_cgroup_print_config at all.<br>
<br>
Then, the code snippet could be as:<br>
<br>
                } else if (!strcmp(key, CONFIG_LTPDIR_KEY) &&<br>
!root->ltp_dir.dir_name) {<br>
                        cgroup_dir_mk(&root->mnt_dir, cgroup_ltp_dir,<br>
&root->ltp_dir);<br>
                        cgroup_dir_mk(&root->ltp_dir,<br>
cgroup_ltp_drain_dir, &root->drain_dir);<br>
                        if (!strcmp(value, "yes"))<br>
                                root->ltp_dir.we_created_it = 1;<br>
<br>
<br>
> +<br>
> +               } else if (!strcmp(key, CONFIG_TESTID_KEY) && strcmp(value, "NULL") &&<br>
> +                                  !root->test_dir.dir_name) {<br>
> +                       cgroup_dir_mk(&root->ltp_dir, value, &root->test_dir);<br>
> +                       root->test_dir.we_created_it = 1;<br>
> +               }<br>
> +       }<br>
> +<br>
> +       return 0;<br>
> +}<br>
<br>
<br>
<br></blockquote><div> </div><div>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 :) <br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
--<br>
Regards,<br>
Li Wang<br>
<br></blockquote><div><br></div><div>Thanks as always,<br></div><div>- Luke<br></div></div></div>