-----Original Message----- From: "Steve French" <smfrench@xxxxxxxxx> To: "Christoph Hellwig" <hch@xxxxxxxxxxxxx> Date: Fri, 15 Feb 2008 15:02:19 -0600 Subject: Re: [linux-cifs-client] review 5, was Re: projected date for mount.cifs to support DFS junction points > > On 2/15/08, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > On Fri, Feb 15, 2008 at 07:37:35PM +0300, Q (Igor Mammedov) wrote: > > > Here is what I've done the last weekend. > > > Attached: > > > fixed patch [5/5] (0001-DFS-patch-that-connects-inode-with-dfs-handling-ops.patch). > Not merged yet. > > > > fixed mixed case in struct member 0002-Fixed-mixed-case-name-in-structure-dfs_info3_param.patch) > > Now merged into cifs-2.6.git > > > The second one is trivially correct and should be pushed to Linus asap > > as small cleanup. Patch 1 is not exactly what I had in mind, I was > > thinking about factoring out the common bits of cifs-cifs_get_inode_info > > and cifs-cifs_get_inode_info_unix to avoid having all the logic to > > set the various options twice. I've attached two quick and untested > > patches showing what I mean. I think in this case having the ifdef > > for that two line statement setting the inode operations here is fine. > I reviewed and merged into cifs-2.6.git one of the two patches from > Christoph (the cifs_set_ops one), but wanted to look more carefully at > the other (cifs_get_info_remote) to make sure that buf was being freed > in the cifs_get_inode_info path (otherwise it is fine). At first glance cifs_get_inode_info_remote won't work cause it's old dfs code not new one. But I caught what Christoph meant now, and will try to rewrite it this way. > > But thinking about it I'm not even sure if it's good idea to make > > dfs support conditional. Any reason it can't just be included > > unconditionally? > It would make the code slightly smaller (perhaps useful someday for > OLPC or embedded) and removes a piece of code that is not needed in > all home networks (although DFS is useful even to some of these). I > lean toward removing the ifdef when it has made it through one or two > more release cycles and is no longer experimental. SInce there are a > few experimental features (Kerberos and DFS) that are broadly useful - > but not all users need both, I don't mind keeping the configure for > each different for the short term but don't have a strong opinion on > this. IMHO, +1 to keeping DFS ifdefs for a while. > -- > Thanks, > > Steve > - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html