Re: smb2 work status

[Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Mon, 18 Jul 2011 11:47:30 -0400
Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:

> On Mon, Jul 18, 2011 at 07:45:01PM +0400, Pavel Shilovsky wrote:
> > If I understand you right, you mean to separate inode and file
> > operations for smb2 into a different structures, set them once with
> > ifdefs in cifs_set_ops() and keep them in smb2inode.c and smb2file.c
> > files.
> 
> That's the idea.  You'll really have to prototype it to check if it
> works out nicely.  If we end up duplicating too much it might not
> be feasible, but fro ma quick look it should be much better.
> 

Agreed. Once you mount, you know what protocol version you're using.
There's little value in continually checking that.

Another idea to consider would be to keep the same set of VFS ops, and
abstract out the lower level set of protocol operations. This is the
model that NFS uses for NFSv2 and 3. v4 is a bit different but it
does use the same file and inode ops for some things. See, for example,
nfs_v3_clientops.

Even if you do take Christoph's suggestion, I think there is value in
abstracting out the protocol operations as well. Better organization
like this will make the code less of a pain to maintain over the long
term.

There are also some things in this set that really ought to be
encapsulated into accessor routines. For example:

+       /* Don't want to modify the buffer as a side effect of this call. */
+       if (server->is_smb2 == false)
+               smb_buffer->smb_buf_length = cpu_to_be32(smb_buf_length);
+#ifdef CONFIG_CIFS_SMB2
+       else
+               smb2_buffer->smb2_buf_length = cpu_to_be32(smb_buf_length);
+#endif


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>