Skip to content

Commit

Permalink
Change logging to use an application callback instead of writing to s…
Browse files Browse the repository at this point in the history
…tderr

Stderr/filedescriptor 2 does not exist for daemons (fork twice and close
all file descriptors) or embedded systems that might not even have a console
or a filesystem to write to.

This fixes RPC_LOG and LOG. Still todo is decide how to handle
TLS_LOG.
All references to stderr must be removed from the library.

Signed-off-by: Ronnie Sahlberg <[email protected]>
  • Loading branch information
sahlberg committed Aug 15, 2024
1 parent e6065ce commit ec62485
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 46 deletions.
12 changes: 11 additions & 1 deletion examples/nfs-stats-cb.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ void stats_cb(struct rpc_context *rpc,
data->size, data->status, data->response_time);
}

void log_cb(struct rpc_context *rpc,
int level, char *msg, void *private_data)
{
fprintf(stderr, "LOG: level:%d msg:%s\n", level, msg);
}

static struct file_context *
open_file(const char *url, int flags)
{
Expand Down Expand Up @@ -122,14 +128,18 @@ open_file(const char *url, int flags)
return NULL;
}

rpc_set_stats_cb(nfs_get_rpc_context(file_context->nfs), stats_cb, NULL);

rpc_set_debug(nfs_get_rpc_context(file_context->nfs), 9);
rpc_set_log_cb(nfs_get_rpc_context(file_context->nfs), log_cb, NULL);

if (nfs_mount(file_context->nfs, file_context->url->server,
file_context->url->path) != 0) {
fprintf(stderr, "Failed to mount nfs share : %s\n",
nfs_get_error(file_context->nfs));
free_file_context(file_context);
return NULL;
}
rpc_set_stats_cb(nfs_get_rpc_context(file_context->nfs), stats_cb, NULL);

