[PATCH] fstests: btrfs/012 don't follow symlinks for populating $SCRATCH_MNT

[PATCH] fstests: btrfs/012 don't follow symlinks for populating $SCRATCH_MNT

[Josef Bacik]

/lib/modules/$(uname -r)/ can have symlinks to the source tree where the
kernel was built from, which can have all sorts of stuff, which will
make the runtime for this test exceedingly long.  We're just trying to
copy some data into our tree to test with, we don't need the entire
devel tree of whatever we're doing, so use -P to not follow symlinks
when copying.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/btrfs/012 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/btrfs/012 b/tests/btrfs/012
index d9faf81c..1b6e8a6b 100755
--- a/tests/btrfs/012
+++ b/tests/btrfs/012
@@ -43,7 +43,7 @@ mount -t ext4 $SCRATCH_DEV $SCRATCH_MNT
 
 _require_fs_space $SCRATCH_MNT $(du -s /lib/modules/`uname -r` | ${AWK_PROG} '{print $1}')
 
-cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT
+cp -aPR /lib/modules/`uname -r`/ $SCRATCH_MNT
 _scratch_unmount
 
 # Convert it to btrfs, mount it, verify the data
@@ -67,7 +67,7 @@ umount $SCRATCH_MNT/mnt
 
 # Now put some fresh data on the btrfs fs
 mkdir -p $SCRATCH_MNT/new 
-cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT/new
+cp -aPR /lib/modules/`uname -r`/ $SCRATCH_MNT/new
 
 _scratch_unmount
 
-- 
2.26.3

Re: [PATCH] fstests: btrfs/012 don't follow symlinks for populating $SCRATCH_MNT

[Roman Mamedov]

On Fri, 24 Mar 2023 16:13:19 -0400
Josef Bacik <josef@toxicpanda.com> wrote:

> /lib/modules/$(uname -r)/ can have symlinks to the source tree where the
> kernel was built from, which can have all sorts of stuff, which will
> make the runtime for this test exceedingly long.  We're just trying to
> copy some data into our tree to test with, we don't need the entire
> devel tree of whatever we're doing, so use -P to not follow symlinks
> when copying.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  tests/btrfs/012 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/btrfs/012 b/tests/btrfs/012
> index d9faf81c..1b6e8a6b 100755
> --- a/tests/btrfs/012
> +++ b/tests/btrfs/012
> @@ -43,7 +43,7 @@ mount -t ext4 $SCRATCH_DEV $SCRATCH_MNT
>  
>  _require_fs_space $SCRATCH_MNT $(du -s /lib/modules/`uname -r` | ${AWK_PROG} '{print $1}')
>  
> -cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT
> +cp -aPR /lib/modules/`uname -r`/ $SCRATCH_MNT

But did you face the described problem in actual operation?

"man cp" says 

       -a, --archive
              same as -dR --preserve=all
...
       -d     same as --no-dereference --preserve=links
...
       -P, --no-dereference
              never follow symbolic links in SOURCE

So -a includes -d, which already includes -P that is now being added.

Moreover, even -R is redundant in the original line and could be removed.

-- 
With respect,
Roman

Re: [PATCH] fstests: btrfs/012 don't follow symlinks for populating $SCRATCH_MNT

[Zorro Lang]

On Sat, Mar 25, 2023 at 01:46:28AM +0500, Roman Mamedov wrote:
> On Fri, 24 Mar 2023 16:13:19 -0400
> Josef Bacik <josef@toxicpanda.com> wrote:
> 
> > /lib/modules/$(uname -r)/ can have symlinks to the source tree where the
> > kernel was built from, which can have all sorts of stuff, which will
> > make the runtime for this test exceedingly long.  We're just trying to
> > copy some data into our tree to test with, we don't need the entire
> > devel tree of whatever we're doing, so use -P to not follow symlinks
> > when copying.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > ---
> >  tests/btrfs/012 | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/btrfs/012 b/tests/btrfs/012
> > index d9faf81c..1b6e8a6b 100755
> > --- a/tests/btrfs/012
> > +++ b/tests/btrfs/012
> > @@ -43,7 +43,7 @@ mount -t ext4 $SCRATCH_DEV $SCRATCH_MNT
> >  
> >  _require_fs_space $SCRATCH_MNT $(du -s /lib/modules/`uname -r` | ${AWK_PROG} '{print $1}')
> >  
> > -cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT
> > +cp -aPR /lib/modules/`uname -r`/ $SCRATCH_MNT
> 
> But did you face the described problem in actual operation?

Same question, due to from what I tested[1], current options "-aR" don't
follow the symbolic. Is there any different on your system, or your fs?
Can you show more details about the issue you hit?

Thanks,
Zorro

[1]
$ mkdir testdir
$ cd testdir
$ ln -s ../xfstests link-dir
$ ln -s ../img link-file
$ cd ..
$ cp -aR testdir/ copydir
$ ls -l copydir/
total 0
lrwxrwxrwx. 1 zorro zorro 11 Mar 25 12:29 link-dir -> ../xfstests
lrwxrwxrwx. 1 zorro zorro  6 Mar 25 12:29 link-file -> ../img




> 
> "man cp" says 
> 
>        -a, --archive
>               same as -dR --preserve=all
> ...
>        -d     same as --no-dereference --preserve=links
> ...
>        -P, --no-dereference
>               never follow symbolic links in SOURCE
> 
> So -a includes -d, which already includes -P that is now being added.
> 
> Moreover, even -R is redundant in the original line and could be removed.
> 
> -- 
> With respect,
> Roman
> 

Re: [PATCH] fstests: btrfs/012 don't follow symlinks for populating $SCRATCH_MNT

[Christoph Hellwig]

On Fri, Mar 24, 2023 at 04:13:19PM -0400, Josef Bacik wrote:
> /lib/modules/$(uname -r)/ can have symlinks to the source tree where the
> kernel was built from, which can have all sorts of stuff, which will
> make the runtime for this test exceedingly long.  We're just trying to
> copy some data into our tree to test with, we don't need the entire
> devel tree of whatever we're doing, so use -P to not follow symlinks
> when copying.

Btw, if you're touching this test can you make it _notrun if the
directory doesn't exist?  This test always fails for VMs without
any modules.

Re: [PATCH] fstests: btrfs/012 don't follow symlinks for populating $SCRATCH_MNT

[Darrick J. Wong]

On Sat, Mar 25, 2023 at 01:18:52AM -0700, Christoph Hellwig wrote:
> On Fri, Mar 24, 2023 at 04:13:19PM -0400, Josef Bacik wrote:
> > /lib/modules/$(uname -r)/ can have symlinks to the source tree where the
> > kernel was built from, which can have all sorts of stuff, which will
> > make the runtime for this test exceedingly long.  We're just trying to
> > copy some data into our tree to test with, we don't need the entire
> > devel tree of whatever we're doing, so use -P to not follow symlinks
> > when copying.
> 
> Btw, if you're touching this test can you make it _notrun if the
> directory doesn't exist?  This test always fails for VMs without
> any modules.

If you just want some filler content for the "old" ext4 filesystem, why
not simply copy something that's guaranteed to exist like /etc or /var?
And perhaps use timeout(1) to control the cp runtime?

(I don't have the symlink problem, but I occasionally watch this test
suddenly take forever if I accidentally turn off module compression
and/or slim debuginfo...)

--D