CLD: End-to-end verbosity v3

PATCH Applied Pete Zaitcev 2 participants 3 messages Apr 13, 2010 – Apr 14, 2010 Library code

Share & tools

Pete Zaitcev

author

2010-04-13 20:30 UTC #0 lore raw

PATCH 1/3 CLD: End-to-end verbosity v3

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 <[email protected]>

---
 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(-)

include/cldc.h +0 -1

index b3e4f07..12acd32 100644
@@ -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 */

include/hail_log.h +15 -9

index 88074a3..53a6780 100644
@@ -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__ */

include/ncld.h +1 -1

index dbf126a..05f57a2 100644
@@ -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);

lib/cldc.c +61 ~~-43~~

215 lines not on this page · raw message

pkg/cld.sysconf +1 -1

index 2cd45c5..9f17230 100644
@@ -2,5 +2,5 @@
 #
 
 #example: 
-#OPTS="--debug"
+#OPTS="--debug 1"

server/server.c +12 -7

index 3208e0f..6e02b9a 100644
@@ -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

test/basic-io.c +2 -2

index 31bfcab..1741590 100644
@@ -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);

test/basic-session.c +1 -1

index 6caf046..c3e2a98 100644
@@ -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);

test/lock-file.c +1 -1

index d0ea9c1..36cc62c 100644
@@ -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);

tools/cldcli.c +9 -8

index 2a2c82b..7c73091 100644
@@ -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",

tools/cldfuse.c +1 -1

index f40ad75..2cba917 100644
@@ -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",

Jeff Garzik

Applied

2010-04-14 04:01 UTC #1 lore raw

Quoted 28 lines · Pete Zaitcev

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<[email protected]>

Quoted 13 lines

---
  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(-)

applied, thanks for the revisions

Long term, I think the per-packet session logging macro should be made more clear, perhaps calling it HAIL_SESS or even HAIL_PKT

Jeff

Pete Zaitcev

author

2010-04-14 16:41 UTC #2 lore raw

Quoted 2 lines · On Wed, 14 Apr 2010 00:01:23 -0400 Jeff Garzik <[email protected]> wrote:

Long term, I think the per-packet session logging macro should be made more clear, perhaps calling it HAIL_SESS or even HAIL_PKT

I saved this. Then, when you have a better idea already... SUDDENLY, A PATCH! :-)

-- Pete