On Fri, Jul 5, 2013 at 5:26 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> On Fri, 2013-07-05 at 17:12 -0500, Steve French wrote:
>> Yes - I noticed that too (although with David's patch at least it is
>> better). For the POSIX case for readdir we are ok, but for the
>> non-posix case for readdir there is are only two infolevels, one too
>> big and one too small (FILE_STANDARD_INFORMATION) that seem to return
>> nlinks so we should be setting them to default values if we can't get
>> them from the server (ie if cf_attr->nlink is 0). This patch should
>> fix it by catching it when we are about to update the inode to make
>> sure we are putting a legal value in (I have not tested it yet).
>> Thoughts on this minor followon patch?
>>
>> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
>> index 449b6cf..8329c18 100644
>> --- a/fs/cifs/inode.c
>> +++ b/fs/cifs/inode.c
>> @@ -134,7 +134,13 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr
>> inode->i_mtime = fattr->cf_mtime;
>> inode->i_ctime = fattr->cf_ctime;
>> inode->i_rdev = fattr->cf_rdev;
>> - set_nlink(inode, fattr->cf_nlink);
>> + /* number of links on a directory is at least 2, at least 1 for file */
>> + if ((fattr->cf_cifsattrs & ATTR_DIRECTORY) && (fattr->cf_nlink < 2))
>> + set_nlink(inode, 2);
>> + else if (fattr->cf_nlink < 1)
>> + set_nlink(inode, 1);
>> + else
>> + set_nlink(inode, fattr->cf_nlink);
>> inode->i_uid = fattr->cf_uid;
>> inode->i_gid = fattr->cf_gid;
>>
>
> There are cases where you might see a legit i_nlink value of 0. For
> instance...
>
> client> fd = open("file", ...);
> server> unlink("file");
> client> fstat(file, ...);
>
> ...now the server may legitimately report a link count of 0, but with
> this patch we'll end up fudging it to something higher. Hence, I'm a
> little leery of doing this sort of blanket overriding of cf_nlink value,
> especially in the case where POSIX extensions are in force.
>
> I think we should only do this when we really think we can't trust this
> value -- i.e. the non-POSIX case. Experience has shown that
> NumberOfLinks value is not *exactly* a 1:1 match with an i_nlink value
> when talking to windows-y servers.
Makes sense - but as Tom notes we should check for the delete pending
case when number of links is zero, and as you note we should nont be
overriding number of links everywhere - but should fix the non-posix readdir
case
--
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
