[PATCH] SYSVOL ACL fixes Re: [PATCH] Fix 'samba-tool ntacl sysvolcheck' failures and remove NT4 compat

[Jeremy Allison]

On Tue, Nov 13, 2012 at 05:00:01PM +1100, Andrew Bartlett wrote:
> 
> The ACL patches here, on master, appear to be the key changes required
> to have GPOs work.  At least, they work for me with a Windows 7 client
> setting and applying GPOs.  (The patches already posted are unchanged
> from the previous mail).
> 
> If I could please have *everyone* who is having trouble with sysvol ACLs
> and is willing to run master try these patches.  You will have to run
> 'samba-tool ntacl sysvolreset' to get the correct ACLs.  
> 
> They are also in my gpo-acl-fix branch at
> git://git.samba.org/abartlet/samba.git
> 
> There are fixes for both the ntvfs and smbd file servers.  The tests
> included with them show that we now correctly store the GPO ACLs in both
> cases. 
> 
> If we confirm this indeed fixes ACLs, then we have finally solved a
> major blocker for the 4.0 release.

I'm reviewing these for inclusion in master right now.

However, they're still not broken up into micro-patches that
make them easier to understand.

For example, inside this fix:

--------------------------------------------------------------
commit fd4835fc720d6780c40e845c1fedfad9dbb2bfe9
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Nov 12 16:45:09 2012 +1100

    smbd: Correctly set fsp->is_directory before dealing with ACLs
    
    Without this change, samba-tool ntacl sysvolreset and samba-tool ntacl
    sysvolcheck were unreliable
    
    This does a stat on a real fsp in set_nt_acl_no_snum and uses
    SMB_VFS_GET_NT_ACL() to ensure the called code knows if it is dealing
    with a file or a directory.
    
    Andrew Bartlett
--------------------------------------------------------------

There are at least three logically separate parts. One is the
change to source3/torture/cmd_vfs.c that reads:

-       fsp->is_directory = False;
+       fsp->is_directory = S_ISDIR(smb_fname->st.st_ex_mode);

1). This should be a separate patch, as it doesn't relate in any
way to the changes to get_nt_acl_no_snum()/set_nt_acl_no_snum().

2). The set_nt_acl_no_snum() changes should be a separate patch.

3). The get_nt_acl_no_snum() changes should be a separate patch.

I understand it's easier to just lump what seem to be logically
related changes into one patch, but breaking them up into small
changes make them *much* easier to review, and allows each change
to be understood to be correct on its own.

I'll post a modified change set that includes these things
separated out so you can see what I mean.

Jeremy.