Re: [PATCH v2 01/18] overlay: implement fsck utility

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

 



On 2017/12/15 15:45, Amir Goldstein Wrote:
> On Fri, Dec 15, 2017 at 5:35 AM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote:
>> On 2017/12/14 22:13, Miklos Szeredi Wrote:
> [...]
> 
>>>
>>>>
>>>> Opaque directories
>>>> ------------------
>>>>
>>>> An opaque directory is a directory with "trusted.overlay.opaque" xattr
>>>> valued "y". There are two legal situations of making opaque directory: 1)
>>>> create directory over whiteout 2) creat directory in merged directory. If an
>>>> opaque directory is found, corresponding matching name in lower layers might
>>>> exist or parent directory might merged, If not, the opaque xattr will be
>>>> treated as invalid and remove.
>>>
>>> Current version of overlay fs doesn't bother with removing opaque
>>> attribute.  So not sure fsck.overlay should care.  Or at least is
>>> should be an optional thing and not worth warning about, since it's
>>> perfectly normal.
>>>
>> Indeed, we can introduce '-f' option to handle this (e2fsck(8)):
>>
>>   -f     Force checking even if the file system seems clean.
>>
> 
> Hmm, I would not use -f for that, because it means something completely
> different in e2fsck.
> ext4 has a "clean" super block flag, which is set on clean unmount.
> -f means to run even if "clean" flag is set.
> Please reserve -f for that future use case.
> What Miklos is saying, and I agree with him, is that fsck.overlay should be
> resolve inconsistencies rather then cleanup leftovers that don't bother anyone.
> 
Yes, it seems that check opaque is useless, we should more take care of what will
lead to inconsistencies, will remove this check.

> BTW, you should also add a check for "impure" xattr.
> There are clear rules about when "impure" should be set - when a pure dir has
> entries with "origin" or "redirect" entries inside it - those rule
> were NOT honored
> since introduction of "redirect" and they were even not fully honored sine
> introduction of "origin" ("ovl: mark parent impure on ovl_link()" was
> added later)
> 
> Missing "impure" will cause wrong d_ino in readdir of impure dir.
> 
This is listed in my todo list, will realize this check soon.

> [...]
> 
>>>
>>>> 2) The origin directory should be redirected only once in one layer,
>>>> which mean there is only one redirect xattr point to this origin directory in
>>>> the specified layer.
>>>> 3) A whiteout or an opaque directory with the same name to origin should
>>>> exist in the same directory as the redirect directory.
>>>>
>>>> If not, 1) The redirect xattr is invalid and need to remove 2) One of
>>>> the redirect xattr is redundant but not sure which one is, ask user 3)
>>>> Create a whiteout device or set opaque xattr to an existing directory if the
>>>> parent directory was meregd or remove xattr if not.
>>>
>>> Hmm, in this case also should ask the user, as it's not clear that the
>>> "new" copy resulting from removal of whiteout on upper is the wanted
>>> one or the "old" renamed copy.
>>>
>> Thanks for point this out, do you mean the newly created directory over whiteout
>> between old renamed directory? If so, Amir have already pointed out in the first
>> iteration and I handled it in current version. Please see:
>>
>> https://www.spinics.net/lists/linux-unionfs/msg03268.html
>>
>> Now, fsck will also check dir's redirect xattr for this case, it will add opaque
>> xattr only when parent is merged and not redirected.
>>
> 
> I think what Miklos meant is what I wrote in comment to test 203.
> User needs to be asked which dir should be the merge dir and not
> assume that the redirected dir is the correct answer.
> 
Understand, will fix it.

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