* [PATCH v3] fstests: test xfs_copy V5 XFS without -d option
@ 2016-09-30 5:23 Zorro Lang
2016-09-30 9:14 ` Eryu Guan
0 siblings, 1 reply; 3+ messages in thread
From: Zorro Lang @ 2016-09-30 5:23 UTC (permalink / raw)
To: fstests
Before xfsprogs commit a872b62 (xfs_copy: band-aids for CRC
filesystems), xfs_copy requires the "-d" option to copy a V5 XFS,
because it can't rewrite the UUID of V5 XFS properly.
Now xfs_copy already full support to copy a V5 XFS. But for above
old problem, xfstests use below patch to make sure xfs_copy always
use "-d" option to copy a V5 XFS:
8346e53 common: append -d option to XFS_COPY_PROG when testing v5 xfs
That cause xfstests miss the coverage of copying a V5 XFS without
"-d". For test this feature I did below things:
1. Change init_rc(), add "-d" to $XFS_COPY_PROG if xfs_copy can't
copy a V5 XFS properly.
2. xfs/073 test V4 xfs forcibly by specify "-m crc=0" in case. I
think it's useless now, so remove it.
3. remove the xfs_copy "-d" option from xfs/032
Signed-off-by: Zorro Lang <zlang@redhat.com>
---
Hi,
V2:
1. remove require_xfs_copy() function
2. change the code logic of init_rc function about how to add "-d" to
$XFS_COPY_PROG
3. remove xfs_copy "-d" option of xfs/032
V3 add comments to explain the change in init_rc()
Thanks,
Zorro
common/rc | 15 ++++++++++++---
tests/xfs/032 | 2 +-
tests/xfs/073 | 8 ++------
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/common/rc b/common/rc
index 13afc6a..b0183b2 100644
--- a/common/rc
+++ b/common/rc
@@ -3790,9 +3790,18 @@ init_rc()
xfs_io -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
export XFS_IO_PROG="$XFS_IO_PROG -F"
- # xfs_copy doesn't work on v5 xfs yet without -d option
- if [ "$FSTYP" == "xfs" ] && [[ $MKFS_OPTIONS =~ crc=1 ]]; then
- export XFS_COPY_PROG="$XFS_COPY_PROG -d"
+ # xfs_copy on v5 filesystems do not require the "-d" option if xfs_db
+ # can change the UUID on v5 filesystems
+ if [ "$FSTYP" == "xfs" ]; then
+ touch $tmp.img
+ $MKFS_XFS_PROG $MKFS_OPTIONS -d file,name=$tmp.img,size=512m \
+ >/dev/null 2>/dev/null
+ # xfs_db will return 0 even if it can't generate a new uuid, so
+ # check the output to make sure if it can change UUID of V5 xfs
+ $XFS_DB_PROG -x -c "uuid generate" $tmp.img \
+ | grep -q "invalid UUID\|supported on V5 fs" \
+ && export XFS_COPY_PROG="$XFS_COPY_PROG -d"
+ rm -f $tmp.img
fi
}
diff --git a/tests/xfs/032 b/tests/xfs/032
index 6216379..0e41db8 100755
--- a/tests/xfs/032
+++ b/tests/xfs/032
@@ -65,7 +65,7 @@ while [ $SECTORSIZE -le $PAGESIZE ]; do
$FSSTRESS_PROG -n 100 -d $SCRATCH_MNT >> $seqres.full 2>&1
_scratch_unmount
- $XFS_COPY_PROG -d $SCRATCH_DEV $IMGFILE >> $seqres.full 2>&1 || \
+ $XFS_COPY_PROG $SCRATCH_DEV $IMGFILE >> $seqres.full 2>&1 || \
_fail "Copy failed for Sector size $SECTORSIZE Block size $BLOCKSIZE"
# Must use "-n" to get exit code; without it xfs_repair always returns 0
$XFS_REPAIR_PROG -n -f $IMGFILE >> $seqres.full 2>&1 || \
diff --git a/tests/xfs/073 b/tests/xfs/073
index 9e29223..7228dd9 100755
--- a/tests/xfs/073
+++ b/tests/xfs/073
@@ -138,7 +138,7 @@ _require_loop
rm -f $seqres.full
-_scratch_mkfs_xfs -m crc=0 -dsize=41m,agcount=2 >>$seqres.full 2>&1
+_scratch_mkfs_xfs -dsize=41m,agcount=2 >>$seqres.full 2>&1
_scratch_mount 2>/dev/null || _fail "initial scratch mount failed"
echo
@@ -158,11 +158,7 @@ _verify_copy $imgs.image $SCRATCH_DEV $SCRATCH_MNT
echo
echo === copying scratch device to single target, large ro device
-mkfs_crc_opts="-m crc=0"
-if [ -n "$XFS_MKFS_HAS_NO_META_SUPPORT" ]; then
- mkfs_crc_opts=""
-fi
-${MKFS_XFS_PROG} $mkfs_crc_opts -dfile,name=$imgs.source,size=100g \
+${MKFS_XFS_PROG} -dfile,name=$imgs.source,size=100g \
| _filter_mkfs 2>/dev/null
rmdir $imgs.source_dir 2>/dev/null
mkdir $imgs.source_dir
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] fstests: test xfs_copy V5 XFS without -d option
2016-09-30 5:23 [PATCH v3] fstests: test xfs_copy V5 XFS without -d option Zorro Lang
@ 2016-09-30 9:14 ` Eryu Guan
2016-09-30 11:37 ` Zorro Lang
0 siblings, 1 reply; 3+ messages in thread
From: Eryu Guan @ 2016-09-30 9:14 UTC (permalink / raw)
To: Zorro Lang; +Cc: fstests
On Fri, Sep 30, 2016 at 01:23:40PM +0800, Zorro Lang wrote:
> Before xfsprogs commit a872b62 (xfs_copy: band-aids for CRC
> filesystems), xfs_copy requires the "-d" option to copy a V5 XFS,
> because it can't rewrite the UUID of V5 XFS properly.
>
> Now xfs_copy already full support to copy a V5 XFS. But for above
> old problem, xfstests use below patch to make sure xfs_copy always
> use "-d" option to copy a V5 XFS:
>
> 8346e53 common: append -d option to XFS_COPY_PROG when testing v5 xfs
>
> That cause xfstests miss the coverage of copying a V5 XFS without
> "-d". For test this feature I did below things:
>
> 1. Change init_rc(), add "-d" to $XFS_COPY_PROG if xfs_copy can't
> copy a V5 XFS properly.
> 2. xfs/073 test V4 xfs forcibly by specify "-m crc=0" in case. I
> think it's useless now, so remove it.
> 3. remove the xfs_copy "-d" option from xfs/032
After thinking about it more, I think we lose "-d" coverage by doing so
in xfs/032. Perhaps we can do two rounds of xfs_copy test, one with "-d"
and one without it.
>
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
>
> Hi,
>
> V2:
> 1. remove require_xfs_copy() function
> 2. change the code logic of init_rc function about how to add "-d" to
> $XFS_COPY_PROG
> 3. remove xfs_copy "-d" option of xfs/032
>
> V3 add comments to explain the change in init_rc()
>
> Thanks,
> Zorro
>
> common/rc | 15 ++++++++++++---
> tests/xfs/032 | 2 +-
> tests/xfs/073 | 8 ++------
> 3 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 13afc6a..b0183b2 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3790,9 +3790,18 @@ init_rc()
> xfs_io -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
> export XFS_IO_PROG="$XFS_IO_PROG -F"
>
> - # xfs_copy doesn't work on v5 xfs yet without -d option
> - if [ "$FSTYP" == "xfs" ] && [[ $MKFS_OPTIONS =~ crc=1 ]]; then
> - export XFS_COPY_PROG="$XFS_COPY_PROG -d"
> + # xfs_copy on v5 filesystems do not require the "-d" option if xfs_db
> + # can change the UUID on v5 filesystems
> + if [ "$FSTYP" == "xfs" ]; then
> + touch $tmp.img
> + $MKFS_XFS_PROG $MKFS_OPTIONS -d file,name=$tmp.img,size=512m \
I don't think we need $MKFS_OPTIONS here.
> + >/dev/null 2>/dev/null
Ah, ">/dev/null 2>/dev/null" not ">/dev/null 2>&1", that works too but
not commonly used in this way :)
Thanks,
Eryu
> + # xfs_db will return 0 even if it can't generate a new uuid, so
> + # check the output to make sure if it can change UUID of V5 xfs
> + $XFS_DB_PROG -x -c "uuid generate" $tmp.img \
> + | grep -q "invalid UUID\|supported on V5 fs" \
> + && export XFS_COPY_PROG="$XFS_COPY_PROG -d"
> + rm -f $tmp.img
> fi
> }
>
> diff --git a/tests/xfs/032 b/tests/xfs/032
> index 6216379..0e41db8 100755
> --- a/tests/xfs/032
> +++ b/tests/xfs/032
> @@ -65,7 +65,7 @@ while [ $SECTORSIZE -le $PAGESIZE ]; do
> $FSSTRESS_PROG -n 100 -d $SCRATCH_MNT >> $seqres.full 2>&1
> _scratch_unmount
>
> - $XFS_COPY_PROG -d $SCRATCH_DEV $IMGFILE >> $seqres.full 2>&1 || \
> + $XFS_COPY_PROG $SCRATCH_DEV $IMGFILE >> $seqres.full 2>&1 || \
> _fail "Copy failed for Sector size $SECTORSIZE Block size $BLOCKSIZE"
> # Must use "-n" to get exit code; without it xfs_repair always returns 0
> $XFS_REPAIR_PROG -n -f $IMGFILE >> $seqres.full 2>&1 || \
> diff --git a/tests/xfs/073 b/tests/xfs/073
> index 9e29223..7228dd9 100755
> --- a/tests/xfs/073
> +++ b/tests/xfs/073
> @@ -138,7 +138,7 @@ _require_loop
>
> rm -f $seqres.full
>
> -_scratch_mkfs_xfs -m crc=0 -dsize=41m,agcount=2 >>$seqres.full 2>&1
> +_scratch_mkfs_xfs -dsize=41m,agcount=2 >>$seqres.full 2>&1
> _scratch_mount 2>/dev/null || _fail "initial scratch mount failed"
>
> echo
> @@ -158,11 +158,7 @@ _verify_copy $imgs.image $SCRATCH_DEV $SCRATCH_MNT
>
> echo
> echo === copying scratch device to single target, large ro device
> -mkfs_crc_opts="-m crc=0"
> -if [ -n "$XFS_MKFS_HAS_NO_META_SUPPORT" ]; then
> - mkfs_crc_opts=""
> -fi
> -${MKFS_XFS_PROG} $mkfs_crc_opts -dfile,name=$imgs.source,size=100g \
> +${MKFS_XFS_PROG} -dfile,name=$imgs.source,size=100g \
> | _filter_mkfs 2>/dev/null
> rmdir $imgs.source_dir 2>/dev/null
> mkdir $imgs.source_dir
> --
> 2.7.4
>
> --
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] fstests: test xfs_copy V5 XFS without -d option
2016-09-30 9:14 ` Eryu Guan
@ 2016-09-30 11:37 ` Zorro Lang
0 siblings, 0 replies; 3+ messages in thread
From: Zorro Lang @ 2016-09-30 11:37 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests
On Fri, Sep 30, 2016 at 05:14:37PM +0800, Eryu Guan wrote:
> On Fri, Sep 30, 2016 at 01:23:40PM +0800, Zorro Lang wrote:
> > Before xfsprogs commit a872b62 (xfs_copy: band-aids for CRC
> > filesystems), xfs_copy requires the "-d" option to copy a V5 XFS,
> > because it can't rewrite the UUID of V5 XFS properly.
> >
> > Now xfs_copy already full support to copy a V5 XFS. But for above
> > old problem, xfstests use below patch to make sure xfs_copy always
> > use "-d" option to copy a V5 XFS:
> >
> > 8346e53 common: append -d option to XFS_COPY_PROG when testing v5 xfs
> >
> > That cause xfstests miss the coverage of copying a V5 XFS without
> > "-d". For test this feature I did below things:
> >
> > 1. Change init_rc(), add "-d" to $XFS_COPY_PROG if xfs_copy can't
> > copy a V5 XFS properly.
> > 2. xfs/073 test V4 xfs forcibly by specify "-m crc=0" in case. I
> > think it's useless now, so remove it.
> > 3. remove the xfs_copy "-d" option from xfs/032
>
> After thinking about it more, I think we lose "-d" coverage by doing so
> in xfs/032. Perhaps we can do two rounds of xfs_copy test, one with "-d"
> and one without it.
Hmm, make sense, so let's keep the "-d" option, and write another case
without the -d, or as you said do two rounds of xfs_copy test.
Thanks,
Zorro
>
> >
> > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > ---
> >
> > Hi,
> >
> > V2:
> > 1. remove require_xfs_copy() function
> > 2. change the code logic of init_rc function about how to add "-d" to
> > $XFS_COPY_PROG
> > 3. remove xfs_copy "-d" option of xfs/032
> >
> > V3 add comments to explain the change in init_rc()
> >
> > Thanks,
> > Zorro
> >
> > common/rc | 15 ++++++++++++---
> > tests/xfs/032 | 2 +-
> > tests/xfs/073 | 8 ++------
> > 3 files changed, 15 insertions(+), 10 deletions(-)
> >
> > diff --git a/common/rc b/common/rc
> > index 13afc6a..b0183b2 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -3790,9 +3790,18 @@ init_rc()
> > xfs_io -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
> > export XFS_IO_PROG="$XFS_IO_PROG -F"
> >
> > - # xfs_copy doesn't work on v5 xfs yet without -d option
> > - if [ "$FSTYP" == "xfs" ] && [[ $MKFS_OPTIONS =~ crc=1 ]]; then
> > - export XFS_COPY_PROG="$XFS_COPY_PROG -d"
> > + # xfs_copy on v5 filesystems do not require the "-d" option if xfs_db
> > + # can change the UUID on v5 filesystems
> > + if [ "$FSTYP" == "xfs" ]; then
> > + touch $tmp.img
> > + $MKFS_XFS_PROG $MKFS_OPTIONS -d file,name=$tmp.img,size=512m \
>
> I don't think we need $MKFS_OPTIONS here.
I think we need it. Generally we always do _scratch_mkfs, which use
$MKFS_OPTIONS by default. If we don't use $MKFS_OPTIONS at here, some
problems will happen if test on old xfsprogs which doesn't enable CRC
by default, but the user specify MKFS_OPTIONS="-m crc=1".
And we add "-d" option for those old xfsprogs too.
Thanks,
Zorro
>
> > + >/dev/null 2>/dev/null
>
> Ah, ">/dev/null 2>/dev/null" not ">/dev/null 2>&1", that works too but
> not commonly used in this way :)
sigh... That's a mistake:)
>
> Thanks,
> Eryu
>
> > + # xfs_db will return 0 even if it can't generate a new uuid, so
> > + # check the output to make sure if it can change UUID of V5 xfs
> > + $XFS_DB_PROG -x -c "uuid generate" $tmp.img \
> > + | grep -q "invalid UUID\|supported on V5 fs" \
> > + && export XFS_COPY_PROG="$XFS_COPY_PROG -d"
> > + rm -f $tmp.img
> > fi
> > }
> >
> > diff --git a/tests/xfs/032 b/tests/xfs/032
> > index 6216379..0e41db8 100755
> > --- a/tests/xfs/032
> > +++ b/tests/xfs/032
> > @@ -65,7 +65,7 @@ while [ $SECTORSIZE -le $PAGESIZE ]; do
> > $FSSTRESS_PROG -n 100 -d $SCRATCH_MNT >> $seqres.full 2>&1
> > _scratch_unmount
> >
> > - $XFS_COPY_PROG -d $SCRATCH_DEV $IMGFILE >> $seqres.full 2>&1 || \
> > + $XFS_COPY_PROG $SCRATCH_DEV $IMGFILE >> $seqres.full 2>&1 || \
> > _fail "Copy failed for Sector size $SECTORSIZE Block size $BLOCKSIZE"
> > # Must use "-n" to get exit code; without it xfs_repair always returns 0
> > $XFS_REPAIR_PROG -n -f $IMGFILE >> $seqres.full 2>&1 || \
> > diff --git a/tests/xfs/073 b/tests/xfs/073
> > index 9e29223..7228dd9 100755
> > --- a/tests/xfs/073
> > +++ b/tests/xfs/073
> > @@ -138,7 +138,7 @@ _require_loop
> >
> > rm -f $seqres.full
> >
> > -_scratch_mkfs_xfs -m crc=0 -dsize=41m,agcount=2 >>$seqres.full 2>&1
> > +_scratch_mkfs_xfs -dsize=41m,agcount=2 >>$seqres.full 2>&1
> > _scratch_mount 2>/dev/null || _fail "initial scratch mount failed"
> >
> > echo
> > @@ -158,11 +158,7 @@ _verify_copy $imgs.image $SCRATCH_DEV $SCRATCH_MNT
> >
> > echo
> > echo === copying scratch device to single target, large ro device
> > -mkfs_crc_opts="-m crc=0"
> > -if [ -n "$XFS_MKFS_HAS_NO_META_SUPPORT" ]; then
> > - mkfs_crc_opts=""
> > -fi
> > -${MKFS_XFS_PROG} $mkfs_crc_opts -dfile,name=$imgs.source,size=100g \
> > +${MKFS_XFS_PROG} -dfile,name=$imgs.source,size=100g \
> > | _filter_mkfs 2>/dev/null
> > rmdir $imgs.source_dir 2>/dev/null
> > mkdir $imgs.source_dir
> > --
> > 2.7.4
> >
> > --
> > 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
> --
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-09-30 11:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-30 5:23 [PATCH v3] fstests: test xfs_copy V5 XFS without -d option Zorro Lang
2016-09-30 9:14 ` Eryu Guan
2016-09-30 11:37 ` Zorro Lang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.