On Thu, Mar 17, 2011 at 10:04 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > On Thu, 17 Mar 2011 09:54:48 -0500 > Steve French <smfrench@xxxxxxxxx> wrote: > >> On Thu, Mar 17, 2011 at 9:47 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: >> > On Thu, 17 Mar 2011 09:41:34 -0500 >> > Steve French <smfrench@xxxxxxxxx> wrote: >> > >> >> On Thu, Mar 17, 2011 at 9:35 AM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: >> >> > Btw, what branch are these commits for? I dearly hope you're not trying >> >> > to push half-assed code to mainline. Please do your development on a >> >> > branch first, and once there is a useable implementation it can be >> >> > reviewed and synced over. Take a look at pnfs development for example. >> >> >> >> We have an implementation that was in the test tree for a year, went >> >> through 3 test events that is being ported. I don't think it will >> >> take more than a week or so to get enough of it in. Unless it gets >> >> upstream, it is not going to get much wider review than from the >> >> original group (Pavel, Jeremy etc). Note that all code is marked as >> >> "broken" and the majority in distinct c files - this is not that >> >> different than pNFS, which only recently got usable in mainline but >> >> was checked in many releases back. >> >> >> > >> > I agree with Christoph here. If the code is marked "broken" then >> > merging it seems premature. I'd like to see the majority of this code >> > in a branch that we can test before it goes in anywhere. >> > >> > The exception would be targeted patches that prepare the existing >> > code to work with the new SMB2 code. Those can be reviewed and we can >> > should be able to take those prior to merging the smb2 code wholesale. >> >> The code to prepare for SMB2 should be mostly in place in >> tree now: >> - error mapping >> - transport >> - mount >> - stats >> >> . The sendrcv2 routine was the last part of that and >> was waiting on the bigendian change. I can defer >> all inode/file/address routines if you prefer until they are reviewed >> together, but the existing version in smb2.git could be used >> as a base depending on how much commonality people prefer. >> >> >> We have had over a year, including work by 4 developers in a distinct >> tree and got little meaningful feeback on features though until >> recently when it started showing up in cifs-2.6.git In over a year, >> the mainline code diverged in multiple trivial ways that made the port harder >> because the code was not visible to those outside the small number >> of developers who understand smb2. >> > > One of the stated goals here is to minimize disruption to the existing > cifs codebase. The best way to do that is to break out the changes > needed to the existing codebase into a separate set of patches and post > those for review. The endianness change for the smb_buf_length is a > good example of this sort of change. Once those changes have reviewed > and merged, then we can start discussing how to merge the rest of the > SMB2 support. > > I really don't think this approach is at all unreasonable. It may mean > a little more work on your end to break out these changes logically, > but I think it really needs to be done that way. Whatever makes it easier to get broader review is good. It is frustrating of course that some of these changes we are discussing have been brought up before, and weren't high priority until the discussion of how to make smb2 transport patch as small as possible (smaller than what was in smb2.git). As requested, I will put the smbtransport patch into a new smb2 specific branch on cifs-2.6.git. Since the smb2 related code which touches core cifs files has already been discussed, and should be ok now (with the bigendian change for smb_buf_length), with no disruption to cifs code expected - the five new smb2 files (which don't get compiled in for default cifs) could be removed via a single patch, and moved to the smb2 specific branch. The major area I expect review comments on is not these but rather the final stage of the port of the smb2 code - how to minimize the file_ops, inode_ops, aops and having a distinct smb2 branch while these get discussed would be helpful. -- Thanks, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
