Looks good, thanks.
Ethan
On Tue, May 22, 2012 at 5:19 PM, Ben Pfaff <b...@nicira.com> wrote:
> This will be more useful later when we introduces "worker" subprocesses.
> I don't have any current plans to introduce threading, but I can't
> think of a disadvantage to wording this in a general manner.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> NEWS | 4 +++-
> lib/daemon.c | 9 +++------
> lib/util.c | 14 +++++++++++++-
> lib/util.h | 1 +
> lib/vlog.c | 8 ++++++++
> lib/vlog.h | 10 +++++-----
> utilities/ovs-appctl.8.in | 10 ++++++++++
> 7 files changed, 43 insertions(+), 13 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index f8f09e6..5edd87b 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,8 @@
> post-v1.7.0
> ------------------------
> -
> + - New %t and %T log escapes to identify the subprogram within a
> + cooperating group of processes or threads that emitted a log message.
> + The default log patterns now include this information.
>
> v1.7.0 - xx xxx xxxx
> ------------------------
> diff --git a/lib/daemon.c b/lib/daemon.c
> index 6725683..ecca606 100644
> --- a/lib/daemon.c
> +++ b/lib/daemon.c
> @@ -351,13 +351,11 @@ static void
> monitor_daemon(pid_t daemon_pid)
> {
> /* XXX Should log daemon's stderr output at startup time. */
> - const char *saved_program_name;
> time_t last_restart;
> char *status_msg;
> int crashes;
>
> - saved_program_name = program_name;
> - program_name = xasprintf("monitor(%s)", program_name);
> + subprogram_name = "monitor";
> status_msg = xstrdup("healthy");
> last_restart = TIME_MIN;
> crashes = 0;
> @@ -366,7 +364,7 @@ monitor_daemon(pid_t daemon_pid)
> int status;
>
> proctitle_set("%s: monitoring pid %lu (%s)",
> - saved_program_name, (unsigned long int) daemon_pid,
> + program_name, (unsigned long int) daemon_pid,
> status_msg);
>
> do {
> @@ -428,8 +426,7 @@ monitor_daemon(pid_t daemon_pid)
>
> /* Running in new daemon process. */
> proctitle_restore();
> - free((char *) program_name);
> - program_name = saved_program_name;
> + subprogram_name = "";
> }
>
> /* Close standard file descriptors (except any that the client has requested
> we
> diff --git a/lib/util.c b/lib/util.c
> index de3cf3c..bc5fa98 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -34,7 +34,14 @@ VLOG_DEFINE_THIS_MODULE(util);
>
> COVERAGE_DEFINE(util_xalloc);
>
> +/* argv[0] without directory names. */
> const char *program_name;
> +
> +/* Ordinarily "" but set to "monitor" for a monitor process or "worker" for a
> + * worker process. */
> +const char *subprogram_name = "";
> +
> +/* --version option output. */
> static char *program_version;
>
> void
> @@ -243,7 +250,12 @@ ovs_error_valist(int err_no, const char *format, va_list
> args)
> {
> int save_errno = errno;
>
> - fprintf(stderr, "%s: ", program_name);
> + if (subprogram_name[0]) {
> + fprintf(stderr, "%s(%s): ", program_name, subprogram_name);
> + } else {
> + fprintf(stderr, "%s: ", program_name);
> + }
> +
> vfprintf(stderr, format, args);
> if (err_no != 0) {
> fprintf(stderr, " (%s)", ovs_retval_to_string(err_no));
> diff --git a/lib/util.h b/lib/util.h
> index 1bdaeee..dd86de2 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -62,6 +62,7 @@
> #endif
>
> extern const char *program_name;
> +extern const char *subprogram_name;
>
> /* Returns the number of elements in ARRAY. */
> #define ARRAY_SIZE(ARRAY) (sizeof ARRAY / sizeof *ARRAY)
> diff --git a/lib/vlog.c b/lib/vlog.c
> index a7d9e48..4c12007 100644
> --- a/lib/vlog.c
> +++ b/lib/vlog.c
> @@ -646,6 +646,14 @@ format_log_message(const struct vlog_module *module,
> enum vlog_level level,
> case 'r':
> ds_put_format(s, "%lld", time_msec() - time_boot_msec());
> break;
> + case 't':
> + ds_put_cstr(s, subprogram_name[0] ? subprogram_name : "main");
> + break;
> + case 'T':
> + if (subprogram_name[0]) {
> + ds_put_format(s, "(%s)", subprogram_name);
> + }
> + break;
> default:
> ds_put_char(s, p[-1]);
> break;
> diff --git a/lib/vlog.h b/lib/vlog.h
> index 2dce3c6..3b00512 100644
> --- a/lib/vlog.h
> +++ b/lib/vlog.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -49,10 +49,10 @@ const char *vlog_get_level_name(enum vlog_level);
> enum vlog_level vlog_get_level_val(const char *name);
>
> /* Facilities that we can log to. */
> -#define VLOG_FACILITIES \
> - VLOG_FACILITY(SYSLOG, "%05N|%c|%p|%m") \
> - VLOG_FACILITY(CONSOLE, "%D{%Y-%m-%dT%H:%M:%SZ}|%05N|%c|%p|%m") \
> - VLOG_FACILITY(FILE, "%D{%Y-%m-%dT%H:%M:%SZ}|%05N|%c|%p|%m")
> +#define VLOG_FACILITIES \
> + VLOG_FACILITY(SYSLOG, "%05N|%c%T|%p|%m") \
> + VLOG_FACILITY(CONSOLE, "%D{%Y-%m-%dT%H:%M:%SZ}|%05N|%c%T|%p|%m") \
> + VLOG_FACILITY(FILE, "%D{%Y-%m-%dT%H:%M:%SZ}|%05N|%c%T|%p|%m")
> enum vlog_facility {
> #define VLOG_FACILITY(NAME, PATTERN) VLF_##NAME,
> VLOG_FACILITIES
> diff --git a/utilities/ovs-appctl.8.in b/utilities/ovs-appctl.8.in
> index c2ed26b..7edd182 100644
> --- a/utilities/ovs-appctl.8.in
> +++ b/utilities/ovs-appctl.8.in
> @@ -187,6 +187,16 @@ The program's process ID (pid), as a decimal number.
> The number of milliseconds elapsed from the start of the application
> to the time the message was logged.
> .
> +.IP \fB%t\fR
> +The subprogram name, that is, an identifying name for the process or
> +thread that emitted the log message, such as \fBmonitor\fR for the
> +process used for \fB\-\-monitor\fR or \fBmain\fR for the primary
> +process or thread in a program.
> +.
> +.IP \fB%T\fR
> +The subprogram name enclosed in parentheses, e.g. \fB(monitor)\fR, or
> +the empty string for the primary process or thread in a program.
> +.
> .IP \fB%%\fR
> A literal \fB%\fR.
> .RE
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
