[PATCHES] Fix smbd open race and render defer_open test less flakey

[Jeremy Allison]

On Wed, Sep 10, 2014 at 01:57:44PM +0200, Michael Adam wrote:
> Hi,
> 
> The defer_open test is currently one of the most frequent
> causes of autobuild failure.
> 
> I have just brought a few patches upstream (reviewed by Metze)
> that correct the defer_open test to reliably produce
> toreture-compatible "failure" instead of  "error"
> if something goes wrong.
> (These reason for this was in the missing tracking of
> child process status in the test wrapper not in the
> test itself, so this might also have positive effects
> on a few other tests.)
> 
> Now these changes gave me the opportunity to finally
> start analyzing what is going on.
> There are two causes of failure which I frequently saw:
> 
> 1. return code OBJECT_NAME_NOT FOUND  for the create call.
> 2. reply with SHARING_VIOLATION took too long
> 
> The first is due to a race in open_file() which I
> fixed in the other attached patch: The file vanished
> between check for existence and the acl check.
> ==> review/push/comments appreciated.
> 

Great catch Michael ! The only change I'd make
is in this hunk:

+                               if (!NT_STATUS_IS_OK(status)) {
+                                       DEBUG(10, ("open_file: "
+                                               "file %s vanished since we "
+                                               "checked for existence.\n",
+                                               smb_fname_str_dbg(smb_fname)));
+                                       file_existed = false;
+                                       SET_STAT_INVALID(fsp->fsp_name->st);
+                               }

to make it clear by using:

+                               if (NT_STATUS_EQUAL(NT_STATUS_OBJECT_NAME_NOT_FOUND, status)) {
+                                       DEBUG(10, ("open_file: "
+                                               "file %s vanished since we "
+                                               "checked for existence.\n",
+                                               smb_fname_str_dbg(smb_fname)));
+                                       file_existed = false;
+                                       SET_STAT_INVALID(fsp->fsp_name->st);
+                               }

But that's not a blocker - we can add a tidyup
patch on top afterwards if you agree.

Both changes LGTM - pushing !

FYI. We need a bug for the race condition fix, I think
it's in 4.1.x (but not 4.0.x - I'll check).

Cheers,

	Jeremy.