diff mbox

[V3] xfstest: overlay: Absolute redirect should be followed even if ancestor is opaque

Message ID 20180316025554.GM30836@localhost.localdomain (mailing list archive)
State New, archived
Headers show

Commit Message

Eryu Guan March 16, 2018, 2:55 a.m. UTC
On Fri, Mar 16, 2018 at 10:53:20AM +0800, Eryu Guan wrote:

> > +# Create opaque parent with absolute redirect child in middle layer
> > +mkdir $SCRATCH_MNT/pure
> > +mv $SCRATCH_MNT/origin $SCRATCH_MNT/pure/redirect
> > +$UMOUNT_PROG $SCRATCH_MNT
> > +_overlay_scratch_mount_dirs $lowerdir2:$lowerdir $upperdir $workdir -o redirect_dir=on
> > +mv $SCRATCH_MNT/pure/redirect $SCRATCH_MNT/redirect
> 
> I think we'd better do a "ls $SCRATCH_MNT/redirect/" here too, right
> after the rename and before mount cycle, to make sure 'foo' is there as
> well. I can add it on commit (and change the .out file too) if this
> change looks OK to you.

Something like this:

git a/tests/overlay/057 b/tests/overlay/057
index 10b46bfa8cff..d54024e98bcd 100644

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

Comments

Amir Goldstein March 16, 2018, 5:58 a.m. UTC | #1
On Fri, Mar 16, 2018 at 4:55 AM, Eryu Guan <guaneryu@gmail.com> wrote:
> On Fri, Mar 16, 2018 at 10:53:20AM +0800, Eryu Guan wrote:
>
>> > +# Create opaque parent with absolute redirect child in middle layer
>> > +mkdir $SCRATCH_MNT/pure
>> > +mv $SCRATCH_MNT/origin $SCRATCH_MNT/pure/redirect
>> > +$UMOUNT_PROG $SCRATCH_MNT
>> > +_overlay_scratch_mount_dirs $lowerdir2:$lowerdir $upperdir $workdir -o redirect_dir=on
>> > +mv $SCRATCH_MNT/pure/redirect $SCRATCH_MNT/redirect
>>
>> I think we'd better do a "ls $SCRATCH_MNT/redirect/" here too, right
>> after the rename and before mount cycle, to make sure 'foo' is there as
>> well. I can add it on commit (and change the .out file too) if this
>> change looks OK to you.
>

OK, but...

> Something like this:
>
> git a/tests/overlay/057 b/tests/overlay/057
> index 10b46bfa8cff..d54024e98bcd 100644
> --- a/tests/overlay/057
> +++ b/tests/overlay/057
> @@ -86,6 +86,8 @@ mv $SCRATCH_MNT/origin $SCRATCH_MNT/pure/redirect
>  $UMOUNT_PROG $SCRATCH_MNT
>  _overlay_scratch_mount_dirs $lowerdir2:$lowerdir $upperdir $workdir -o redirect_dir=on
>  mv $SCRATCH_MNT/pure/redirect $SCRATCH_MNT/redirect
> +# Verify that redirects are followed

The wording here is not technically accurate.
"redirect" are "followed" on lookup and there is no lookup here
(dentry is already in cache). It would be more accurate to say
some thing like "List content of renamed merge dir before mount cycle"
and then maybe to use the same language, "Verify that redirects are
followed by listing content of renamed merge dir after after cycle".

> +ls $SCRATCH_MNT/redirect/
>
>  # Verify that redirects are followed after mount cycle
>  $UMOUNT_PROG $SCRATCH_MNT
> diff --git a/tests/overlay/057.out b/tests/overlay/057.out
> index 750830212460..b3b4017d32ae 100644
> --- a/tests/overlay/057.out
> +++ b/tests/overlay/057.out
> @@ -1,2 +1,3 @@
>  QA output created by 057
>  foo
> +foo
>
> Thanks,
> Eryu
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eryu Guan March 16, 2018, 6:18 a.m. UTC | #2
On Fri, Mar 16, 2018 at 07:58:35AM +0200, Amir Goldstein wrote:
> On Fri, Mar 16, 2018 at 4:55 AM, Eryu Guan <guaneryu@gmail.com> wrote:
> > On Fri, Mar 16, 2018 at 10:53:20AM +0800, Eryu Guan wrote:
> >
> >> > +# Create opaque parent with absolute redirect child in middle layer
> >> > +mkdir $SCRATCH_MNT/pure
> >> > +mv $SCRATCH_MNT/origin $SCRATCH_MNT/pure/redirect
> >> > +$UMOUNT_PROG $SCRATCH_MNT
> >> > +_overlay_scratch_mount_dirs $lowerdir2:$lowerdir $upperdir $workdir -o redirect_dir=on
> >> > +mv $SCRATCH_MNT/pure/redirect $SCRATCH_MNT/redirect
> >>
> >> I think we'd better do a "ls $SCRATCH_MNT/redirect/" here too, right
> >> after the rename and before mount cycle, to make sure 'foo' is there as
> >> well. I can add it on commit (and change the .out file too) if this
> >> change looks OK to you.
> >
> 
> OK, but...
> 
> > Something like this:
> >
> > git a/tests/overlay/057 b/tests/overlay/057
> > index 10b46bfa8cff..d54024e98bcd 100644
> > --- a/tests/overlay/057
> > +++ b/tests/overlay/057
> > @@ -86,6 +86,8 @@ mv $SCRATCH_MNT/origin $SCRATCH_MNT/pure/redirect
> >  $UMOUNT_PROG $SCRATCH_MNT
> >  _overlay_scratch_mount_dirs $lowerdir2:$lowerdir $upperdir $workdir -o redirect_dir=on
> >  mv $SCRATCH_MNT/pure/redirect $SCRATCH_MNT/redirect
> > +# Verify that redirects are followed
> 
> The wording here is not technically accurate.
> "redirect" are "followed" on lookup and there is no lookup here
> (dentry is already in cache). It would be more accurate to say
> some thing like "List content of renamed merge dir before mount cycle"
> and then maybe to use the same language, "Verify that redirects are
> followed by listing content of renamed merge dir after after cycle".

I'll update as suggested. Thanks!

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

Patch

--- a/tests/overlay/057
+++ b/tests/overlay/057
@@ -86,6 +86,8 @@  mv $SCRATCH_MNT/origin $SCRATCH_MNT/pure/redirect
 $UMOUNT_PROG $SCRATCH_MNT
 _overlay_scratch_mount_dirs $lowerdir2:$lowerdir $upperdir $workdir -o redirect_dir=on
 mv $SCRATCH_MNT/pure/redirect $SCRATCH_MNT/redirect
+# Verify that redirects are followed
+ls $SCRATCH_MNT/redirect/
 
 # Verify that redirects are followed after mount cycle
 $UMOUNT_PROG $SCRATCH_MNT
diff --git a/tests/overlay/057.out b/tests/overlay/057.out
index 750830212460..b3b4017d32ae 100644
--- a/tests/overlay/057.out
+++ b/tests/overlay/057.out
@@ -1,2 +1,3 @@ 
 QA output created by 057
 foo
+foo