Patchset to add asynchronous open/close to master

[Jeremy Allison]

On Thu, Jun 14, 2012 at 01:12:02PM +0200, Stefan (metze) Metzmacher wrote:
> 
> I have and it's really tricky code with a lot of possible side effects,
> I wouldn't say it's simple, maybe as simple as SPNEGO or SASL:-)

Ok, maybe it's because I've been thinking about this for some
time :-).

> aio_pthread_connect() sets global variables!
> actually it's really a bug to set global variables from a per share option,
> as a new tree connect changes the behavior of another one.
> 
> Please fix the code and make them global options.
> Or implement the per share option correct.

Yes, you're correct. I'll fix that.

> Why does it call SMB_VFS_NEXT* then, when not doing async stuff,
> I think it should always or never call raw syscalls.
> 
> Otherwise it's confusing, at least add some comments.

The reason it calls the SMB_VFS_NEXT* stuff is otherwise
I have to duplicate the code in all the modules/vfs_default.c
functions I call. I really don't want to do that.

I'll add comments to make that clear.

> I know, but as far as I can see the cancel engine would just call
> smbd_smb2_create_cancel and will return NT_STATUS_CANCELLED to the client
> while the thread still hangs in the syscall.
> 
> I think we should not return NT_STATUS_CANCELLED in that case.

Ok, I think that's fixable by searching for an outstanding
open state struct and ignoring the cancel in this case.

> Sure, but reading SMB_VFS_NEXT* calls also in the new code is confusing!
> Please avoid it or at least add comments.

I will add comments to make the clearer.

> You said you want this in 3.6 and for 3.6 this is an incompatible change!
> For master it's fine to add it.
> 
> The only option that might work for 3.6 is to add it to the end of the
> structure,
> but I still won't be very happy with changing it.

This patchset is for master. For the 3.6.x version I can
move it to the end. But I do think we need to talk about
the difference between keeping a stable API for the VFS,
which we do (all modules should be able to be recompiled
without change) and a stable ABI for the VFS (binary
modules for a previous version work).

I think we promise a stable API, but not a stable ABI.

> The reason for beeing to critical is that you want this in 3.6:-)

Oh, that's very sweet :-). Thanks Metze - I really appreciate
that. Actually this patchset was for master only, I was planning
to cross the 3.6.x bridge once it was in master :-).

Cheers and thanks for your very helpful review !

Jeremy.