From e21fadf45d9eb2e4fca8737c0f8958a13a4f6f0d Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Tue, 7 May 2024 16:17:59 +0200 Subject: [PATCH] log: split log variables from root object The original plan was to avoid any global variables in the library. Thus the logging variables were tied to the root object which is available via the fabric API. The default NVME API doesn't have the root object thus we had to add a global variable to set log level etc. But the API to set these variables is done via the root object. This approach enforces the caller side to create an default root object to initialize the default logging level. This introduces unnecessary complexity on the caller side and wastes resources. Let's split the default logging setting from the root object. Signed-off-by: Daniel Wagner --- src/libnvme.map | 5 ++++ src/nvme/json.c | 2 +- src/nvme/log.c | 66 +++++++++++++++++++++++++++---------------- src/nvme/log.h | 25 ++++++++++++---- src/nvme/mi.c | 14 +++++---- src/nvme/private.h | 12 +++++--- src/nvme/tree.c | 14 ++++----- test/nbft/nbft-dump.c | 5 +++- 8 files changed, 95 insertions(+), 48 deletions(-) diff --git a/src/libnvme.map b/src/libnvme.map index 95d9c98c..b610a8ff 100644 --- a/src/libnvme.map +++ b/src/libnvme.map @@ -1,4 +1,9 @@ # SPDX-License-Identifier: LGPL-2.1-or-later +LIBNVME_1.10 { + global: + nvme_init_default_logging; +}; + LIBNVME_1.9 { global: nvme_ctrl_get_cntlid; diff --git a/src/nvme/json.c b/src/nvme/json.c index a02bd2d8..ab95f656 100644 --- a/src/nvme/json.c +++ b/src/nvme/json.c @@ -633,7 +633,7 @@ int json_dump_tree(nvme_root_t r) } json_object_object_add(json_root, "hosts", host_array); - ret = json_object_to_fd(fileno(r->fp), json_root, JSON_C_TO_STRING_PRETTY); + ret = json_object_to_fd(fileno(r->log.fp), json_root, JSON_C_TO_STRING_PRETTY); if (ret < 0) { nvme_msg(r, LOG_ERR, "Failed to write, %s\n", json_util_get_last_err()); diff --git a/src/nvme/log.c b/src/nvme/log.c index d3f84104..83fb64d5 100644 --- a/src/nvme/log.c +++ b/src/nvme/log.c @@ -26,13 +26,18 @@ #define LOG_CLOCK CLOCK_MONOTONIC #endif -static nvme_root_t root; +static struct nvme_log def_log = { + .fp = NULL, + .level = DEFAULT_LOGLEVEL, + .pid = false, + .timestamp = false, +}; void __attribute__((format(printf, 4, 5))) __nvme_msg(nvme_root_t r, int level, const char *func, const char *format, ...) { - FILE *fp = stderr; + struct nvme_log *l; va_list ap; char pidbuf[16]; char timebuf[32]; @@ -50,18 +55,15 @@ __nvme_msg(nvme_root_t r, int level, _cleanup_free_ char *message = NULL; int idx = 0; - if (!r) - r = root; - if (r) - fp = r->fp; + l = &r->log; + else + l = &def_log; - if (r && level > r->log_level) - return; - if (!r && level > DEFAULT_LOGLEVEL) + if (level > l->level) return; - if (r && r->log_timestamp) { + if (l->timestamp) { struct timespec now; clock_gettime(LOG_CLOCK, &now); @@ -71,7 +73,7 @@ __nvme_msg(nvme_root_t r, int level, } else *timebuf = '\0'; - if (r && r->log_pid) { + if (l->pid) { snprintf(pidbuf, sizeof(pidbuf), "%ld", (long)getpid()); idx |= 1 << 1; } else @@ -89,42 +91,56 @@ __nvme_msg(nvme_root_t r, int level, message = NULL; va_end(ap); - fprintf(fp, "%s%s", + fprintf(l->fp, "%s%s", header ? header : "", message ? message : ""); } void nvme_init_logging(nvme_root_t r, int lvl, bool log_pid, bool log_tstamp) { - r->log_level = lvl; - r->log_pid = log_pid; - r->log_timestamp = log_tstamp; + r->log.level = lvl; + r->log.pid = log_pid; + r->log.timestamp = log_tstamp; } int nvme_get_logging_level(nvme_root_t r, bool *log_pid, bool *log_tstamp) { - if (!r) - r = root; - if (!r) - return DEFAULT_LOGLEVEL; + struct nvme_log *l; + + if (r) + l = &r->log; + else + l = &def_log; + if (log_pid) - *log_pid = r->log_pid; + *log_pid = l->pid; if (log_tstamp) - *log_tstamp = r->log_timestamp; - return r->log_level; + *log_tstamp = l->timestamp; + return l->level; +} + +void nvme_init_default_logging(FILE *fp, int level, bool log_pid, bool log_tstamp) +{ + def_log.fp = fp; + def_log.level = level; + def_log.pid = log_pid; + def_log.timestamp = log_tstamp; } void nvme_set_root(nvme_root_t r) { - root = r; + def_log.fp = r->log.fp; + def_log.level = r->log.level; + def_log.pid = r->log.pid; + def_log.timestamp = r->log.timestamp; } void nvme_set_debug(bool debug) { - root->log_level = debug ? LOG_DEBUG : DEFAULT_LOGLEVEL; + def_log.level = debug ? LOG_DEBUG : DEFAULT_LOGLEVEL; } bool nvme_get_debug(void) { - return root->log_level == LOG_DEBUG; + return def_log.level == LOG_DEBUG; } diff --git a/src/nvme/log.h b/src/nvme/log.h index cd243ea9..80c642aa 100644 --- a/src/nvme/log.h +++ b/src/nvme/log.h @@ -35,6 +35,18 @@ */ void nvme_init_logging(nvme_root_t r, int lvl, bool log_pid, bool log_tstamp); +/** + * nvme_init_default_logging() - Initialize default (fallback) logging + * @fp: File descriptor for logging messages + * @lvl: Logging level to set + * @log_pid: Boolean to enable logging of the PID + * @log_tstamp: Boolean to enable logging of the timestamp + * + * Sets the default logging settings for the library in case the root object + * is absent. + */ +void nvme_init_default_logging(FILE *fp, int lvl, bool log_pid, bool log_tstamp); + /** * nvme_get_logging_level() - Get current logging level * @r: nvme_root_t context @@ -59,24 +71,27 @@ int nvme_get_logging_level(nvme_root_t r, bool *log_pid, bool *log_tstamp); * will be set as well. This means the global root object is always pointing to * the latest created root object. Note the first @nvme_free_tree call will reset * the global root object. + * + * This function is deprecated. Use nvme_init_default_logging or/and + * nvme_init_logging instead. */ -void nvme_set_root(nvme_root_t r); +void nvme_set_root(nvme_root_t r) __attribute__((deprecated)); /** * nvme_set_debug - Set NVMe command debugging output * @debug: true to enable or false to disable * - * Don't use it, it's debricated. + * This function is deprecated. Use nvme_init_default_logging instead. */ -void nvme_set_debug(bool debug); +void nvme_set_debug(bool debug) __attribute__((deprecated)); /** * nvme_get_debug - Get NVMe command debugging output * - * Don't use it, it's debricated. + * This function is deprecated. Use nvme_get_logging_level instead. * * Return: false if disabled or true if enabled. */ -bool nvme_get_debug(void); +bool nvme_get_debug(void) __attribute__((deprecated)); #endif /* _LOG_H */ diff --git a/src/nvme/mi.c b/src/nvme/mi.c index 5c83e463..d5d3a327 100644 --- a/src/nvme/mi.c +++ b/src/nvme/mi.c @@ -41,18 +41,22 @@ static bool nvme_mi_probe_enabled_default(void) */ nvme_root_t nvme_mi_create_root(FILE *fp, int log_level) { - struct nvme_root *r = calloc(1, sizeof(*r)); + struct nvme_root *r; + r = calloc(1, sizeof(*r)); if (!r) { + errno = ENOMEM; return NULL; } - r->log_level = log_level; - r->fp = stderr; + + r->log.fp = fp ? fp : stderr; + r->log.level = log_level; + r->mi_probe_enabled = nvme_mi_probe_enabled_default(); - if (fp) - r->fp = fp; + list_head_init(&r->hosts); list_head_init(&r->endpoints); + return r; } diff --git a/src/nvme/private.h b/src/nvme/private.h index 18363502..ca5a6cf9 100644 --- a/src/nvme/private.h +++ b/src/nvme/private.h @@ -160,15 +160,19 @@ struct nvme_fabric_options { bool trsvcid; }; +struct nvme_log { + FILE *fp; + int level; + bool pid; + bool timestamp; +}; + struct nvme_root { char *config_file; char *application; struct list_head hosts; struct list_head endpoints; /* MI endpoints */ - FILE *fp; - int log_level; - bool log_pid; - bool log_timestamp; + struct nvme_log log; bool modified; bool mi_probe_enabled; struct nvme_fabric_options *options; diff --git a/src/nvme/tree.c b/src/nvme/tree.c index eb9486de..f1171883 100644 --- a/src/nvme/tree.c +++ b/src/nvme/tree.c @@ -187,19 +187,20 @@ int nvme_scan_topology(struct nvme_root *r, nvme_scan_filter_t f, void *f_args) nvme_root_t nvme_create_root(FILE *fp, int log_level) { - struct nvme_root *r = calloc(1, sizeof(*r)); + struct nvme_root *r; + r = calloc(1, sizeof(*r)); if (!r) { errno = ENOMEM; return NULL; } - r->log_level = log_level; - r->fp = stderr; - if (fp) - r->fp = fp; + + r->log.fp = fp ? fp : stderr; + r->log.level = log_level; + list_head_init(&r->hosts); list_head_init(&r->endpoints); - nvme_set_root(r); + return r; } @@ -368,7 +369,6 @@ void nvme_free_tree(nvme_root_t r) free(r->config_file); if (r->application) free(r->application); - nvme_set_root(NULL); free(r); } diff --git a/test/nbft/nbft-dump.c b/test/nbft/nbft-dump.c index 5fb2f2e9..5fed3c24 100644 --- a/test/nbft/nbft-dump.c +++ b/test/nbft/nbft-dump.c @@ -10,6 +10,7 @@ #include #include #include "libnvme.h" +#include "nvme/log.h" static void print_hex(unsigned char *buf, int len) { @@ -104,12 +105,14 @@ static void print_nbft(struct nbft_info *table) int main(int argc, char **argv) { struct nbft_info *table = NULL; - + if (argc < 2) { fprintf(stderr, "Usage: %s TABLE\n", argv[0]); return 1; } + nvme_init_default_logging(stderr, DEFAULT_LOGLEVEL, false, false); + if (nvme_nbft_read(&table, argv[1]) != 0) { fprintf(stderr, "Error parsing the NBFT table %s: %m\n", argv[1]);