Re: ntpd: really enable debug messages
On Sat, Jan 23, 2016 at 08:48:22PM +0100, Reyk Floeter wrote:
> On Sat, Jan 23, 2016 at 12:39:19PM -0600, Brent Cook wrote:
> > I'm going with this instead. That way it works like the manual
> > specifies already (-v enables logging debug messages)
> >
>
> Yes, the -v flag is better, but see below.
>
> > cvs server: Diffing .
> > Index: ntpd.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
> > retrieving revision 1.103
> > diff -u -p -r1.103 ntpd.c
> > --- ntpd.c 11 Jan 2016 15:30:56 - 1.103
> > +++ ntpd.c 23 Jan 2016 18:36:52 -
> > @@ -138,7 +138,7 @@ main(int argc, char *argv[])
> > switch (ch) {
> > case 'd':
> > lconf.debug = 1;
> > - log_verbose(1);
> > + log_verbose(2);
>
> This will get overridden by the various log_init() calls later.
>
> You should better set a "verbose" variable in the switch statements
> and call log_verbose() after the various log_init() calls later
> (ntpd's puristic privsep has to call it in various places).
>
> log_init(..)
> log_verbose(verbose)
>
> Splitting log_init() and log_verbose() allowed us to change the
> verbose flag during runtime; for example "relayctl verbose" allows to
> toggle the flag and calls log_verbose() internally. Maybe ntpctl is
> too minimalistic for it, but I would prefer if ntpd could follow the
> same/similar semantics in its main().
>
How's this? It makes the logic look just like relayd. This was a good
exercise, because I found a nice bug while trying to exercise the
log_debug messages. ok?
Index: ntpd.h
===
RCS file: /cvs/src/usr.sbin/ntpd/ntpd.h,v
retrieving revision 1.127
diff -u -p -r1.127 ntpd.h
--- ntpd.h 19 Dec 2015 20:44:35 - 1.127
+++ ntpd.h 24 Jan 2016 19:07:25 -
@@ -213,9 +213,10 @@ struct ntpd_conf {
struct ntp_status status;
struct ntp_freq freq;
u_int32_t scale;
+ int debug;
+ int verbose;
u_int8_tlisten_all;
u_int8_tsettime;
- u_int8_tdebug;
u_int8_tnoaction;
u_int8_tfilters;
time_t constraint_last;
Index: ntpd.c
===
RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
retrieving revision 1.103
diff -u -p -r1.103 ntpd.c
--- ntpd.c 11 Jan 2016 15:30:56 - 1.103
+++ ntpd.c 24 Jan 2016 19:07:25 -
@@ -132,18 +132,16 @@ main(int argc, char *argv[])
memset(, 0, sizeof(lconf));
- log_init(1, LOG_DAEMON);/* log to stderr until daemonized */
-
while ((ch = getopt(argc, argv, "df:nsSv")) != -1) {
switch (ch) {
case 'd':
- lconf.debug = 1;
- log_verbose(1);
+ lconf.debug = 2;
break;
case 'f':
conffile = optarg;
break;
case 'n':
+ lconf.debug = 2;
lconf.noaction = 1;
break;
case 's':
@@ -153,7 +151,7 @@ main(int argc, char *argv[])
lconf.settime = 0;
break;
case 'v':
- log_verbose(1);
+ lconf.verbose++;
break;
default:
usage();
@@ -161,6 +159,9 @@ main(int argc, char *argv[])
}
}
+ /* log to stderr until daemonized */
+ log_init(lconf.debug ? lconf.debug : 1, LOG_DAEMON);
+
argc -= optind;
argv += optind;
if (argc > 0)
@@ -190,6 +191,7 @@ main(int argc, char *argv[])
reset_adjtime();
if (!lconf.settime) {
log_init(lconf.debug, LOG_DAEMON);
+ log_verbose(lconf.verbose);
if (!lconf.debug)
if (daemon(1, 0))
fatal("daemon");
@@ -269,6 +271,7 @@ main(int argc, char *argv[])
lconf.settime = 0;
timeout = INFTIM;
log_init(lconf.debug, LOG_DAEMON);
+ log_verbose(lconf.verbose);
log_warnx("no reply received in time, skipping initial "
"time setting");
Re: ntpd: really enable debug messages
On Sat, Jan 23, 2016 at 12:39:19PM -0600, Brent Cook wrote:
> I'm going with this instead. That way it works like the manual
> specifies already (-v enables logging debug messages)
>
Yes, the -v flag is better, but see below.
> cvs server: Diffing .
> Index: ntpd.c
> ===
> RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
> retrieving revision 1.103
> diff -u -p -r1.103 ntpd.c
> --- ntpd.c 11 Jan 2016 15:30:56 - 1.103
> +++ ntpd.c 23 Jan 2016 18:36:52 -
> @@ -138,7 +138,7 @@ main(int argc, char *argv[])
> switch (ch) {
> case 'd':
> lconf.debug = 1;
> - log_verbose(1);
> + log_verbose(2);
This will get overridden by the various log_init() calls later.
You should better set a "verbose" variable in the switch statements
and call log_verbose() after the various log_init() calls later
(ntpd's puristic privsep has to call it in various places).
log_init(..)
log_verbose(verbose)
Splitting log_init() and log_verbose() allowed us to change the
verbose flag during runtime; for example "relayctl verbose" allows to
toggle the flag and calls log_verbose() internally. Maybe ntpctl is
too minimalistic for it, but I would prefer if ntpd could follow the
same/similar semantics in its main().
Reyk
> break;
> case 'f':
> conffile = optarg;
> @@ -153,7 +153,7 @@ main(int argc, char *argv[])
> lconf.settime = 0;
> break;
> case 'v':
> - log_verbose(1);
> + log_verbose(2);
> break;
> default:
> usage();
>
> On Wed, Jan 20, 2016 at 7:23 AM, Sebastian Benoit wrote:
> > in relayd we use -v for that, so you need to run -d to get lots of
> > output. check main() there?
> >
> > i think thats more intuitive, but maybe i'm just used to it.
> >
> > Brent Cook(bust...@gmail.com) on 2016.01.20 06:31:44 -0600:
> >> Since the relatively recent logging unification, log_init needs a
> >> debug level > 1 in order for log_debug to print anything. This change
> >> makes it so 'ntpd -d' stays in the foreground but doesn't log much
> >> (the current behavior, different than previous releases though), 'ntpd
> >> -dd' actually prints more verbose debug messages.
> >>
> >> Index: ntpd.8
> >> ===
> >> RCS file: /cvs/src/usr.sbin/ntpd/ntpd.8,v
> >> retrieving revision 1.40
> >> diff -u -p -u -p -r1.40 ntpd.8
> >> --- ntpd.8 30 Oct 2015 16:41:53 - 1.40
> >> +++ ntpd.8 20 Jan 2016 12:31:16 -
> >> @@ -50,6 +50,7 @@ If this option is specified,
> >> .Nm
> >> will run in the foreground and log to
> >> .Em stderr .
> >> +It may be specified again to enable more verbose debug logs.
> >> .It Fl f Ar file
> >> Use
> >> .Ar file
> >> Index: ntpd.c
> >> ===
> >> RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
> >> retrieving revision 1.103
> >> diff -u -p -u -p -r1.103 ntpd.c
> >> --- ntpd.c 11 Jan 2016 15:30:56 - 1.103
> >> +++ ntpd.c 20 Jan 2016 12:31:16 -
> >> @@ -137,7 +137,7 @@ main(int argc, char *argv[])
> >> while ((ch = getopt(argc, argv, "df:nsSv")) != -1) {
> >> switch (ch) {
> >> case 'd':
> >> - lconf.debug = 1;
> >> + lconf.debug++;
> >> log_verbose(1);
> >> break;
> >> case 'f':
> >>
> >
> > --
>
--
Re: ntpd: really enable debug messages
I'm going with this instead. That way it works like the manual
specifies already (-v enables logging debug messages)
cvs server: Diffing .
Index: ntpd.c
===
RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
retrieving revision 1.103
diff -u -p -r1.103 ntpd.c
--- ntpd.c 11 Jan 2016 15:30:56 - 1.103
+++ ntpd.c 23 Jan 2016 18:36:52 -
@@ -138,7 +138,7 @@ main(int argc, char *argv[])
switch (ch) {
case 'd':
lconf.debug = 1;
- log_verbose(1);
+ log_verbose(2);
break;
case 'f':
conffile = optarg;
@@ -153,7 +153,7 @@ main(int argc, char *argv[])
lconf.settime = 0;
break;
case 'v':
- log_verbose(1);
+ log_verbose(2);
break;
default:
usage();
On Wed, Jan 20, 2016 at 7:23 AM, Sebastian Benoit wrote:
> in relayd we use -v for that, so you need to run -d to get lots of
> output. check main() there?
>
> i think thats more intuitive, but maybe i'm just used to it.
>
> Brent Cook(bust...@gmail.com) on 2016.01.20 06:31:44 -0600:
>> Since the relatively recent logging unification, log_init needs a
>> debug level > 1 in order for log_debug to print anything. This change
>> makes it so 'ntpd -d' stays in the foreground but doesn't log much
>> (the current behavior, different than previous releases though), 'ntpd
>> -dd' actually prints more verbose debug messages.
>>
>> Index: ntpd.8
>> ===
>> RCS file: /cvs/src/usr.sbin/ntpd/ntpd.8,v
>> retrieving revision 1.40
>> diff -u -p -u -p -r1.40 ntpd.8
>> --- ntpd.8 30 Oct 2015 16:41:53 - 1.40
>> +++ ntpd.8 20 Jan 2016 12:31:16 -
>> @@ -50,6 +50,7 @@ If this option is specified,
>> .Nm
>> will run in the foreground and log to
>> .Em stderr .
>> +It may be specified again to enable more verbose debug logs.
>> .It Fl f Ar file
>> Use
>> .Ar file
>> Index: ntpd.c
>> ===
>> RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
>> retrieving revision 1.103
>> diff -u -p -u -p -r1.103 ntpd.c
>> --- ntpd.c 11 Jan 2016 15:30:56 - 1.103
>> +++ ntpd.c 20 Jan 2016 12:31:16 -
>> @@ -137,7 +137,7 @@ main(int argc, char *argv[])
>> while ((ch = getopt(argc, argv, "df:nsSv")) != -1) {
>> switch (ch) {
>> case 'd':
>> - lconf.debug = 1;
>> + lconf.debug++;
>> log_verbose(1);
>> break;
>> case 'f':
>>
>
> --
ntpd: really enable debug messages
Since the relatively recent logging unification, log_init needs a
debug level > 1 in order for log_debug to print anything. This change
makes it so 'ntpd -d' stays in the foreground but doesn't log much
(the current behavior, different than previous releases though), 'ntpd
-dd' actually prints more verbose debug messages.
Index: ntpd.8
===
RCS file: /cvs/src/usr.sbin/ntpd/ntpd.8,v
retrieving revision 1.40
diff -u -p -u -p -r1.40 ntpd.8
--- ntpd.8 30 Oct 2015 16:41:53 - 1.40
+++ ntpd.8 20 Jan 2016 12:31:16 -
@@ -50,6 +50,7 @@ If this option is specified,
.Nm
will run in the foreground and log to
.Em stderr .
+It may be specified again to enable more verbose debug logs.
.It Fl f Ar file
Use
.Ar file
Index: ntpd.c
===
RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
retrieving revision 1.103
diff -u -p -u -p -r1.103 ntpd.c
--- ntpd.c 11 Jan 2016 15:30:56 - 1.103
+++ ntpd.c 20 Jan 2016 12:31:16 -
@@ -137,7 +137,7 @@ main(int argc, char *argv[])
while ((ch = getopt(argc, argv, "df:nsSv")) != -1) {
switch (ch) {
case 'd':
- lconf.debug = 1;
+ lconf.debug++;
log_verbose(1);
break;
case 'f':
Re: ntpd: really enable debug messages
in relayd we use -v for that, so you need to run -d to get lots of
output. check main() there?
i think thats more intuitive, but maybe i'm just used to it.
Brent Cook(bust...@gmail.com) on 2016.01.20 06:31:44 -0600:
> Since the relatively recent logging unification, log_init needs a
> debug level > 1 in order for log_debug to print anything. This change
> makes it so 'ntpd -d' stays in the foreground but doesn't log much
> (the current behavior, different than previous releases though), 'ntpd
> -dd' actually prints more verbose debug messages.
>
> Index: ntpd.8
> ===
> RCS file: /cvs/src/usr.sbin/ntpd/ntpd.8,v
> retrieving revision 1.40
> diff -u -p -u -p -r1.40 ntpd.8
> --- ntpd.8 30 Oct 2015 16:41:53 - 1.40
> +++ ntpd.8 20 Jan 2016 12:31:16 -
> @@ -50,6 +50,7 @@ If this option is specified,
> .Nm
> will run in the foreground and log to
> .Em stderr .
> +It may be specified again to enable more verbose debug logs.
> .It Fl f Ar file
> Use
> .Ar file
> Index: ntpd.c
> ===
> RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
> retrieving revision 1.103
> diff -u -p -u -p -r1.103 ntpd.c
> --- ntpd.c 11 Jan 2016 15:30:56 - 1.103
> +++ ntpd.c 20 Jan 2016 12:31:16 -
> @@ -137,7 +137,7 @@ main(int argc, char *argv[])
> while ((ch = getopt(argc, argv, "df:nsSv")) != -1) {
> switch (ch) {
> case 'd':
> - lconf.debug = 1;
> + lconf.debug++;
> log_verbose(1);
> break;
> case 'f':
>
--
