[PATCH] Use setproctitle in winbindd

[Christof Schmitt]

On Tue, Jan 09, 2018 at 08:54:45PM +0100, Ralph Böhme wrote:
> On Wed, Jan 10, 2018 at 08:10:17AM +1300, Andrew Bartlett via samba-technical wrote:
> > On Tue, 2018-01-09 at 12:07 -0700, Christof Schmitt via samba-technical 
> > wrote:
> > > On Tue, Jan 09, 2018 at 11:17:57AM +0100, Ralph Böhme via samba-technical wrote:
> > > > Hi!
> > > > 
> > > > Looking at the process list on a Samba DC with all those accurately named samba
> > > > processes, I got jealous and wanted the same in winbindd as well.
> > > > 
> > > > Patch attached, already reviewed by Andreas, so I'm going to push later if noone
> > > > objects.
> > > > 
> > > > How does it look like? Eg:
> > > > 
> > > > root     31717  0.0  0.9 436076 20028 ?        Ss   Jan06   0:07 ./bin/winbindd -D
> > > > root     31724  0.0  0.7 316200 15152 ?        S    Jan06   0:01  \_ winbindd: domain child [TITAN]
> > > > root     31727  0.0  0.8 436080 18200 ?        S    Jan06   0:00  \_ winbindd: domain child [WDOM2]
> > > > root     31728  0.0  0.9 443576 19112 ?        S    Jan06   0:01  \_ winbindd: idmap child         
> > > > root     31729  0.0  0.4 309044  9004 ?        S    Jan06   0:01  \_ winbindd: domain child [BUILTIN]
> > > > 
> > > > -slow
> > > 
> > > Hi,
> > > 
> > > i think this is a good idea. Sometimes it is important to identify a
> > > domain child process and currently the only method is checking the open
> > > files of the processes for a log file with the domain name.
> > > 
> > > Have you looked how other parts of the code set the title? For smbd,
> > > this is done from reinit_after_fork that then passes the last parameter
> > > to prctl_set_comment which then calls
> > > prctl(PR_SET_NAME).
> > > 
> > > It would be good to have a common path to set the process names.
> > > 
> > > Christof
> > 
> > I agree.  A pattern where we set the prctl(PR_SET_NAME) with a
> > truncated name and the setproctitle with a longer one would be a very
> > good idea, ideally as a function that calls both, eg
> > set_samba_process_titles(short, long). 
> 
> what for? Why calling prctl? My understanding was calling setproctitle is all
> that's needed for fancy ps output across the board. Isn't that what the samba
> binary calls as well?

As Andrew pointed out, argv[0] and the process name (or "comm" in
Linux) are two different names. The situation now for Samba is that
depending on which one is displayed, the naming is inconsistent. See
e.g. this output from a 'make testenv':

$ ps axjf

13506 13566 13484  3698 pts/1    13484 S+    1000   0:00  |               |       \_ samba: root process
13566 13570 13484  3698 pts/1    13484 S+    1000   0:00  |               |       |   \_ samba: tfork waiter process
13570 13571 13571 13571 ?           -1 Ss    1000   0:00  |               |       |   |   \_ /samba/bin/winbindd -D --option=server role check:inhibit=yes --foreground --stdout
13571 13581 13571 13571 ?           -1 S     1000   0:00  |               |       |   |       \_ winbindd: domain child [ADDOMAIN]            .
13571 13585 13571 13571 ?           -1 S     1000   0:00  |               |       |   |       \_ winbindd: idmap child                        .
13571 13586 13571 13571 ?           -1 S     1000   0:00  |               |       |   |       \_ winbindd: domain child [BUILTIN]             .
13566 13578 13484  3698 pts/1    13484 S+    1000   0:00  |               |       |   \_ samba: tfork waiter process
13578 13579 13579 13579 ?           -1 Ss    1000   0:00  |               |       |       \_ /samba/bin/smbd -D --option=server role check:inhibit=yes --foreground --log-stdout
13579 13583 13579 13579 ?           -1 S     1000   0:00  |               |       |           \_ /samba/bin/smbd -D --option=server role check:inhibit=yes --foreground --log-stdout
13579 13584 13579 13579 ?           -1 S     1000   0:00  |               |       |           \_ /samba/bin/smbd -D --option=server role check:inhibit=yes --foreground --log-stdout
13579 13587 13579 13579 ?           -1 S     1000   0:00  |               |       |           \_ /samba/bin/smbd -D --option=server role check:inhibit=yes --foreground --log-stdout

$ ps -ejH

13566 13484  3698 pts/1    00:00:00                 samba
13570 13484  3698 pts/1    00:00:00                   samba
13571 13571 13571 ?        00:00:00                     winbindd
13581 13571 13571 ?        00:00:00                       winbindd
13585 13571 13571 ?        00:00:00                       winbindd
13586 13571 13571 ?        00:00:00                       winbindd
13578 13484  3698 pts/1    00:00:00                   samba
13579 13579 13579 ?        00:00:00                     smbd
13583 13579 13579 ?        00:00:00                       smbd-notifyd
13584 13579 13579 ?        00:00:00                       cleanupd
13587 13579 13579 ?        00:00:00                       lpqd

My proposal would be to have a new function in lib/util/util_process.c that
takes two arguments and tries to set both names. A fallback would be if only
the short name (<= 16 bytes) is provided, that can be used for both cases. Then
this function should be the preferred approach to set the process names. Time
permitting, i will work on a patch.

Christof