Re: [PATCH xfstest v2 3/4] overlay: add fsck.overlay redirect directory test

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



On 2017/12/30 17:35, Amir Goldstein Wrote:
On Sat, Dec 30, 2017 at 6:06 AM, zhangyi (F) <yizhang089@xxxxxxxxx> wrote:
On 2017/12/28 20:37, Amir Goldstein Wrote:

+# Test valid redirect xattr with general directory cover lower origin
+# target, should ask user this directory is merged or not by default
+# and do nothing in auto mode
+echo "+ Valid redirect(4)"
+mkdir $lowerdir/origin
+mkdir $upperdir/origin
+make_redirect_dir $upperdir/testdir "origin"
+
+_overlay_fsck_dirs $lowerdir $upperdir $workdir -p >> $seqres.full 2>&1
|| \
+       _fail "fsck should not fail"
+check_redirect "origin" $upperdir/testdir
+$GETFATTR_PROG --absolute-names -d -m $OVL_OPAQUE_XATTR
$upperdir/testdir
+
+echo "n" > $tmp.input
This approach seems a bit fragile to me.
Maybe you will want to add some questions in the future.
To me it makes better sense to check behavior of fsck -y is automated
test,
because some user who does not understand the questions is surely going
to run fsck -y to fix problems.

+_overlay_fsck_dirs $lowerdir $upperdir $workdir <$tmp.input >>
$seqres.full 2>&1 || \
+       _fail "fsck should not fail"
+check_redirect "origin" $upperdir/testdir
+check_opaque $upperdir/origin
+rm -rf $lowerdir/* $upperdir/*
+
Hi Amir:

Think about this again. IIUC, I think there is a consistency problem about
this case.
If we allow user to create a new directory in upper layer merge with lower
redirect origin target when overlay offline, we will see duplicate d_ino in
overlay filesystem after next mount, becasue current ovl_getattr() will get
the lower d_ino for both merged dir and redirect dir.

eg:
mnt:         dira(ino=x)    dirb(ino=x)
upper:      *dira(ino=y)   dirb(ino=z, redirect=dirb)
lower:       dira(ino=x)

*) User remove whiteout or opaque dir and then newly created when overlayfs
is offline.

Furthermore, if user rename dira again, will lead to duplicate redirect
xattr directly.
So I think we should forbid user do this and check by fsck.overlay. Or
handle by overlay filesytem?  thought?

I am not sure I follow. Of course there is a consistency problem with
allowing merge
and redirect to same target. merge dir is just a private case of
"redirect to .".
merge+redirect is not different in any way than multiple redirect.
All our discussion w.r.t the "Is merge dir?" question was revolving around the
fact that either the merge dir or the redirected dir needs to be fixed.
Exactly,If user selsect the "merge" option(not opaque), we also should fix the
redirected dir which point to the same origin. Thanks!

And duplicate d_ino is the *least* of the problem.
With duplicate redirect, there can be many objects (all descendants)
in the file system
with the same st_dev;st_ino, which may have different data.

This work in my queue is verifying multiple redirect/merge at lookup time,
but *only* if directories where indexed to begin with.
fsck.overlay will be needed to find and correct duplicate redirects
and even merge
dirs that were copied up without an index and fix the index (if
-overify=on is given).

There is a limitation of verify=on that it cannot use index to verify
mulitple redirect
from mid layers. This is where only fsck.overlay can be used.
OK, I can add this check and fix latter.  :)

Cheers,
Yi.
--
To unsubscribe from this list: send the line "unsubscribe fstests" 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 Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux