Bug 10711 - nmbd fails to accept --piddir option
nmbd fails to accept --piddir option
Status: RESOLVED FIXED
Product: Samba 3.6
Classification: Unclassified
Component: Nmbd
3.6.24
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks: 10077
  Show dependency treegraph
 
Reported: 2014-07-12 00:56 UTC by Andrew Church
Modified: 2014-11-10 20:30 UTC (History)
2 users (show)

See Also:


Attachments
nmbd-dynconfig-fix.diff (477 bytes, patch)
2014-07-12 00:56 UTC, Andrew Church
no flags Details
git-am fix for 4.1.next. (1.49 KB, patch)
2014-07-14 23:14 UTC, Jeremy Allison
metze: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Church 2014-07-12 00:56:39 UTC
Created attachment 10104 [details]
nmbd-dynconfig-fix.diff

nmbd.c is missing POPT_COMMON_DYNCONFIG in its long_options[] list, so it fails to accept options such as --piddir.

Trivial patch attached.
Comment 1 Jeremy Allison 2014-07-14 20:02:52 UTC
Hmmmm. --piddir seems to be missing in all master branch code.

Was this intentional ? More investigation needed..
Comment 2 Jeremy Allison 2014-07-14 20:05:39 UTC
Ah. Here is the culprit:

--------------------------------------------------------
commit e02c94d13bab4fb4ad791b7c0dedd963c33804cb

Author: Andrew Bartlett <abartlet@samba.org>
Date:   Wed Jan 8 13:35:34 2014 +1300

    cmdline: Remove dynconfig hooks in command line processing
    
    This removes the ability to set paths like the sbindir, bindir, and changes the tool for setting lockdir
    statedir etc to be via --option="lock dir=/var/lock".
    
    These were originally added by commit 90a6873b0570f2691ba8d8fd11154c856bdd4415
    by James Peach <jpeach@samba.org>
    
    The important use case, qemu, does not use these options, but specifies these directories via an smb.conf.
    
    They are being removed to remove a layer from the loadparm system, now that options
    can be specified from the command line.  It will also make it easier to generate the affected
    parameters from the XML documentation if this layer of indirection is removed.
    
    Andrew Bartlett
    
    Signed-off-by: Andrew Bartlett <abartlet@samba.org>
    Reviewed-by: Stefan Metzmacher <metze@samba.org>
    
--------------------------------------------------------

So it was intentional, at least for Samba 4.2.x.
Comment 3 Jeremy Allison 2014-07-14 23:14:35 UTC
Created attachment 10108 [details]
git-am fix for 4.1.next.

Can you try the attached ? We won't be adding this fix to 3.6.x as that is in security-fix only mode, but we could tidy this up for 4.1.next and 4.0.next.

Thanks,

Jeremy.
Comment 4 Andrew Church 2014-07-15 00:46:51 UTC
The patch works fine for me, thanks!
Comment 5 Jeremy Allison 2014-07-15 19:28:56 UTC
Comment on attachment 10108 [details]
git-am fix for 4.1.next.

Metze, what do you think for 4.1.next ?
Comment 6 Stefan Metzmacher 2014-07-15 23:20:27 UTC
(In reply to comment #2)
> Ah. Here is the culprit:
> 
> --------------------------------------------------------
> commit e02c94d13bab4fb4ad791b7c0dedd963c33804cb
> 
> Author: Andrew Bartlett <abartlet@samba.org>
> Date:   Wed Jan 8 13:35:34 2014 +1300
> 
>     cmdline: Remove dynconfig hooks in command line processing
> 
>     This removes the ability to set paths like the sbindir, bindir, and changes
> the tool for setting lockdir
>     statedir etc to be via --option="lock dir=/var/lock".
> 
>     These were originally added by commit
> 90a6873b0570f2691ba8d8fd11154c856bdd4415
>     by James Peach <jpeach@samba.org>
> 
>     The important use case, qemu, does not use these options, but specifies
> these directories via an smb.conf.
> 
>     They are being removed to remove a layer from the loadparm system, now that
> options
>     can be specified from the command line.  It will also make it easier to
> generate the affected
>     parameters from the XML documentation if this layer of indirection is
> removed.
> 
>     Andrew Bartlett
> 
>     Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>     Reviewed-by: Stefan Metzmacher <metze@samba.org>
> 
> --------------------------------------------------------
> 
> So it was intentional, at least for Samba 4.2.x.

We need to make sure the manpages are updated for 4.2.0 if not done already...
Comment 7 Jeremy Allison 2014-07-15 23:29:20 UTC
Re-assigning to Karolin for inclusion in 4.1.next, 4.0.next.
Comment 8 Karolin Seeger 2014-10-29 20:02:17 UTC
Pushed to autobuild-v4-[0|1]-test.
Comment 9 Karolin Seeger 2014-11-04 20:06:06 UTC
(In reply to Karolin Seeger from comment #8)
Pushed to v4-0-test, waiting for autobuild-v4-1-test.
Comment 10 Karolin Seeger 2014-11-10 20:30:44 UTC
(In reply to Karolin Seeger from comment #9)
Pushed to v4-1-test.

Closing out bug report.

Thanks!