From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotwebd log levels
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Fri, 6 Dec 2024 15:46:22 +0100

On Fri, Dec 06, 2024 at 03:08:14PM +0100, Omar Polo wrote:
> > -	log_debug("%s: Allocating %d file descriptors",
> > +	log_info("%s: Allocating %d file descriptors",
> >  	    __func__, PRIV_FDS__MAX + GOTWEB_PACK_NUM_TEMPFILES);
> 
> 
> not so sure about this

This is for the case where, for whatever reason, someone sees a problem
with the amount of files opened by the process.

> >  	TAILQ_FOREACH(sock, &gotwebd_env->sockets, entry) {
> > -		log_debug("%s: configuring socket %d (%d)", __func__,
> > +		log_info("%s: configuring socket %d (%d)", __func__,
> >  		    sock->conf.id, sock->fd);
> 
> why it's useful to show this at info level and not debug?

Indeed, this is probably not needed since it is redundant with
the message logged just below:

> > @@ -210,7 +210,7 @@ sockets_launch(void)
> >  
> >  		evtimer_set(&sock->pause, sockets_accept_paused, sock);
> >  
> > -		log_debug("%s: running socket listener %d", __func__,
> > +		log_info("%s: running socket listener %d", __func__,
> >  		    sock->conf.id);
> >  	}
> >  

> > @@ -603,7 +599,7 @@ sockets_rlimit(int maxfd)
> >  
> >  	if (getrlimit(RLIMIT_NOFILE, &rl) == -1)
> >  		fatal("%s: failed to get resource limit", __func__);
> > -	log_debug("%s: max open files %llu", __func__,
> > +	log_info("%s: max open files %llu", __func__,
> >  	    (unsigned long long)rl.rlim_max);
> 
> same here

This again is useful to debug problems related to file descriptor limits
configured on the system.

Regarding socket listener setup, how about this diff

diff /home/stsp/src/got
path + /home/stsp/src/got
commit - 569d4e71e7656ad23a5d3ac728981a46950f103a
blob - bf08e907f6aa6059cac714973e9dd139009154dc
file + gotwebd/sockets.c
--- gotwebd/sockets.c
+++ gotwebd/sockets.c
@@ -199,14 +199,12 @@ sockets_launch(void)
 	const struct got_error *error;
 
 	TAILQ_FOREACH(sock, &gotwebd_env->sockets, entry) {
-		log_info("%s: configuring socket %d (%d)", __func__,
-		    sock->conf.id, sock->fd);
-
 		event_set(&sock->ev, sock->fd, EV_READ | EV_PERSIST,
 		    sockets_socket_accept, sock);
 
-		if (event_add(&sock->ev, NULL))
-			fatalx("event add sock");
+		if (event_add(&sock->ev, NULL) == -1)
+			fatal("event_add sock id %d, fd %d",
+			    sock->conf.id, sock->fd);
 
 		evtimer_set(&sock->pause, sockets_accept_paused, sock);