diff mbox series

[v2,04/16] API/cgroup: Implement tst_cgroup_load_config()

Message ID 5a65fad42ee618e0191cc664d8da7feeaa754cc0.1642601554.git.luke.nowakowskikrijger@canonical.com
State Superseded
Headers show
Series Expand Cgroup lib and modify controller tests | expand

Commit Message

Luke Nowakowski-Krijger Jan. 19, 2022, 2:44 p.m. UTC
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>
---
v2: Add root null check in parse_root_config.
    Remove checking for ltp_drain_dir key from config as it was
    redundant.
    Remove unsued variable in parse_ctrl_config.
    Cleanup some compiler warnings.

 include/tst_cgroup.h |   4 +-
 lib/tst_cgroup.c     | 106 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+), 1 deletion(-)

Comments

Richard Palethorpe Jan. 24, 2022, 12:05 p.m. UTC | #1
Hello Luke,

Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:

> 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.

I'm afraid I have to say this looks way more complicated than it needs
to be. We control the input format after all.

If you can make each line the same format then it may be possible to
just use a single scanf on each line. Note that it's only possible to
have ~14 controllers, so we can even afford to repeat the root info on
each line.
Luke Nowakowski-Krijger March 2, 2022, 6:08 p.m. UTC | #2
Hi Richard and Li,

Sorry for the hiatus and the delay on getting this done. There was some
other stuff keeping me from finishing things up here.

On Mon, Jan 24, 2022 at 4:22 AM Richard Palethorpe <rpalethorpe@suse.de>
wrote:

> Hello Luke,
>
> Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:
>
> > 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.
>
> I'm afraid I have to say this looks way more complicated than it needs
> to be. We control the input format after all.
>
> If you can make each line the same format then it may be possible to
> just use a single scanf on each line. Note that it's only possible to
> have ~14 controllers, so we can even afford to repeat the root info on
> each line.
>
>
I have read through all the feedback and I agree with all of it. I
implemented something along the lines of what you described here and I
agree that it looks a lot better and much much more simple. Hopefully get
that out to you soon to review.


> --
> Thank you,
> Richard.
>

Thanks,
Luke
diff mbox series

Patch

diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index 7c309edbd..0987cb047 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 7a406c9db..df541d26a 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -375,6 +375,112 @@  static struct cgroup_root *cgroup_find_root(const char *const mnt_path)
 	return NULL;
 }
 
+static int parse_ctrl_config(const char *const config_entry)
+{
+	char *token;
+	struct cgroup_ctrl *ctrl = NULL;
+
+	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 = NULL;
+
+	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;
+		}
+
+		if (!root)
+			tst_brk(TBROK, "Root has not been found, possibly malformed config?");
+
+		*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);
+			cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
+			if (!strcmp(value, "yes")) {
+				root->ltp_dir.we_created_it = 1;
+				root->drain_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 testcases/lib/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