[PATCH] AD DC auto added vfs modules not present if "vfs objects" is explicitly configured

[Andrew Bartlett]

David,

Thanks for looking into this.  Indeed we do need to warn our users
clearly about this. 

On Fri, 2014-05-02 at 12:55 +0200, David Disseldorp wrote:
> Hi Klaus,
> 
> Thanks for your input...
> 
> On Thu, 01 May 2014 00:00:16 +0200, Klaus Hartnegg wrote:
> 
> > On 30.04.2014 11:35, David Disseldorp wrote:
> > > In fixing this, I'm evaluating the following options:
> > > 1. Print a loud warning in the log (and possibly fail to start) when
> > >     an AD DC configured server includes an explicit "vfs objects"
> > >     definition that does not include the modules that it requires
> > >     (dfs_samba4, acl_xattr).
> > > 2. Append the required vfs modules to any explicit "vfs objects"
> > >     definition, filtering out duplicates.
> > 
> > (1) would replace the problem with another one.
> > (2) would fix it.
> 
> VFS modules often have strict ordering requirements, and may not play
> well with others. So I don't think (2) is a plausible fix.
> 
> > How about (3): the provision script should add that line to smb.conf, 
> > samba should warn if the required vfs objects are not in the list any 
> > more, and then it should behave as if they were.
> 
> This sounds reasonable, but I'm not at all familiar with the existing
> provision script code. Is it currently responsible for writing out a
> fresh AD DC smb.conf? Are there situations where it modifies an existing
> config in-place? If the latter is true, then we'd still be susceptible
> to the issues faced with (2).

We only write out a fresh one.  We will soon need to start dealing with
a similar issue - you might recall the various complaints from users
about using the AD DC on ZFS.  This is essentially the same thing, once
they get past the hard-coded check for working posix ACLs in the script.
That is, we need to load the zfsacl module in the same way btrfs is
desired here. 

> I've attached a patch that adds the warning proposed with (1)/(3).
> Feedback / testing would be much appreciated.

The only issue I see is that your patch would handle the case where a
global vfs objects is defined, but would not catch the case where it is
defined in a share. 

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba