Re: [PATCH] ovl: fix inconsistent d_ino for legacy merge dir

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2018/1/2 16:14, Amir Goldstein Wrote:
> On Tue, Jan 2, 2018 at 10:00 AM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote:
>> On 2017/12/31 17:49, Amir Goldstein Wrote:
>>> For a merge dir that was copied up before v4.12 or that was hand crafted
>>> offline (e.g. mkdir {upper/lower}/dir), upper dir does not contain the
>>> 'trusted.overlay.origin' xattr.  In that case, stat(2) on the merge dir
>>> returns the lower dir st_ino, but getdents(2) returns the upper dir d_ino.
>>>
>>> After this change, on merge dir lookup, missing origin xattr on upper
>>> dir will be fixed and 'impure' xattr will be fixed on parent of the legacy
>>> merge dir.
>>>
>>> Suggested-by: zhangyi (F) <yi.zhang@xxxxxxxxxx>
>>> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>>> ---
>>>
>>> Zhangyi,
>>>
>>> Please review/test this patch.
>>> I ended up applying it at the bottom of verify_dir [1] series,
>>> so it could be merged to v4.15 and/or easily backported to v4.14
>>> (where consistent d_ino was merged) if Miklos thinks this is an important
>>> use case to fix.
>>>
>>
>> Hi Amir,
>> This patch looks good (one nit below) but still have a case which will lead to problem
>> we missed in last discussion:
>> Call getdent(2) before invoke ovl_lookup() after first mount.
>>
>> eg:
>> 1) mkdir {upper/lower}/dir merge work
>> 2) mount to merge
>> 3) getdent merge
>> 2253114  directory    24          2  dir
>> 4) ls merge
>> 5) getdent merge
>> 2253113  directory    24          2  dir
>>
>> Strictly speaking, it is a problem ,but not sure it's worth to fix now.
> 
> Yeh, it's true.
> 
> Please note, that the sole purpose of "impure" xattr in the world,
> is to optimize away the need to lookup directory entries in getdents
> for fixing d_ino.
> So yes, I think this specific corner case is something that users will have to
> be educated about.
> 
> Also please note that I have not found any program that relies of d_ino of
> directories (not that I would know where to look). I did check that
> 'find -inum <ino>'
> does NOT reply of values of d_ino for subdirs. 'find' always call
> stat(2) for subdirs.
> So that is one less program to worry about.
> 

I see, you can add:
Reviewed-by: zhangyi (F) <yi.zhang@xxxxxxxxxx>

Thanks,
Yi.

--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux