info on set_my_unique_id()

[simo]

On Wed, 2011-08-17 at 10:51 +0200, Volker Lendecke wrote:
> On Tue, Aug 16, 2011 at 08:33:02PM +0000, simo wrote:
> > I am trying to understand how important it is to have a unique_id in a
> > new process (for spoolssd/lsassd etc..)
> > 
> > I see that you call set_my_unique_id() in smbd_accpet_connection() right
> > after sys_fork() and before reinit_after_fork()
> > 
> > It looks like an additional unique_id that goes beyond the pid, so I am
> > trying to understand when it should be set.
> 
> The main point of the unique ID is to correctly identify
> dead processes. smbd does validation of the locking.tdb and
> brlock.tdb entries by doing a process_exists on the server
> id associated with an entry. We have had the bug that an
> smbd holding a share mode had died, and the pid was taken
> over by another long-running process before another process
> stepped in and discareded the orphaned entry from
> locking.tdb. Thus this file was not accessible anymore until
> that other long-running process had exited. In our specific
> case, that process belonged to the implementation of the
> clustered file system everything was based on, so
> essentially that file was blocked until the next reboot of
> the node.
> 
> So a unique id is essential for all processes which could
> leave traces of themselves in any kind of database. For
> spoolssd I could imagine putting itself into some kind of
> notify database, for lsassd I don't have an application
> handy right now. But from my point of view it's definitely
> more than just smbd that needs to do this.
> 
> > Should it be always reset when you fork a new process ?
> > In that case, should it be moved to sys_fork() ?
> > Alternatively could it be moved in reinit_after_fork() removing the
> > server_id argument and regenerating the server_id within it ?
> 
> I haven't deeply thought about this for a while, so take the
> following a bit carefully.
> 
> Potentially generating the unique ID could go even one step
> further down, into the messaging layer. The main reason why
> smbd puts its server id into locking.tdb is that it can
> receive messages when a file is closed or an oplock is about
> to be broken. When we start thinking about a single process
> model for smbd3 I think the notion of a server_id is
> conceptually very closely related to the messaging_context
> we have right now. So yes, I'd say that generating a unique
> ID should move down. How far? Not sure. You could argue that
> the unique ID is going closely along with the PID and thus
> put it into sys_fork, but I think this would prevent us from
> doing a single process model, because the unique ID is too
> closely tied to messaging targets.

I will think about this as well, maybe we can find some time to discuss
it on IRC or by phone at some point as it bothers me somewhat.

Another question on the series.
I may have overlooked something so please feel free to educate me if
this is the case.

In some places we use pid_to_procid() in order to get a server_id from a
pid. But we do not lookup the database to fill server_id instead we slap
in my_unique_id and my_vnn, which refer to "this" process which is not
necessarily the one referenced by 'pid'.

I see this used in messaging_send(). I think all uses are safe for now
as where we use the pid we are basically counting on contacting only
local processes so my_vnn (when used) is probably going to match anyway.

But in this case I wonder if it wouldn't be safer to not fill the
unique_id and the vnn fields and leave them blank so that if a function
that really needs the unique_id is passed this 'fake' server_id it can
simply return an error.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>