ntfs broken when share through samba3

[Lowell Gilbert]

mday at apple.com (Mark Day) writes:

> On Oct 16, 2006, at 2:42 PM, Lowell Gilbert wrote:
>
>> absorbb at gmail.com (Ильдар Нурисламов) writes:
>>
>>> This old already reported bug.
>>> But situation have'nt changed.
>>
>> For example, kern/86965.
>>
>>> There is very simple patch that fix this bug:
>>>
>>> --- usr/src/sys/fs/ntfs/ntfs_vnops.c	Mon Mar 13 00:50:01 2006
>>> +++ home/voxel/stuff/ntfs_vnops.c	Thu Aug 31 09:22:08 2006
>>> @@ -187,7 +187,8 @@
>>>  	vap->va_fsid = dev2udev(ip->i_dev);
>>>  	vap->va_fileid = ip->i_number;
>>>  	vap->va_mode = ip->i_mp->ntm_mode;
>>> -	vap->va_nlink = ip->i_nlink;
>>> +	vap->va_nlink = (ip->i_nlink ? ip->i_nlink : 1);
>>> +	//vap->va_nlink = ip->i_nlink;
>>>  	vap->va_uid = ip->i_mp->ntm_uid;
>>>  	vap->va_gid = ip->i_mp->ntm_gid;
>>>  	vap->va_rdev = 0;				/* XXX UNODEV ? */
>>>
>>> but it seems to be not beaty solution
>>
>> Not beautiful, indeed.
>>
>> I was playing around with this, and although that change would work
>> around the problem in (at least) most cases, I am not sure that it is
>> truly correct.
>>
>> I am not an expert at filesystems, and certainly have little knowledge
>> of NTFS.  However, my observations confuse me considerably.  The main
>> issue is that if you read from a file (on NTFS, with a link count of
>> zero according to ls(1)), the link count becomes populated.  I cannot
>> see how that would happen, because the ntnode structure link count is
>> not modified except when reading the whole structure from the disk,
>> and the on-disk node is not being changed.  To confuse things further,
>> the link count is changed to 2, not 1, on ordinary files that have
>> only a single directory entry.  I do not believe that streams are at
>> issue, because the file has no open file descriptors remaining
>> according to fstat(1).
>
> IIRC, the NTFS code tries to populate a vnode based on the limited
> information present in the directory entries it sees.  It's trying to
> avoid having to go read the Master File Table record (the i-node
> equivalent) until it actually needs that information (such as the link
> count).  The ntfs_loadntnode() routine will read in the MFT record and
> populate the rest of the vnode's fields.  There's a flag
> (VG_DONTLOADIN) to pass to ntfs_vgetex to control whether the MFT- 
> based fields get filled in when get the vnode.
>
> Hope this helps,

Yes, that clears things up for me considerably.  Thank you, Mark.

One thing I can see from that is that the proper loading of the ntnode
from the MFT will not be affected by faking a link count; the
IN_LOADED flag will take care of that.  Furthermore, using that same
flag to add to this patch, so that a zero count in the MFT will not be
ignored, will avoid the rest of my major concerns.  The patch would
end up more like:

 --- usr/src/sys/fs/ntfs/ntfs_vnops.c	Mon Mar 13 00:50:01 2006
 +++ home/voxel/stuff/ntfs_vnops.c	Thu Aug 31 09:22:08 2006
 @@ -187,7 +187,8 @@
  	vap->va_fsid = dev2udev(ip->i_dev);
  	vap->va_fileid = ip->i_number;
  	vap->va_mode = ip->i_mp->ntm_mode;
 -	vap->va_nlink = ip->i_nlink;
 +	vap->va_nlink = (ip->i_nlink || ip->i_flag & IN_LOADED ? ip->i_nlink : 1);
  	vap->va_uid = ip->i_mp->ntm_uid;
  	vap->va_gid = ip->i_mp->ntm_gid;
  	vap->va_rdev = 0;				/* XXX UNODEV ? */

This still doesn't meet POSIX requirements, but to do that would
require reading the whole MFT entry every time, instead of just the
directory entries.  That optimization speeds things up a lot in large
filename searches, so this seems like a good compromise to me.

Or am I missing something?