if (flags == O_RDONLY) {
if (nfs_open(file_context->nfs, file_context->url->file, flags,
Expand Down
24 changes: 8 additions & 16 deletions include/libnfs-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,10 @@ struct rpc_context {
/* Stats callback */
rpc_stats_cb stats_cb;
void *stats_private_data;

/* Logging callback */
rpc_log_cb log_cb;
void *log_private_data;
};

struct rpc_pdu {
Expand Down Expand Up @@ -593,26 +597,15 @@ void nfs_set_error_locked(struct nfs_context *nfs, char *error_string, ...)

#if defined(PS2_EE)
#define RPC_LOG(rpc, level, format, ...) ;
#define LOG(rpc, level, format, ...) ;
#else
#define RPC_LOG(rpc, level, format, ...) \
do { \
if (level <= rpc->debug) { \
fprintf(stderr, "libnfs:%d rpc %p " format "\n", level, rpc, ## __VA_ARGS__); \
if (rpc->log_cb && level <= rpc->debug) { \
char buf[256]; \
snprintf(buf, 255, "libnfs:%d rpc %p " format, level, rpc, ## __VA_ARGS__); \
rpc->log_cb(rpc, level, buf, rpc->log_private_data); \
} \
} while (0)
/*
* Use LOG() for logging from code where there is no rpc_context.
* It only provides simple unconditional logging since we don't have any debug
* level to compare against.
*
* Note: Use it sparingly only for critical logs which cannot be conveyed to the
* user through any better means.
*/
#define LOG(format, ...) \
do { \
fprintf(stderr, "libnfs: " format "\n", ## __VA_ARGS__); \
} while (0)
#endif

const char *nfs_get_server(struct nfs_context *nfs);
Expand All @@ -632,7 +625,6 @@ void rpc_set_resiliency(struct rpc_context *rpc,
void rpc_set_interface(struct rpc_context *rpc, const char *ifname);

void rpc_set_tcp_syncnt(struct rpc_context *rpc, int v);
void rpc_set_debug(struct rpc_context *rpc, int level);
void rpc_set_poll_timeout(struct rpc_context *rpc, int poll_timeout);
int rpc_get_poll_timeout(struct rpc_context *rpc);
void rpc_set_timeout(struct rpc_context *rpc, int timeout);
Expand Down
22 changes: 21 additions & 1 deletion include/nfsc/libnfs-raw.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ EXTERN struct rpc_context *rpc_init_context(void);
EXTERN void rpc_destroy_context(struct rpc_context *rpc);

/*
* Stats callback for all ASYNC rpc functions
* Stats callback for all ASYNC rpc functions.
* When the stats callback is provided it will geterate a callback
* every time a PDU is queued for sending as well as when it has received
* on the socket.
*/
struct rpc_pdu;
struct rpc_stats_cb_data {
Expand All @@ -139,6 +142,23 @@ typedef void (*rpc_stats_cb)(struct rpc_context *rpc,
EXTERN void rpc_set_stats_cb(struct rpc_context *rpc, rpc_stats_cb cb,
void *private_data);

/*
* Set debug level for logging.
*/
void rpc_set_debug(struct rpc_context *rpc, int level);
/*
* Logging is done via a callback.
* Log level is set via rpc_set_debug()/nfs_set_debug()
*/
typedef void (*rpc_log_cb)(struct rpc_context *rpc,
int level, char *msg, void *private_data);
/*
* The callback executes in the context of the event-loop so it is vital
* that the callback will never block and will return as fast as possible.
*/
EXTERN void rpc_set_log_cb(struct rpc_context *rpc, rpc_log_cb cb,
void *private_data);


/*
* Commands that are in flight are kept on linked lists and keyed by
Expand Down
8 changes: 8 additions & 0 deletions lib/libnfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -2595,3 +2595,11 @@ void rpc_set_stats_cb(struct rpc_context *rpc, rpc_stats_cb cb,
rpc->stats_cb = cb;
rpc->stats_private_data = private_data;
}

void rpc_set_log_cb(struct rpc_context *rpc, rpc_log_cb cb,
void *private_data)
{
rpc->log_cb = cb;
rpc->log_private_data = private_data;
}

24 changes: 2 additions & 22 deletions lib/multithreading.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,22 +142,16 @@ int nfs_mt_mutex_init(libnfs_mutex_t *mutex)

ret = pthread_mutexattr_init(&attr);
if (ret != 0) {
LOG("pthread_mutexattr_init() failed: %d (%s)", ret, strerror(ret));
assert(0);
return ret;
}

ret = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK);
if (ret != 0) {
LOG("pthread_mutexattr_settype() failed: %d (%s)", ret, strerror(ret));
assert(0);
return ret;
}

ret = pthread_mutex_init(mutex, &attr);
if (ret != 0) {
LOG("pthread_mutex_init() failed: %d (%s)", ret, strerror(ret));
assert(0);
return ret;
}
#else
Expand All @@ -174,26 +168,12 @@ int nfs_mt_mutex_destroy(libnfs_mutex_t *mutex)

int nfs_mt_mutex_lock(libnfs_mutex_t *mutex)
{
const int ret = pthread_mutex_lock(mutex);

if (ret != 0) {
LOG("pthread_mutex_lock() failed: %d (%s)", ret, strerror(ret));
assert(0);
}

return ret;
return pthread_mutex_lock(mutex);
}

int nfs_mt_mutex_unlock(libnfs_mutex_t *mutex)
{
const int ret = pthread_mutex_unlock(mutex);

if (ret != 0) {
LOG("pthread_mutex_unlock() failed: %d (%s)", ret, strerror(ret));
assert(0);
}

return ret;
return pthread_mutex_unlock(mutex);
}

#if defined(__APPLE__) && defined(HAVE_DISPATCH_DISPATCH_H)
Expand Down
12 changes: 6 additions & 6 deletions lib/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,14 @@ set_tcp_sockopt(int sockfd, int optname, int value)
}

static int
set_keepalive(int sockfd)
set_keepalive(struct rpc_context *rpc, int sockfd)
{
#ifdef SO_KEEPALIVE
const int enable_keepalive = 1;

if (setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE,
&enable_keepalive, sizeof(enable_keepalive)) != 0) {
LOG("setsockopt(SO_KEEPALIVE) failed: %s", strerror(errno));
RPC_LOG(rpc, 2, "setsockopt(SO_KEEPALIVE) failed: %s", strerror(errno));
return -1;
}
#endif
Expand All @@ -211,7 +211,7 @@ set_keepalive(int sockfd)
const int keepidle_secs = 60;

if (set_tcp_sockopt(sockfd, TCP_KEEPIDLE, keepidle_secs) != 0) {
LOG("setsockopt(TCP_KEEPIDLE) failed: %s", strerror(errno));
RPC_LOG(rpc, 2, "setsockopt(TCP_KEEPIDLE) failed: %s", strerror(errno));
return -1;
}
}
Expand All @@ -223,7 +223,7 @@ set_keepalive(int sockfd)
const int keepinterval_secs = 60;

if (set_tcp_sockopt(sockfd, TCP_KEEPINTVL, keepinterval_secs) != 0) {
LOG("setsockopt(TCP_KEEPINTVL) failed: %s", strerror(errno));
RPC_LOG(rpc, 2, "setsockopt(TCP_KEEPINTVL) failed: %s", strerror(errno));
return -1;
}
}
Expand All @@ -235,7 +235,7 @@ set_keepalive(int sockfd)
const int keepcnt = 3;

if (set_tcp_sockopt(sockfd, TCP_KEEPCNT, keepcnt) != 0) {
LOG("setsockopt(TCP_KEEPCNT) failed: %s", strerror(errno));
RPC_LOG(rpc, 2, "setsockopt(TCP_KEEPCNT) failed: %s", strerror(errno));
return -1;
}
}
Expand Down Expand Up @@ -1474,7 +1474,7 @@ rpc_connect_sockaddr_async(struct rpc_context *rpc)
* Enable keepalive to detect and terminate dead connections when server
* TCP stops responding.
*/
if (set_keepalive(rpc->fd) != 0) {
if (set_keepalive(rpc, rpc->fd) != 0) {
rpc_set_error(rpc, "Cannot enable keepalive for fd %d: %s",
rpc->fd, strerror(errno));
return -1;
Expand Down

0 comments on commit ec62485

Please sign in to comment.