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