commit 9a28a60d61b6e6f6e992cefdc0ee056b721d0c8b Author: Jacob Welsh AuthorDate: Wed Nov 2 21:20:42 2022 +0000 Commit: Jacob Welsh CommitDate: Wed Nov 2 21:39:00 2022 +0000 Type: feature removal plugins/old-stats, doveadm: remove stats based on unreliable /proc/self/io This had brought in its own special undocumented "preinit" extension to the modules API. diff --git a/src/doveadm/doveadm-oldstats.c b/src/doveadm/doveadm-oldstats.c index 4be575ea92..dfbad83b12 100644 --- a/src/doveadm/doveadm-oldstats.c +++ b/src/doveadm/doveadm-oldstats.c @@ -48,8 +48,6 @@ struct top_context { }; static struct top_context *sort_ctx = NULL; -static const char *disk_input_field = "disk_input"; -static const char *disk_output_field = "disk_output"; static char ** p_read_next_line(pool_t pool, struct istream *input) @@ -316,8 +314,8 @@ static void stats_top_get_sorting(struct top_context *ctx) return; } if (strcmp(ctx->sort_type, "disk") == 0) { - if (!stats_header_find(ctx, disk_input_field, &ctx->sort_idx1) || - !stats_header_find(ctx, disk_output_field, &ctx->sort_idx2)) + if (!stats_header_find(ctx, "disk_input", &ctx->sort_idx1) || + !stats_header_find(ctx, "disk_output", &ctx->sort_idx2)) i_fatal("disk sort type is missing fields"); return; } @@ -407,15 +405,12 @@ static void stats_top_output(struct top_context *ctx) { static const char *names[] = { "user", "service", "user_cpu", "sys_cpu", - "", "" + "disk_input", "disk_output" }; struct winsize ws; struct top_line *const *lines; unsigned int i, j, row, maxrow, count, indexes[N_ELEMENTS(names)]; - names[4] = disk_input_field; - names[5] = disk_output_field; - /* ANSI clear screen and move cursor to top of screen */ printf("\x1b[2J\x1b[1;1H"); fflush(stdout); doveadm_print_deinit(); @@ -548,10 +543,6 @@ static void cmd_stats_top(struct doveadm_cmd_context *cctx) path = t_strconcat(doveadm_settings->base_dir, "/old-stats", NULL); } - if (!doveadm_cmd_param_bool(cctx, "show-disk-io", &b) && b) { - disk_input_field = "read_bytes"; - disk_output_field = "write_bytes"; - } if (!doveadm_cmd_param_str(cctx, "sort-field", &sort_type)) sort_type = "disk"; @@ -585,10 +576,9 @@ DOVEADM_CMD_PARAMS_END struct doveadm_cmd_ver2 doveadm_cmd_oldstats_top_ver2 = { .cmd = cmd_stats_top, .name = "oldstats top", - .usage = "[-s ] [-b] []", + .usage = "[-s ] []", DOVEADM_CMD_PARAMS_START DOVEADM_CMD_PARAM('s', "socket-path", CMD_PARAM_STR, 0) -DOVEADM_CMD_PARAM('b', "show-disk-io", CMD_PARAM_BOOL, 0) DOVEADM_CMD_PARAM('\0', "sort-field", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL) DOVEADM_CMD_PARAMS_END }; diff --git a/src/plugins/old-stats/mail-stats-fill.c b/src/plugins/old-stats/mail-stats-fill.c index 10d8c398f9..a2156ab578 100644 --- a/src/plugins/old-stats/mail-stats-fill.c +++ b/src/plugins/old-stats/mail-stats-fill.c @@ -7,101 +7,6 @@ #include -#define PROC_IO_PATH "/proc/self/io" - -static bool proc_io_disabled = FALSE; -static int proc_io_fd = -1; - -static int -process_io_buffer_parse(const char *buf, struct mail_stats *stats) -{ - const char *const *tmp; - - tmp = t_strsplit(buf, "\n"); - for (; *tmp != NULL; tmp++) { - if (str_begins(*tmp, "rchar: ")) { - if (str_to_uint64(*tmp + 7, &stats->read_bytes) < 0) - return -1; - } else if (str_begins(*tmp, "wchar: ")) { - if (str_to_uint64(*tmp + 7, &stats->write_bytes) < 0) - return -1; - } else if (str_begins(*tmp, "syscr: ")) { - if (str_to_uint32(*tmp + 7, &stats->read_count) < 0) - return -1; - } else if (str_begins(*tmp, "syscw: ")) { - if (str_to_uint32(*tmp + 7, &stats->write_count) < 0) - return -1; - } - } - return 0; -} - -static int process_io_open(void) -{ - uid_t uid; - - if (proc_io_fd != -1) - return proc_io_fd; - - if (proc_io_disabled) - return -1; - - proc_io_fd = open(PROC_IO_PATH, O_RDONLY); - if (proc_io_fd == -1 && errno == EACCES) { - /* kludge: if we're running with permissions temporarily - dropped, get them temporarily back so we can open - /proc/self/io. */ - uid = geteuid(); - if (seteuid(0) == 0) { - proc_io_fd = open(PROC_IO_PATH, O_RDONLY); - if (seteuid(uid) < 0) { - /* oops, this is bad */ - i_fatal("stats: seteuid(%s) failed", dec2str(uid)); - } - } - errno = EACCES; - } - if (proc_io_fd == -1) { - /* ignore access errors too, certain security options can - prevent root access to this file when not owned by root */ - if (errno != ENOENT && errno != EACCES) - i_error("open(%s) failed: %m", PROC_IO_PATH); - proc_io_disabled = TRUE; - return -1; - } - return proc_io_fd; -} - -static void process_read_io_stats(struct mail_stats *stats) -{ - char buf[1024]; - int fd, ret; - - if ((fd = process_io_open()) == -1) - return; - - ret = pread(fd, buf, sizeof(buf), 0); - if (ret <= 0) { - if (ret == -1) - i_error("read(%s) failed: %m", PROC_IO_PATH); - else - i_error("read(%s) returned EOF", PROC_IO_PATH); - } else if (ret == sizeof(buf)) { - /* just shouldn't happen.. */ - i_error("%s is larger than expected", PROC_IO_PATH); - proc_io_disabled = TRUE; - } else { - buf[ret] = '\0'; - T_BEGIN { - if (process_io_buffer_parse(buf, stats) < 0) { - i_error("Invalid input in file %s", - PROC_IO_PATH); - proc_io_disabled = TRUE; - } - } T_END; - } -} - static void user_trans_stats_get(struct stats_user *suser, struct mail_stats *dest_r) { @@ -141,16 +46,5 @@ void mail_stats_fill(struct stats_user *suser, struct mail_stats *stats_r) stats_r->disk_input = (unsigned long long)usage.ru_inblock * 512ULL; stats_r->disk_output = (unsigned long long)usage.ru_oublock * 512ULL; i_gettimeofday(&stats_r->clock_time); - process_read_io_stats(stats_r); user_trans_stats_get(suser, stats_r); } - -void mail_stats_global_preinit(void) -{ - (void)process_io_open(); -} - -void mail_stats_fill_global_deinit(void) -{ - i_close_fd(&proc_io_fd); -} diff --git a/src/plugins/old-stats/mail-stats.c b/src/plugins/old-stats/mail-stats.c index 3662d23a20..c2230ad7f8 100644 --- a/src/plugins/old-stats/mail-stats.c +++ b/src/plugins/old-stats/mail-stats.c @@ -19,11 +19,6 @@ static struct stats_parser_field mail_stats_fields[] = { EN("disk_input", disk_input), EN("disk_output", disk_output), - EN("read_count", read_count), - EN("read_bytes", read_bytes), - EN("write_count", write_count), - EN("write_bytes", write_bytes), - /*EN("mopen", trans_stats.open_lookup_count), EN("mstat", trans_stats.stat_lookup_count), EN("mfstat", trans_stats.fstat_lookup_count),*/ diff --git a/src/plugins/old-stats/mail-stats.h b/src/plugins/old-stats/mail-stats.h index 7254b696d0..07546a57a3 100644 --- a/src/plugins/old-stats/mail-stats.h +++ b/src/plugins/old-stats/mail-stats.h @@ -17,9 +17,6 @@ struct mail_stats { uint32_t vol_cs, invol_cs; /* disk input/output bytes */ uint64_t disk_input, disk_output; - /* read()/write() syscall count and number of bytes */ - uint32_t read_count, write_count; - uint64_t read_bytes, write_bytes; /* based on struct mailbox_transaction_stats: */ uint32_t trans_lookup_path; @@ -35,7 +32,4 @@ void mail_stats_fill(struct stats_user *suser, struct mail_stats *mail_stats); void mail_stats_add_transaction(struct mail_stats *stats, const struct mailbox_transaction_stats *trans_stats); -void mail_stats_global_preinit(void); -void mail_stats_fill_global_deinit(void); - #endif diff --git a/src/plugins/old-stats/stats-plugin.c b/src/plugins/old-stats/stats-plugin.c index 9d33016f04..15e8d693f5 100644 --- a/src/plugins/old-stats/stats-plugin.c +++ b/src/plugins/old-stats/stats-plugin.c @@ -466,16 +466,10 @@ void old_stats_plugin_init(struct module *module) mail_storage_hooks_add(module, &stats_mail_storage_hooks); } -void old_stats_plugin_preinit(void) -{ - mail_stats_global_preinit(); -} - void old_stats_plugin_deinit(void) { if (global_stats_conn != NULL) stats_connection_unref(&global_stats_conn); - mail_stats_fill_global_deinit(); mail_storage_hooks_remove(&stats_mail_storage_hooks); stats_unregister(&mail_stats_item); } diff --git a/src/plugins/old-stats/stats-plugin.h b/src/plugins/old-stats/stats-plugin.h index dedcfbd176..33e0aedd94 100644 --- a/src/plugins/old-stats/stats-plugin.h +++ b/src/plugins/old-stats/stats-plugin.h @@ -54,7 +54,6 @@ extern MODULE_CONTEXT_DEFINE(stats_user_module, &mail_user_module_register); extern MODULE_CONTEXT_DEFINE(stats_storage_module, &mail_storage_module_register); void old_stats_plugin_init(struct module *module); -void old_stats_plugin_preinit(void); void old_stats_plugin_deinit(void); #endif