From a9d7ad1175c4ac4afa77d1058b3863f7c3418eab Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Wed, 8 Dec 2021 21:09:12 +0100 Subject: [PATCH] switch to libosmocore multihread-logging When osmo-remsim was originally developed, libosmocore logging was not yet thread-safe. This meant that the worker threads of remsim-bankd and remsim-server could not log via the libosmocore logging framework but directly used stderr/stdout, which produced rather inconsistent log output. However, since 1.3.0, libosmocore has received support for multi-threaded applications. Let's make use of this and consistently use it in remsim-server and remsim-bankd. This obviously also means adding some more log categories. Change-Id: I7bd5264c559b756927046563a2d00c54826bee9b --- src/bankd/bankd.h | 4 ++-- src/bankd/bankd_main.c | 2 ++ src/debug.c | 15 +++++++++++++++ src/debug.h | 3 +++ src/server/remsim_server.c | 2 ++ src/server/rest_api.c | 8 ++++---- src/server/rspro_server.c | 4 ++-- src/slotmap.c | 8 ++++---- 8 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/bankd/bankd.h b/src/bankd/bankd.h index 6de3213..c007802 100644 --- a/src/bankd/bankd.h +++ b/src/bankd/bankd.h @@ -20,8 +20,8 @@ extern struct value_string worker_state_names[]; #define LOGW(w, fmt, args...) \ - printf("[%03u %s] %s:%u " fmt, (w)->num, get_value_string(worker_state_names, (w)->state), \ - __FILE__, __LINE__, ## args) + LOGP(DBANKDW, LOGL_INFO, "[%03u %s] " fmt, (w)->num, get_value_string(worker_state_names, (w)->state), \ + ## args) struct bankd; diff --git a/src/bankd/bankd_main.c b/src/bankd/bankd_main.c index 3842b0c..5b70b88 100644 --- a/src/bankd/bankd_main.c +++ b/src/bankd/bankd_main.c @@ -80,6 +80,8 @@ static void bankd_init(struct bankd *bankd) log_set_print_category(osmo_stderr_target, 1); log_set_print_category_hex(osmo_stderr_target, 0); osmo_fsm_log_addr(0); + log_set_print_tid(osmo_stderr_target, 1); + log_enable_multithread(); asn_debug = 0; diff --git a/src/debug.c b/src/debug.c index a538697..2dfc16e 100644 --- a/src/debug.c +++ b/src/debug.c @@ -41,6 +41,21 @@ static const struct log_info_cat default_categories[] = { .loglevel = LOGL_INFO, .enabled = 1, }, + [DREST] = { + .name = "DREST", + .loglevel = LOGL_INFO, + .enabled = 1, + }, + [DSLOTMAP] = { + .name = "DSLOTMAP", + .loglevel = LOGL_INFO, + .enabled = 1, + }, + [DBANKDW] = { + .name = "DBANKDW", + .loglevel = LOGL_INFO, + .enabled = 1, + }, }; const struct log_info log_info = { diff --git a/src/debug.h b/src/debug.h index 51331f4..88288ca 100644 --- a/src/debug.h +++ b/src/debug.h @@ -5,6 +5,9 @@ enum { DMAIN, DST2, DRSPRO, + DREST, + DSLOTMAP, + DBANKDW, }; extern const struct log_info log_info; diff --git a/src/server/remsim_server.c b/src/server/remsim_server.c index 49107b9..c2d54e5 100644 --- a/src/server/remsim_server.c +++ b/src/server/remsim_server.c @@ -93,6 +93,8 @@ int main(int argc, char **argv) log_set_print_category(osmo_stderr_target, 1); log_set_print_category_hex(osmo_stderr_target, 0); osmo_fsm_log_addr(0); + log_set_print_tid(osmo_stderr_target, 1); + log_enable_multithread(); handle_options(argc, argv); diff --git a/src/server/rest_api.c b/src/server/rest_api.c index ff1b588..540fbd4 100644 --- a/src/server/rest_api.c +++ b/src/server/rest_api.c @@ -304,7 +304,7 @@ static void trigger_main_thread_via_eventfd(void) rc = write(g_event_ofd.fd, &one, sizeof(one)); if (rc < 8) - fprintf(stderr, "Error writing to eventfd(): %d\n", rc); + LOGP(DREST, LOGL_ERROR, "Error writing to eventfd(): %d\n", rc); } static int api_cb_slotmaps_post(const struct _u_request *req, struct _u_response *resp, void *user_data) @@ -318,7 +318,7 @@ static int api_cb_slotmaps_post(const struct _u_request *req, struct _u_response json_req = ulfius_get_json_body_request(req, &json_err); if (!json_req) { - fprintf(stderr, "REST: No JSON Body\n"); + LOGP(DREST, LOGL_NOTICE, "REST: No JSON Body\n"); goto err; } @@ -327,7 +327,7 @@ static int api_cb_slotmaps_post(const struct _u_request *req, struct _u_response goto err; map = slotmap_add(g_rps->slotmaps, &slotmap.bank, &slotmap.client); if (!map) { - fprintf(stderr, "REST: Cannot add slotmap\n"); + LOGP(DREST, LOGL_NOTICE, "REST: Cannot add slotmap\n"); goto err; } slotmap_state_change(map, SLMAP_S_NEW, NULL); @@ -511,7 +511,7 @@ int rest_api_init(void *ctx, uint16_t port) ulfius_add_endpoint(&g_instance, &api_endpoints[i]); if (ulfius_start_framework(&g_instance) != U_OK) { - fprintf(stderr, "Cannot start REST API on port %u\n", port); + LOGP(DREST, LOGL_FATAL, "Cannot start REST API on port %u\n", port); return -1; } return 0; diff --git a/src/server/rspro_server.c b/src/server/rspro_server.c index b35800f..6514f80 100644 --- a/src/server/rspro_server.c +++ b/src/server/rspro_server.c @@ -709,11 +709,11 @@ int event_fd_cb(struct osmo_fd *ofd, unsigned int what) /* read from the socket to "confirm" the event and make it non-readable again */ rc = read(ofd->fd, &value, 8); if (rc < 8) { - fprintf(stderr, "Error reading eventfd: %d\n", rc); + LOGP(DMAIN, LOGL_ERROR, "Error reading eventfd: %d\n", rc); return rc; } - printf("rspro_server: Event FD arrived, checking for any pending work\n"); + LOGP(DMAIN, LOGL_INFO, "Event FD arrived, checking for any pending work\n"); pthread_rwlock_rdlock(&srv->rwlock); llist_for_each_entry(conn, &srv->banks, list) { diff --git a/src/slotmap.c b/src/slotmap.c index b837f01..fe0c6b7 100644 --- a/src/slotmap.c +++ b/src/slotmap.c @@ -103,14 +103,14 @@ struct slot_mapping *slotmap_add(struct slotmaps *maps, const struct bank_slot * map = slotmap_by_bank(maps, bank); if (map) { - fprintf(stderr, "BANKD %u:%u already in use, cannot add new map\n", + LOGP(DSLOTMAP, LOGL_ERROR, "BANKD %u:%u already in use, cannot add new map\n", bank->bank_id, bank->slot_nr); return NULL; } map = slotmap_by_client(maps, client); if (map) { - fprintf(stderr, "CLIENT %u:%u already in use, cannot add new map\n", + LOGP(DSLOTMAP, LOGL_ERROR, "CLIENT %u:%u already in use, cannot add new map\n", client->client_id, client->slot_nr); return NULL; } @@ -132,7 +132,7 @@ struct slot_mapping *slotmap_add(struct slotmaps *maps, const struct bank_slot * #endif slotmaps_unlock(maps); - printf("Slot Map %s added\n", slotmap_name(mapname, sizeof(mapname), map)); + LOGP(DSLOTMAP, LOGL_INFO, "Slot Map %s added\n", slotmap_name(mapname, sizeof(mapname), map)); return map; } @@ -142,7 +142,7 @@ void _slotmap_del(struct slotmaps *maps, struct slot_mapping *map) { char mapname[64]; - printf("Slot Map %s deleted\n", slotmap_name(mapname, sizeof(mapname), map)); + LOGP(DSLOTMAP, LOGL_INFO, "Slot Map %s deleted\n", slotmap_name(mapname, sizeof(mapname), map)); llist_del(&map->list); #ifdef REMSIM_SERVER