It has been observed that it's not possible to enable the session-level
verbose dump with -v switch of cldcli anymore (post XDR and ncld).
The main issue is that it is impossible to set the session verbosity by
an application control. Secondary issue is that if application attempts
to change the verbosity level, it cannot do so for newly-created sessions,
because the session structure is allocated when session is created.
Contributing to the confusion is the desire not to enable excessive
verbosity in the CLD sessions whenever debugging is requested.
To fix these issues, the patch does the following:
- Drops all commented-out inserts that assign additional verbosity
from libraries. Permits top-level applications to specify verbosity
(what cldcli -v was intended to do).
- Splits per-packet verbosity away from debugging messages.
- Establishes -D 2 to mean "CLD protocol verbosity" across applications
and the daemon (this replaces the "old" -v). The existing -D 1 now
controls the program debugging only, and there is no bit mask.
Coincidentially this fixes the crash whenever there's a resolution
failure for an SRV record.
Signed-Off-By: Pete Zaitcev <zaitcev@xxxxxxxxxx>
---
include/cldc.h | 1
include/hail_log.h | 24 +++++----
include/ncld.h | 2
lib/cldc.c | 104 ++++++++++++++++++++++++-----------------
pkg/cld.sysconf | 2
server/server.c | 19 ++++---
test/basic-io.c | 4 -
test/basic-session.c | 2
test/lock-file.c | 2
tools/cldcli.c | 17 +++---
tools/cldfuse.c | 2
11 files changed, 104 insertions(+), 75 deletions(-)
diff --git a/include/cldc.h b/include/cldc.h
index b3e4f07..12acd32 100644
--- a/include/cldc.h
+++ b/include/cldc.h
@@ -97,7 +97,6 @@ struct cldc_ops {
const void *buf, size_t buflen);
void (*event)(void *private, struct cldc_session *,
struct cldc_fh *, uint32_t);
- void (*errlog)(int prio, const char *fmt, ...);
};
/** a single CLD client session */
diff --git a/include/hail_log.h b/include/hail_log.h
index 88074a3..53a6780 100644
--- a/include/hail_log.h
+++ b/include/hail_log.h
@@ -10,31 +10,37 @@
#endif
struct hail_log {
- void (*func)(int prio, const char *fmt, ...)
- ATTR_PRINTF(2,3);
- bool verbose;
+ void (*func)(int prio, const char *fmt, ...) ATTR_PRINTF(2,3);
+ bool debug; /* unmutes HAIL_DEBUG */
+ bool verbose; /* enables CLD session verbosity */
};
-/** Print out a debug message if 'verbose' is enabled */
-#define HAIL_DEBUG(log, ...) \
+/** Print out a CLD session debug message if enabled */
+#define HAIL_VERBOSE(log, ...) \
if ((log)->verbose) { \
(log)->func(LOG_DEBUG, __VA_ARGS__); \
}
+/** Print out an application debug message if enabled */
+#define HAIL_DEBUG(log, ...) \
+ if ((log)->debug) { \
+ (log)->func(LOG_DEBUG, __VA_ARGS__); \
+ }
+
/** Print out an informational log message */
#define HAIL_INFO(log, ...) \
- (log)->func(LOG_INFO, __VA_ARGS__);
+ (log)->func(LOG_INFO, __VA_ARGS__)
/** Print out a warning message */
#define HAIL_WARN(log, ...) \
- (log)->func(LOG_WARNING, __VA_ARGS__);
+ (log)->func(LOG_WARNING, __VA_ARGS__)
/** Print out an error message */
#define HAIL_ERR(log, ...) \
- (log)->func(LOG_ERR, __VA_ARGS__);
+ (log)->func(LOG_ERR, __VA_ARGS__)
/** Print out a critical warning message */
#define HAIL_CRIT(log, ...) \
- (log)->func(LOG_CRIT, __VA_ARGS__);
+ (log)->func(LOG_CRIT, __VA_ARGS__)
#endif /* __HAIL_LOG_H__ */
diff --git a/include/ncld.h b/include/ncld.h
index dbf126a..05f57a2 100644
--- a/include/ncld.h
+++ b/include/ncld.h
@@ -72,7 +72,7 @@ struct ncld_read {
extern struct ncld_sess *ncld_sess_open(const char *host, int port,
int *error, void (*event)(void *, unsigned int), void *ev_arg,
- const char *cld_user, const char *cld_key);
+ const char *cld_user, const char *cld_key, struct hail_log *log);
extern struct ncld_fh *ncld_open(struct ncld_sess *s, const char *fname,
unsigned int mode, int *error, unsigned int events,
void (*event)(void *, unsigned int), void *ev_arg);
diff --git a/lib/cldc.c b/lib/cldc.c
index afe88ab..c7419e1 100644
--- a/lib/cldc.c
+++ b/lib/cldc.c
@@ -183,29 +183,26 @@ static int rxmsg_generic(struct cldc_session *sess,
while (tmp) {
req = tmp->data;
- HAIL_DEBUG(&sess->log, "%s: comparing req->xid (%llu) "
- "with resp.xid_in (%llu)",
- __func__,
- (unsigned long long) req->xid,
- (unsigned long long) resp.xid_in);
+ HAIL_VERBOSE(&sess->log, "%s: comparing req->xid (%llu) "
+ "with resp.xid_in (%llu)", __func__,
+ (unsigned long long) req->xid,
+ (unsigned long long) resp.xid_in);
if (req->xid == resp.xid_in)
break;
tmp = tmp->next;
}
if (!tmp) {
- HAIL_DEBUG(&sess->log, "%s: no match found with "
- "xid_in %llu",
- __func__,
- (unsigned long long) resp.xid_in);
+ HAIL_VERBOSE(&sess->log, "%s: no match found with xid_in %llu",
+ __func__, (unsigned long long) resp.xid_in);
return -1005;
}
if (req->done) {
- HAIL_DEBUG(&sess->log, "%s: re-acking", __func__);
+ HAIL_VERBOSE(&sess->log, "%s: re-acking", __func__);
} else {
- HAIL_DEBUG(&sess->log, "%s: issuing completion, acking",
- __func__);
+ HAIL_VERBOSE(&sess->log, "%s: issuing completion, acking",
+ __func__);
req->done = true;
@@ -264,9 +261,9 @@ static int rxmsg_ack_frag(struct cldc_session *sess,
if (seqid != ack_msg.seqid)
continue;
- HAIL_DEBUG(&sess->log, "%s: seqid %llu, expiring",
- __func__,
- (unsigned long long) ack_msg.seqid);
+ HAIL_VERBOSE(&sess->log, "%s: seqid %llu, expiring",
+ __func__,
+ (unsigned long long) ack_msg.seqid);
req->pkt_info[i] = NULL;
free(pi);
@@ -287,8 +284,8 @@ static int rxmsg_event(struct cldc_session *sess,
xdrmem_create(&xdrs, sess->msg_buf, sess->msg_buf_len, XDR_DECODE);
if (!xdr_cld_msg_event(&xdrs, &ev)) {
- HAIL_INFO(&sess->log, "%s: failed to decode cld_msg_event",
- __func__);
+ HAIL_DEBUG(&sess->log, "%s: failed to decode cld_msg_event",
+ __func__);
xdr_destroy(&xdrs);
return -1008;
}
@@ -397,8 +394,8 @@ static int accept_seqid(struct cldc_session *sess, uint64_t seqid,
sess->next_seqid_in = seqid + 1;
sess->next_seqid_in_tr =
sess->next_seqid_in - CLDC_MSG_REMEMBER;
- HAIL_DEBUG(&sess->log, "%s: setting next_seqid_in to %llu",
- __func__, (unsigned long long) seqid);
+ HAIL_VERBOSE(&sess->log, "%s: setting next_seqid_in to %llu",
+ __func__, (unsigned long long) seqid);
return 0;
case CMO_NOT_MASTER:
@@ -513,9 +510,8 @@ int cldc_receive_pkt(struct cldc_session *sess,
sess->expire_time = current_time + CLDC_SESS_EXPIRE;
if (pkt.mi.order & CLD_PKT_IS_LAST) {
- HAIL_DEBUG(&sess->log, "%s: receiving complete message of "
- "op %s", __func__,
- __cld_opstr(sess->msg_buf_op));
+ HAIL_VERBOSE(&sess->log, "%s: receiving complete message of "
+ "op %s", __func__, __cld_opstr(sess->msg_buf_op));
return rx_complete(sess, &pkt, foot);
} else {
return ack_seqid(sess, foot->seqid);
@@ -821,12 +817,12 @@ static ssize_t new_sess_cb(struct cldc_msg *msg, const void *resp_p,
return 0;
}
-int cldc_new_sess(const struct cldc_ops *ops,
- const struct cldc_call_opts *copts,
- const void *addr, size_t addr_len,
- const char *user, const char *secret_key,
- void *private,
- struct cldc_session **sess_out)
+static int cldc_new_sess_log(const struct cldc_ops *ops,
+ const struct cldc_call_opts *copts,
+ const void *addr, size_t addr_len,
+ const char *user, const char *secret_key,
+ void *private, struct hail_log *log,
+ struct cldc_session **sess_out)
{
struct cldc_session *sess;
struct cldc_msg *msg;
@@ -845,13 +841,9 @@ int cldc_new_sess(const struct cldc_ops *ops,
if (!sess)
return -ENOMEM;
-#if 0
- sess->log.verbose = true;
-#endif
-
sess->private = private;
sess->ops = ops;
- sess->log.func = ops->errlog ? ops->errlog : cldc_errlog;
+ sess->log = *log; /* save off caller's stack */
sess->fh = g_array_sized_new(FALSE, TRUE, sizeof(struct cldc_fh), 16);
strcpy(sess->user, user);
strcpy(sess->secret_key, secret_key);
@@ -886,6 +878,22 @@ int cldc_new_sess(const struct cldc_ops *ops,
return sess_send(sess, msg);
}
+int cldc_new_sess(const struct cldc_ops *ops,
+ const struct cldc_call_opts *copts,
+ const void *addr, size_t addr_len,
+ const char *user, const char *secret_key,
+ void *private,
+ struct cldc_session **sess_out)
+{
+ struct hail_log log;
+
+ log.debug = false;
+ log.verbose = false;
+ log.func = cldc_errlog;
+ return cldc_new_sess_log(ops, copts, addr, addr_len, user, secret_key,
+ private, &log, sess_out);
+}
+
/*
* Force-clean the slate in the library. This may leave the server confused.
*/
@@ -1296,7 +1304,8 @@ char *cldc_dirent_name(struct cld_dirent_cur *dc)
/*
* On error, return the code (not negated code like a kernel function would).
*/
-static int ncld_getsrv(char **hostp, unsigned short *portp)
+static int ncld_getsrv(char **hostp, unsigned short *portp,
+ struct hail_log *log)
{
enum { hostsz = 64 };
char hostb[hostsz];
@@ -1308,7 +1317,7 @@ static int ncld_getsrv(char **hostp, unsigned short *portp)
return errno;
hostb[hostsz-1] = 0;
- if (cldc_getaddr(&host_list, hostb, NULL))
+ if (cldc_getaddr(&host_list, hostb, log))
return 1001;
/*
@@ -1501,7 +1510,6 @@ static struct cldc_ops ncld_ops = {
.timer_ctl = ncld_p_timer_ctl,
.pkt_send = ncld_p_pkt_send,
.event = ncld_p_event,
- .errlog = NULL,
};
static int ncld_new_sess(struct cldc_call_opts *copts, enum cle_err_codes errc)
@@ -1547,18 +1555,28 @@ static int ncld_wait_session(struct ncld_sess *nsess)
* @param ev_arg User-supplied argument to the session event function
* @param cld_user The user identifier to be used to authentication
* @param cld_key The user key to be used to authentication
+ * @param log The application log descriptor (ok to be NULL)
*/
struct ncld_sess *ncld_sess_open(const char *host, int port, int *error,
void (*ev_func)(void *, unsigned int),
- void *ev_arg, const char *cld_user,
- const char *cld_key)
+ void *ev_arg,
+ const char *cld_user, const char *cld_key,
+ struct hail_log *log)
{
struct ncld_sess *nsess;
+ struct hail_log nlog;
struct cldc_call_opts copts;
int err;
GError *gerr;
int rc;
+ if (!log) {
+ nlog.debug = false;
+ nlog.verbose = false;
+ nlog.func = cldc_errlog;
+ log = &nlog;
+ }
+
err = ENOMEM;
nsess = malloc(sizeof(struct ncld_sess));
if (!nsess)
@@ -1574,7 +1592,7 @@ struct ncld_sess *ncld_sess_open(const char *host, int port, int *error,
goto out_cond;
if (!host) {
- err = ncld_getsrv(&nsess->host, &nsess->port);
+ err = ncld_getsrv(&nsess->host, &nsess->port, log);
if (err)
goto out_srv;
} else {
@@ -1605,14 +1623,14 @@ struct ncld_sess *ncld_sess_open(const char *host, int port, int *error,
memset(&copts, 0, sizeof(copts));
copts.cb = ncld_new_sess;
copts.private = nsess;
- if (cldc_new_sess(&ncld_ops, &copts, nsess->udp->addr, nsess->udp->addr_len,
- cld_user, cld_key, nsess, &nsess->udp->sess)) {
+ if (cldc_new_sess_log(&ncld_ops, &copts,
+ nsess->udp->addr, nsess->udp->addr_len,
+ cld_user, cld_key, nsess, log,
+ &nsess->udp->sess)) {
err = 1024;
goto out_session;
}
- /* nsess->udp->sess->log.verbose = 1; */
-
rc = ncld_wait_session(nsess);
if (rc) {
err = 1100 + rc % 1000;
diff --git a/pkg/cld.sysconf b/pkg/cld.sysconf
index 2cd45c5..9f17230 100644
--- a/pkg/cld.sysconf
+++ b/pkg/cld.sysconf
@@ -2,5 +2,5 @@
#
#example:
-#OPTS="--debug"
+#OPTS="--debug 1"
diff --git a/server/server.c b/server/server.c
index 3208e0f..6e02b9a 100644
--- a/server/server.c
+++ b/server/server.c
@@ -117,7 +117,6 @@ static void applog(int prio, const char *fmt, ...)
struct hail_log srv_log = {
.func = applog,
- .verbose = 0,
};
int udp_tx(int sock_fd, struct sockaddr *addr, socklen_t addr_len,
@@ -913,17 +912,22 @@ static void stats_dump(void)
static error_t parse_opt (int key, char *arg, struct argp_state *state)
{
+ int v;
+
switch(key) {
case 'd':
cld_srv.data_dir = arg;
break;
case 'D':
- if (atoi(arg) >= 0 && atoi(arg) <= 2)
- srv_log.verbose = atoi(arg);
- else {
+ v = atoi(arg);
+ if (v < 0 || v > 2) {
fprintf(stderr, "invalid debug level: '%s'\n", arg);
argp_usage(state);
}
+ if (v >= 1)
+ srv_log.debug = true;
+ if (v >= 2)
+ srv_log.verbose = true;
break;
case 'E':
use_syslog = false;
@@ -1120,9 +1124,10 @@ int main (int argc, char *argv[])
if (rc)
goto err_out_pid;
- HAIL_INFO(&srv_log, "initialized: verbose %u%s",
- srv_log.verbose,
- strict_free ? ", strict-free" : "");
+ HAIL_INFO(&srv_log, "initialized: %s%s%s",
+ srv_log.debug ? "debug" : "nodebug",
+ srv_log.verbose ? ", verbose" : "",
+ strict_free ? ", strict-free" : "");
/*
* execute main loop
diff --git a/test/basic-io.c b/test/basic-io.c
index 31bfcab..1741590 100644
--- a/test/basic-io.c
+++ b/test/basic-io.c
@@ -40,7 +40,7 @@ static int test_write(int port)
int error;
nsess = ncld_sess_open(TEST_HOST, port, &error, NULL, NULL,
- TEST_USER, TEST_USER_KEY);
+ TEST_USER, TEST_USER_KEY, NULL);
if (!nsess) {
fprintf(stderr, "ncld_sess_open(host %s port %u) failed: %d\n",
TEST_HOST, port, error);
@@ -74,7 +74,7 @@ static int test_read(int port)
int error;
nsess = ncld_sess_open(TEST_HOST, port, &error, NULL, NULL,
- TEST_USER, TEST_USER_KEY);
+ TEST_USER, TEST_USER_KEY, NULL);
if (!nsess) {
fprintf(stderr, "ncld_sess_open(host %s port %u) failed: %d\n",
TEST_HOST, port, error);
diff --git a/test/basic-session.c b/test/basic-session.c
index 6caf046..c3e2a98 100644
--- a/test/basic-session.c
+++ b/test/basic-session.c
@@ -45,7 +45,7 @@ int main (int argc, char *argv[])
return -1;
nsess = ncld_sess_open(TEST_HOST, port, &error, NULL, NULL,
- TEST_USER, TEST_USER_KEY);
+ TEST_USER, TEST_USER_KEY, NULL);
if (!nsess) {
fprintf(stderr, "ncld_sess_open(host %s port %u) failed: %d\n",
TEST_HOST, port, error);
diff --git a/test/lock-file.c b/test/lock-file.c
index d0ea9c1..36cc62c 100644
--- a/test/lock-file.c
+++ b/test/lock-file.c
@@ -50,7 +50,7 @@ int main (int argc, char *argv[])
return -1;
nsess = ncld_sess_open(TEST_HOST, port, &error, NULL, NULL,
- TEST_USER, TEST_USER_KEY);
+ TEST_USER, TEST_USER_KEY, NULL);
if (!nsess) {
fprintf(stderr, "ncld_sess_open(host %s port %u) failed: %d\n",
TEST_HOST, port, error);
diff --git a/tools/cldcli.c b/tools/cldcli.c
index 2a2c82b..7c73091 100644
--- a/tools/cldcli.c
+++ b/tools/cldcli.c
@@ -98,7 +98,6 @@ static void applog(int prio, const char *fmt, ...)
static struct hail_log cli_log = {
.func = applog,
- .verbose = 0,
};
static void sess_event(void *private, uint32_t what)
@@ -628,15 +627,20 @@ err:
static error_t parse_opt (int key, char *arg, struct argp_state *state)
{
+ int v;
+
switch(key) {
case 'D':
- if (atoi(arg) >= 0 && atoi(arg) <= 2)
- cli_log.verbose = atoi(arg);
- else {
+ v = atoi(arg);
+ if (v < 0 || v > 2) {
fprintf(stderr, TAG ": invalid debug level: '%s'\n",
arg);
argp_usage(state);
}
+ if (v >= 1)
+ cli_log.debug = true;
+ if (v >= 2)
+ cli_log.verbose = true;
break;
case 'h':
if (!push_host(arg))
@@ -649,9 +653,6 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state)
} else
strcpy(our_user, arg);
break;
- case 'v':
- cli_log.verbose = true;
- break;
case ARGP_KEY_ARG:
argp_usage(state); /* too many args */
break;
@@ -711,7 +712,7 @@ int main (int argc, char *argv[])
dr = host_list->data;
nsess = ncld_sess_open(dr->host, dr->port, &error, sess_event, NULL,
- "cldcli", "cldcli");
+ "cldcli", "cldcli", &cli_log);
if (!nsess) {
if (error < 1000) {
fprintf(stderr, TAG ": cannot open CLD session: %s\n",
diff --git a/tools/cldfuse.c b/tools/cldfuse.c
index f40ad75..2cba917 100644
--- a/tools/cldfuse.c
+++ b/tools/cldfuse.c
@@ -375,7 +375,7 @@ int main(int argc, char *argv[])
dr = param.host_list->data;
sess = ncld_sess_open(dr->host, dr->port, &error, sess_event, NULL,
- "cldfuse", "cldfuse");
+ "cldfuse", "cldfuse", &cldfuse_log);
if (!sess) {
if (error < 1000) {
fprintf(stderr, TAG ": cannot open CLD session: %s\n",
--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
