From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Steve French" Subject: Re: review 5, was Re: projected date for mount.cifs to support DFS junction points Date: Fri, 15 Feb 2008 15:02:19 -0600 Message-ID: <524f69650802151302h4e631a74pe382ca932cc85c7@mail.gmail.com> References: <1199988975.7483.3.camel@gn2.draper.com> <524f69650801101228o3639363cp4c9710d747b71ead@mail.gmail.com> <20080111090749.GA14910@infradead.org> <524f69650801110805y56cdbe4nf7587e396b70f32c@mail.gmail.com> <20080113202144.GB24573@infradead.org> <47B5BFCF.60304@mail.ru> <20080215170552.GA27584@infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: linux-fsdevel , linux-cifs-client@lists.samba.org To: "Christoph Hellwig" Return-path: In-Reply-To: <20080215170552.GA27584@infradead.org> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cifs-client-bounces+glfc-linux-cifs-client=gmane.org@lists.samba.org Errors-To: linux-cifs-client-bounces+glfc-linux-cifs-client=gmane.org@lists.samba.org List-Id: linux-fsdevel.vger.kernel.org On 2/15/08, Christoph Hellwig 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). > 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. -- Thanks, Steve