[PATCH] Fix bug 13465

[Ralph Böhme]

On Wed, Jul 11, 2018 at 01:36:30PM -0700, Christof Schmitt wrote:
>On Wed, Jul 11, 2018 at 11:46:41AM -0700, Christof Schmitt via samba-technical wrote:
>> On Wed, Jul 11, 2018 at 03:04:27PM +0200, Ralph Böhme via samba-technical wrote:
>> > On Wed, Jul 11, 2018 at 02:35:58PM +0200, Ralph Böhme wrote:
>> > >On Wed, Jul 11, 2018 at 08:24:16AM +0200, Ralph Böhme wrote:
>> > >>Looking...
>> > >
>> > >gna, the problem is starting any tool from the build directory *as
>> > >root* fails if the install directory doesn not exits, eg
>> > >
>> > ># bin/smbd -b | grep LOCKDIR   LOCKDIR: /opt/samba/var/lock
>> > ># mv /opt/samba /opt/samba.bak # bin/smbd -b | grep LOCKDIR Unable
>> > >to initialize messaging context. # exit
>> > >
>> > >But it works as a normal user where a failing messaging init is not treated as a
>> > >fatal error:
>> > >
>> > >$ bin/smbd -b Unable to initialize messaging context.   LOCKDIR:
>> > >/opt/samba/var/lock
>>
>> I see the same behavior here. This is caused by the call to:
>> client_messaging_context -> lp_load_initial_only -> lp_load_ex -> pm_process -> fopen
>> fopen fails because it tried to open /usr/local/samba/etc/smb.conf which
>> does not exist. I assume your build refers to /opt/samba/ instead of
>> /usr/local/samba/
>>
>> > >Guess we should only call client_messaging_context() in
>> > >popt_common_credentials_callback(), but not in
>> > >popt_common_callback(). This ensures all tools that use that use
>> > >POPT_COMMON_SAMBA get an automatic messaging context when needed
>> > >in popt for the lp_load_client() call.
>> > >
>> > >popt_common_callback() will only call client_messaging_context()
>> > >if (another new function) popt_common_set_init_messaging() has
>> > >been called by the tool before calling poptGetContext().
>> > >
>> > >So any tool that doesn't use POPT_COMMON_SAMBA but uses
>> > >lp_load_something > lp_load_initial_only would just call
>> > >popt_common_set_init_messaging() before poptGetContext().
>> >
>> > otoh, some tools like testparm don't need a msg_ctx themselves, it' just that
>> > loadparm implicitly needs it with include=registry.
>> >
>> > So maybe in order to give tools control over this, so they can decide how to
>> > treat a failing client_messaging_context() we should just leave it up to the
>> > tools to call client_messaging_context() and not call it in
>> > popt_common_callback() at all. Only in popt_common_credentials_callback() as
>> > needs it for lp_load_client().
>>
>> Let me see if i understand this:
>>
>> Keeping the call to client_messaging_context in
>> popt_common_credentials_callback would be fine as
>> POPT_COMMON_CREDENTIALS is only used in client tools, not in smbd.
>>
>> The the patter would be to do the popt parsing first, and then call
>> client_messaging_context afterwards?
>>
>> I can look into making that change.
>
>So the problem is that we do not have a valid config file at this point.
>The change in behavior would be that even 'smbd -b' would require a
>valid config file to get past the popt POST callback. Most parts of
>Samba already require a config file, so that should be no change. If
>'smbd -b' is the only part that is affected, we could also just fix
>that (see attached patch).

I was trying to quickly hack something building on-top of the previous
versions. Seems to pass CI on gitlab, can you check it on a cluster?

https://gitlab.com/samba-team/devel/samba/commits/slow-autobuild
https://gitlab.com/samba-team/devel/samba/pipelines/25579752

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46