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.
- Changes the command line option -D of both cld and cldcli to mean
the same thing.
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 | 103 +++++++++++++++++++++++------------------
server/server.c | 22 +++++---
test/basic-io.c | 4 -
test/basic-session.c | 2
test/lock-file.c | 2
tools/cldcli.c | 24 +++++----
9 files changed, 108 insertions(+), 76 deletions(-)
commit 2b74fc8e0aecd79e466cf0ac7e966af65554307e
Author: Master <zaitcev@xxxxxxxxxxxxxxxxxx>
Date: Tue Feb 9 15:03:18 2010 -0700
Verbosity.
diff --git a/include/cldc.h b/include/cldc.h
index 858fbd3..3b75376 100644
--- a/include/cldc.h
+++ b/include/cldc.h
@@ -88,7 +88,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 690c64b..bcdb721 100644
--- a/include/ncld.h
+++ b/include/ncld.h
@@ -70,7 +70,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 7c30064..1c2160f 100644
--- a/lib/cldc.c
+++ b/lib/cldc.c
@@ -172,29 +172,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;
@@ -253,9 +250,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);
@@ -276,8 +273,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;
}
@@ -386,8 +383,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:
@@ -502,9 +499,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);
@@ -810,12 +806,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;
@@ -834,13 +830,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);
@@ -875,6 +867,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.
*/
@@ -1285,7 +1293,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];
@@ -1297,7 +1306,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;
/*
@@ -1490,7 +1499,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)
@@ -1536,18 +1544,28 @@ static int ncld_wait_session(struct ncld_sess *nsp)
* @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 *nsp;
+ 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;
nsp = malloc(sizeof(struct ncld_sess));
if (!nsp)
@@ -1563,7 +1581,7 @@ struct ncld_sess *ncld_sess_open(const char *host, int port, int *error,
goto out_cond;
if (!host) {
- err = ncld_getsrv(&nsp->host, &nsp->port);
+ err = ncld_getsrv(&nsp->host, &nsp->port, log);
if (err)
goto out_srv;
} else {
@@ -1594,14 +1612,13 @@ 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 = nsp;
- if (cldc_new_sess(&ncld_ops, &copts, nsp->udp->addr, nsp->udp->addr_len,
- cld_user, cld_key, nsp, &nsp->udp->sess)) {
+ if (cldc_new_sess_log(&ncld_ops, &copts,
+ nsp->udp->addr, nsp->udp->addr_len,
+ cld_user, cld_key, nsp, log, &nsp->udp->sess)) {
err = 1024;
goto out_session;
}
- /* nsp->udp->sess->log.verbose = 1; */
-
rc = ncld_wait_session(nsp);
if (rc) {
err = 1100 + rc % 1000;
diff --git a/server/server.c b/server/server.c
index 3208e0f..7c1d6a6 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,
@@ -918,9 +917,17 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state)
cld_srv.data_dir = arg;
break;
case 'D':
- if (atoi(arg) >= 0 && atoi(arg) <= 2)
- srv_log.verbose = atoi(arg);
- else {
+ switch (atoi(arg)) {
+ case 0:
+ break;
+ case 1:
+ srv_log.debug = true;
+ break;
+ case 2:
+ srv_log.debug = true;
+ srv_log.verbose = true;
+ break;
+ default:
fprintf(stderr, "invalid debug level: '%s'\n", arg);
argp_usage(state);
}
@@ -1120,9 +1127,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",
+ srv_log.verbose ? "verbose" :
+ (srv_log.debug ? "debug" : "quiet"),
+ strict_free ? ", strict-free" : "");
/*
* execute main loop
diff --git a/test/basic-io.c b/test/basic-io.c
index 2565280..bd41187 100644
--- a/test/basic-io.c
+++ b/test/basic-io.c
@@ -40,7 +40,7 @@ static int test_write(int port)
int error;
nsp = ncld_sess_open(TEST_HOST, port, &error, NULL, NULL,
- TEST_USER, TEST_USER_KEY);
+ TEST_USER, TEST_USER_KEY, NULL);
if (!nsp) {
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;
nsp = ncld_sess_open(TEST_HOST, port, &error, NULL, NULL,
- TEST_USER, TEST_USER_KEY);
+ TEST_USER, TEST_USER_KEY, NULL);
if (!nsp) {
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 d8f9ec1..61efd71 100644
--- a/test/basic-session.c
+++ b/test/basic-session.c
@@ -45,7 +45,7 @@ int main (int argc, char *argv[])
return -1;
nsp = ncld_sess_open(TEST_HOST, port, &error, NULL, NULL,
- TEST_USER, TEST_USER_KEY);
+ TEST_USER, TEST_USER_KEY, NULL);
if (!nsp) {
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 64e6020..8b154e3 100644
--- a/test/lock-file.c
+++ b/test/lock-file.c
@@ -50,7 +50,7 @@ int main (int argc, char *argv[])
return -1;
nsp = ncld_sess_open(TEST_HOST, port, &error, NULL, NULL,
- TEST_USER, TEST_USER_KEY);
+ TEST_USER, TEST_USER_KEY, NULL);
if (!nsp) {
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 85a7ef8..8a4868c 100644
--- a/tools/cldcli.c
+++ b/tools/cldcli.c
@@ -44,8 +44,6 @@ static struct argp_option options[] = {
"Connect to remote CLD at specified HOST:PORT" },
{ "user", 'u', "USER", 0,
"Set username to USER" },
- { "verbose", 'v', NULL, 0,
- "Enable verbose libcldc output" },
{ }
};
@@ -98,7 +96,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)
@@ -630,11 +627,19 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state)
{
switch(key) {
case 'D':
- if (atoi(arg) >= 0 && atoi(arg) <= 2)
- cli_log.verbose = atoi(arg);
- else {
+ switch (atoi(arg)) {
+ case 0:
+ break;
+ case 1:
+ cli_log.debug = true;
+ break;
+ case 2:
+ cli_log.debug = true;
+ cli_log.verbose = true;
+ break;
+ default:
fprintf(stderr, TAG ": invalid debug level: '%s'\n",
- arg);
+ arg);
argp_usage(state);
}
break;
@@ -649,9 +654,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 +713,7 @@ int main (int argc, char *argv[])
dr = host_list->data;
nsp = ncld_sess_open(dr->host, dr->port, &error, sess_event, NULL,
- "cldcli", "cldcli");
+ "cldcli", "cldcli", &cli_log);
if (!nsp) {
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
