Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a file exists check to adaptived #24

Closed
wants to merge 3 commits into from
Closed

Add a file exists check to adaptived #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b0dfa5
adaptived: add a function to check for file exists
gkennedy12 Nov 8, 2024
9868425
adaptived: add adaptived_file_exists() to the effects
gkennedy12 Nov 8, 2024
be33b14
adaptived: add adaptived_file_exists() to the causes
gkennedy12 Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions adaptived/include/adaptived-utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,15 @@ int adaptived_path_walk_next(struct adaptived_path_walk_handle **handle, char **
*/
void adaptived_path_walk_end(struct adaptived_path_walk_handle **handle);

/**
* Check if the file/directory exists.
* @param path to file/directory
*
* @Note if an asterisk char is in the path, it will be replaced with '\0'
* @return 0 if file exists, else return -EEXIST
*/
int adaptived_file_exists(const char * const path);


enum cg_setting_enum {
CG_SETTING = 0,
Expand Down
4 changes: 4 additions & 0 deletions adaptived/src/causes/cgroup_setting.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ int _cgset_main(struct adaptived_cause * const cse, int time_since_last_run)

val.type = opts->threshold.type;

ret = adaptived_file_exists(opts->setting);
if (ret)
return ret;

ret = adaptived_cgroup_get_value(opts->setting, &val);
if (ret)
return ret;
Expand Down
4 changes: 4 additions & 0 deletions adaptived/src/causes/meminfo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ int meminfo_main(struct adaptived_cause * const cse, int time_since_last_run)
long long ll_value = 0;
int ret;

ret = adaptived_file_exists(opts->meminfo_file);
if (ret)
return ret;

ret = adaptived_get_meminfo_field(opts->meminfo_file, opts->field, &ll_value);
if (ret)
return ret;
Expand Down
4 changes: 4 additions & 0 deletions adaptived/src/causes/memorystat.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ int memorystat_main(struct adaptived_cause * const cse, int time_since_last_run)
long long ll_value = 0;
int ret;

ret = adaptived_file_exists(opts->stat_file);
if (ret)
return ret;

ret = adaptived_cgroup_get_memorystat_field(opts->stat_file, opts->field, &ll_value);
if (ret)
return ret;
Expand Down
8 changes: 8 additions & 0 deletions adaptived/src/causes/pressure.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ int pressure_main(struct adaptived_cause * const cse, int time_since_last_run)
int ret;

if (opts->common.meas == PRESSURE_SOME_TOTAL || opts->common.meas == PRESSURE_FULL_TOTAL) {
ret = adaptived_file_exists(opts->common.pressure_file);
if (ret)
return ret;

ret = adaptived_get_pressure_total(opts->common.pressure_file, opts->common.meas,
&int_press);
if (ret)
Expand Down Expand Up @@ -200,6 +204,10 @@ int pressure_main(struct adaptived_cause * const cse, int time_since_last_run)
return -EINVAL;
}
} else {
ret = adaptived_file_exists(opts->common.pressure_file);
if (ret)
return ret;

ret = adaptived_get_pressure_avg(opts->common.pressure_file, opts->common.meas,
&float_press);
if (ret)
Expand Down
4 changes: 4 additions & 0 deletions adaptived/src/causes/pressure_rate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ int pressure_rate_main(struct adaptived_cause * const cse, int time_since_last_r
/* Currently unsupported */
return -EINVAL;
} else {
ret = adaptived_file_exists(opts->common.pressure_file);
if (ret)
return ret;

ret = adaptived_get_pressure_avg(opts->common.pressure_file, opts->common.meas,
&float_press);
if (ret)
Expand Down
4 changes: 4 additions & 0 deletions adaptived/src/causes/slabinfo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ int slabinfo_main(struct adaptived_cause * const cse, int time_since_last_run)
long long ll_value = 0;
int ret;

ret = adaptived_file_exists(opts->slabinfo_file);
if (ret)
return ret;

ret = adaptived_get_slabinfo_field(opts->slabinfo_file, opts->field, opts->column, &ll_value);
if (ret)
return ret;
Expand Down
6 changes: 6 additions & 0 deletions adaptived/src/causes/top.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,9 @@ int top_main(struct adaptived_cause * const cse, int time_since_last_run)
int ret;

if (opts->field <= TOP_CPU_ST) {
ret = adaptived_file_exists(opts->stat_file);
if (ret)
return ret;
ret = get_proc_stat_total(opts);
if (ret) {
adaptived_err("top_main: get_proc_stat_total() failed. ret=%d\n", ret);
Expand All @@ -433,6 +436,9 @@ int top_main(struct adaptived_cause * const cse, int time_since_last_run)
return 0;
}
} else {
ret = adaptived_file_exists(opts->meminfo_file);
if (ret)
return ret;
memset(&meminfo, 0, sizeof(struct proc_meminfo));
ret = calc_meminfo(opts, &meminfo);
if (ret) {
Expand Down
8 changes: 8 additions & 0 deletions adaptived/src/effects/cgroup_setting.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ static int _cgroup_setting_init(struct adaptived_effect * const eff, struct json
if (ret)
goto error;

ret = adaptived_file_exists(setting_str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if we're going to check if the file exists, I think we need a way for the user to skip this check.

What if they had a config like this:

    causes:
        always
    effects:
        make_a_cgroup
        operate_on_the_cgroup_just_made

A config like the above wouldn't work with the checks as written because the cgroup doesn't exist until the make_a_cgroup's main runs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could mimic the -ENOENT logic for other optional flags

if (ret)
goto error;

opts->setting = malloc(sizeof(char) * strlen(setting_str) + 1);
if (!opts->setting) {
ret = -ENOMEM;
Expand All @@ -90,6 +94,10 @@ static int _cgroup_setting_init(struct adaptived_effect * const eff, struct json
goto error;
ret = 0;
} else {
ret = adaptived_file_exists(setting_str);
if (ret)
goto error;

opts->pre_set_from = malloc(sizeof(char) * strlen(setting_str) + 1);
if (!opts->pre_set_from) {
ret = -ENOMEM;
Expand Down
8 changes: 8 additions & 0 deletions adaptived/src/effects/cgroup_setting_by_psi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ int cgroup_setting_psi_init(struct adaptived_effect * const eff, struct json_obj
if (ret)
goto error;

ret = adaptived_file_exists(cgroup_path_str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cgroup_setting_by_psi effect can operate on a hierarchy, thus * is valid in the path (and it may not map to a valid path.)

I think a better option here would be to do something like:

path_len = strlen(cgroup_path_str);
if (cgroup_path_str[path_len - 1] == '*') {
    /*
     * Skip file existence check because a wildcard path was provided
     */
} else {
    ret = adaptived_file_exists(cgroup_path_str);
    if (ret)
        goto error;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there could be some copy-pasta here, you could incorporate this into adaptived_file_exists() with a parameter like skip_wildcard or something like that

if (ret)
goto error;

opts->cgroup_path = malloc(sizeof(char) * strlen(cgroup_path_str) + 1);
if (!opts->cgroup_path) {
ret = -ENOMEM;
Expand Down Expand Up @@ -433,6 +437,10 @@ int cgroup_setting_psi_main(struct adaptived_effect * const eff)
full_setting_path[strlen(max_cgroup_path) +
strlen(opts->cgroup_setting) + 1] = '\0';

ret = adaptived_file_exists(full_setting_path);
if (ret)
goto error;

ret = calculate_value(opts, full_setting_path, &value);
if (ret)
goto error;
Expand Down
12 changes: 4 additions & 8 deletions adaptived/src/effects/copy_cgroup_setting.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,9 @@ int copy_cgroup_setting_init(struct adaptived_effect * const eff, struct json_ob
if (ret)
goto error;

if (access(setting_str, F_OK) != 0) {
adaptived_err("%s: can't find %s\n", __func__, setting_str);
ret = -EEXIST;
ret = adaptived_file_exists(setting_str);
if (ret)
goto error;
}

opts->from_setting = malloc(sizeof(char) * strlen(setting_str) + 1);
if (!opts->from_setting) {
Expand All @@ -80,11 +78,9 @@ int copy_cgroup_setting_init(struct adaptived_effect * const eff, struct json_ob
if (ret)
goto error;

if (access(setting_str, F_OK) != 0) {
adaptived_err("%s: can't find %s\n", __func__, setting_str);
ret = -EEXIST;
ret = adaptived_file_exists(setting_str);
if (ret)
goto error;
}

opts->to_setting = malloc(sizeof(char) * strlen(setting_str) + 1);
if (!opts->to_setting) {
Expand Down
4 changes: 4 additions & 0 deletions adaptived/src/effects/kill_cgroup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ int kill_cgroup_init(struct adaptived_effect * const eff, struct json_object *ar
if (ret)
goto error;

ret = adaptived_file_exists(cgroup_path_str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kill_cgroup also allows wildcards. See comment above in the PSI effect.

if (ret)
goto error;

opts->cgroup_path = malloc(sizeof(char) * strlen(cgroup_path_str) + 1);
if (!opts->cgroup_path) {
ret = -ENOMEM;
Expand Down
4 changes: 4 additions & 0 deletions adaptived/src/effects/kill_cgroup_by_psi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ int kill_cgroup_psi_init(struct adaptived_effect * const eff, struct json_object
if (ret)
goto error;

ret = adaptived_file_exists(cgroup_path_str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kill_cgroup_by_psi also allows wildcards. See comment above in the PSI effect.

if (ret)
goto error;

opts->cgroup_path = malloc(sizeof(char) * strlen(cgroup_path_str) + 1);
if (!opts->cgroup_path) {
ret = -ENOMEM;
Expand Down
17 changes: 17 additions & 0 deletions adaptived/src/utils/path_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,3 +269,20 @@ API void adaptived_path_walk_end(struct adaptived_path_walk_handle **handle)

*handle = NULL;
}

API int adaptived_file_exists(const char * const path)
{
char check_path[FILENAME_MAX];
char *subp;

strcpy(check_path, path);
subp = strstr(check_path, "*");
Copy link
Member

@drakenclimber drakenclimber Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of replacing * with \0. While unusual, * is valid in filenames and could occur anywhere in the file. Also, * is used by adaptived for wildcarding of paths.

I don't think we want this check.

if (subp)
*subp = '\0';

if (access(check_path, F_OK) != 0) {
adaptived_err("%s: can't find %s, errno=%d\n", __func__, check_path, errno);
return -EEXIST;
}
return 0;
}
Loading