From: Daniel Wagner Date: Tue, 7 May 2024 14:17:59 +0000 (+0200) Subject: log: split log variables from root object X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=bec7a98636546f68ff1efe6794b2d546cf63febc;p=users%2Fsagi%2Flibnvme.git 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 --- 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..e7326037 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(r->log.fd, 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..eaf74e1d 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 = { + .fd = STDERR_FILENO, + .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", + dprintf(l->fd, "%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.fd = fileno(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.fd = r->log.fd; + 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 3793ad46..2635de50 100644 --- a/src/nvme/mi.c +++ b/src/nvme/mi.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -41,18 +42,32 @@ 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; + int fd; + r = calloc(1, sizeof(*r)); if (!r) { + errno = ENOMEM; return NULL; } - r->log_level = log_level; - r->fp = stderr; + + if (fp) { + fd = fileno(fp); + if (fd < 0) { + free(r); + return NULL; + } + } else + fd = STDERR_FILENO; + + r->log.fd = fd; + 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..f2d7b5be 100644 --- a/src/nvme/private.h +++ b/src/nvme/private.h @@ -160,15 +160,19 @@ struct nvme_fabric_options { bool trsvcid; }; +struct nvme_log { + int fd; + 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..d3ce49d8 100644 --- a/src/nvme/tree.c +++ b/src/nvme/tree.c @@ -187,19 +187,30 @@ 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; + int fd; + r = calloc(1, sizeof(*r)); if (!r) { errno = ENOMEM; return NULL; } - r->log_level = log_level; - r->fp = stderr; - if (fp) - r->fp = fp; + + if (fp) { + fd = fileno(fp); + if (fd < 0) { + free(r); + return NULL; + } + } else + fd = STDERR_FILENO; + + r->log.fd = fd; + r->log.level = log_level; + list_head_init(&r->hosts); list_head_init(&r->endpoints); - nvme_set_root(r); + return r; } @@ -368,7 +379,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); }