[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-cifs-client
Subject:    =?koi8-r?Q?Re[2]=3A_[linux-cifs-client]_review_5,
From:       Q <qwerty0987654321 () mail ! ru>
Date:       2008-02-16 8:51:52
Message-ID: E1JQIm0-000M99-00.qwerty0987654321-mail-ru () f80 ! mail ! ru
[Download RAW message or body]



-----Original Message-----
From: "Steve French" <smfrench@gmail.com>
To: "Christoph Hellwig" <hch@infradead.org>
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@infradead.org> 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
> 
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@lists.samba.org
https://lists.samba.org/mailman/listinfo/linux-cifs-client


[prev in list] [next in list] [prev in thread] [next in thread]