On Wed, Aug 17, 2011 at 06:07:21PM +0100, Stuart Henderson wrote:
> On 2011/08/17 17:28, Stuart Henderson wrote:
> > Currently when ospfd is started with "ospfd -v", the verbose logging flag
> > is only set for the parent process, so the detailed messages from rde/ospfe
> > are not logged.
> >
> > The diff below passes it to the other processes so that if you store all
> > ospfd logs you'll get the same information logged with -v as you currently
> > get on-screen with -vd (or if you 'ospfctl log verbose' after startup).
> > I tried doing this by sending an IMSG_CTL_LOG_VERBOSE message but couldn't
> > find the right place/way to do it, so in lieu of another smart idea I went
> > for this simpler method of just adding an argument to the function.
> >
> > I guess ripd, ldpd, ospf6d, bgpd, ldapd and dvmrpd will have a similar
> > problem, if people agree with this (or suggest a better way to do it),
> > I can look at those too.
>
> oops, this is better (slight copy-and-paste fail with |= and & as
> pointed out by blambert, thanks).
>
I don't like it and the diff is wrong -- the parent process will still not
log verbose. I think the problem should be fixed at the source and
change the way log_init() and log_verbose() work. Currently the second
log_init() call in ospfd.c will disable the verbose setting. I guess
log_init needs either 2 flags or it must only or n_debug into verbose so
that the previous call to log_verbose() is sticky.
>
> diff --git ospfd.c ospfd.c
> index bcead83..fe6d6bf 100644
> --- ospfd.c
> +++ ospfd.c
> @@ -244,9 +244,9 @@ main(int argc, char *argv[])
>
> /* start children */
> rde_pid = rde(ospfd_conf, pipe_parent2rde, pipe_ospfe2rde,
> - pipe_parent2ospfe);
> + pipe_parent2ospfe, opts & OSPFD_OPT_VERBOSE);
> ospfe_pid = ospfe(ospfd_conf, pipe_parent2ospfe, pipe_ospfe2rde,
> - pipe_parent2rde);
> + pipe_parent2rde, opts & OSPFD_OPT_VERBOSE);
>
> /* show who we are */
> setproctitle("parent");
> diff --git ospfe.c ospfe.c
> index 81d997c..b91a05d 100644
> --- ospfe.c
> +++ ospfe.c
> @@ -70,7 +70,7 @@ ospfe_sig_handler(int sig, short event, void *bula)
> /* ospf engine */
> pid_t
> ospfe(struct ospfd_conf *xconf, int pipe_parent2ospfe[2], int
> pipe_ospfe2rde[2],
> - int pipe_parent2rde[2])
> + int pipe_parent2rde[2], int verbose)
> {
> struct area *area;
> struct iface *iface;
> @@ -127,6 +127,7 @@ ospfe(struct ospfd_conf *xconf, int pipe_parent2ospfe[2],
> int pipe_ospfe2rde[2],
> fatal("can't drop privileges");
>
> event_init();
> + log_verbose(verbose);
> nbr_init(NBR_HASHSIZE);
> lsa_cache_init(LSA_HASHSIZE);
>
> diff --git ospfe.h ospfe.h
> index 03fdef5..35135a7 100644
> --- ospfe.h
> +++ ospfe.h
> @@ -120,7 +120,7 @@ void recv_hello(struct iface *, struct in_addr,
> u_int32_t,
> char *, u_int16_t);
>
> /* ospfe.c */
> -pid_t ospfe(struct ospfd_conf *, int[2], int[2], int[2]);
> +pid_t ospfe(struct ospfd_conf *, int[2], int[2], int[2],
> int);
> void ospfe_dispatch_main(int, short, void *);
> void ospfe_dispatch_rde(int, short, void *);
> int ospfe_imsg_compose_parent(int, pid_t, void *, u_int16_t);
> diff --git rde.c rde.c
> index 0155e1c..c91837a 100644
> --- rde.c
> +++ rde.c
> @@ -91,7 +91,7 @@ rde_sig_handler(int sig, short event, void *arg)
> /* route decision engine */
> pid_t
> rde(struct ospfd_conf *xconf, int pipe_parent2rde[2], int pipe_ospfe2rde[2],
> - int pipe_parent2ospfe[2])
> + int pipe_parent2ospfe[2], int verbose)
> {
> struct event ev_sigint, ev_sigterm;
> struct timeval now;
> @@ -130,6 +130,7 @@ rde(struct ospfd_conf *xconf, int pipe_parent2rde[2], int
> pipe_ospfe2rde[2],
> fatal("can't drop privileges");
>
> event_init();
> + log_verbose(verbose);
> rde_nbr_init(NBR_HASHSIZE);
> lsa_init(&asext_tree);
>
> diff --git rde.h rde.h
> index 4492f5b..af40407 100644
> --- rde.h
> +++ rde.h
> @@ -111,7 +111,7 @@ struct abr_rtr {
> extern struct lsa_tree asext_tree;
>
> /* rde.c */
> -pid_t rde(struct ospfd_conf *, int [2], int [2], int [2]);
> +pid_t rde(struct ospfd_conf *, int [2], int [2], int [2],
> int);
> int rde_imsg_compose_ospfe(int, u_int32_t, pid_t, void *,
> u_int16_t);
> u_int32_t rde_router_id(void);
>
>
> ...the clowns are safe for a little longer
>
--
:wq Claudio