| Message ID | 2D8A66F2-6AB9-4B7D-90B0-D3B6807FB353@icloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 05, 2017 at 06:13:35PM +0200, Amir Goldstein wrote: > On Tue, Dec 5, 2017 at 5:50 PM, Chengguang Xu <cgxu519@icloud.com> wrote: > > 1. Add a check case in _require_xfs_io_command() to support syncfs. > > 2. Introduce a helper to support scratch shutdown for overlayfs. > > > > Signed-off-by: Chengguang Xu <cgxu519@icloud.com> > > --- > > common/rc | 22 ++++++++++++++++++++-- > > 1 file changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/common/rc b/common/rc > > index 4c053a5..e36ee24 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -669,6 +669,21 @@ _scratch_cleanup_files() > > esac > > } > > > > +_scratch_shutdown() > > +{ > > + > > + case $FSTYP in > > + overlay) > > Looks good, > but first you need to check for [ -z "$OVL_BASE_SCRATCH_MNT" ] > meaning that tester is using "old" overlay config and we are not allowed to > mess with base fs. Agreed, we don't want to shutdown the root fs accidently. > > > + src/godown -f $OVL_BASE_SCRATCH_MNT 2>&1 \ > > + || _notrun "Underlying filesystem does not support shutdown" > > + ;; > > + *) > > + src/godown -f $SCRATCH_MNT 2>&1 \ > > + || _notrun "$FSTYP does not support shutdown" > > + ;; > > + esac > > +} I think this _scratch_shutdown() should be folded into _require_scratch_shutdown(). The name _scratch_shutdown indicates it shuts down the scratch device, and call _notrun from it would be a surprise to users. > > + > > _scratch_mkfs() > > { > > local mkfs_cmd="" > > @@ -2087,6 +2102,10 @@ _require_xfs_io_command() > > "utimes" ) > > testio=`$XFS_IO_PROG -f -c "utimes" 0 0 0 0 $testfile 2>&1` > > ;; > > + "syncfs") > > + touch $testfile > > + testio=`$XFS_IO_PROG -c "syncfs" $testfile 2>&1` > > + ;; > > *) This part should be in a separate patch. Thanks, Eryu > > testio=`$XFS_IO_PROG -c "help $command" 2>&1` > > esac > > @@ -2908,8 +2927,7 @@ _require_scratch_shutdown() > > > > _scratch_mkfs > /dev/null 2>&1 > > _scratch_mount > > - src/godown -f $SCRATCH_MNT 2>&1 \ > > - || _notrun "$FSTYP does not support shutdown" > > + _scratch_shutdown > > _scratch_unmount > > } > > > > -- > > 1.8.3.1 -- 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
> 在 2017年12月6日，上午1:14，Eryu Guan <eguan@redhat.com> 写道： > > On Tue, Dec 05, 2017 at 06:13:35PM +0200, Amir Goldstein wrote: >> On Tue, Dec 5, 2017 at 5:50 PM, Chengguang Xu <cgxu519@icloud.com> wrote: >>> 1. Add a check case in _require_xfs_io_command() to support syncfs. >>> 2. Introduce a helper to support scratch shutdown for overlayfs. >>> >>> Signed-off-by: Chengguang Xu <cgxu519@icloud.com> >>> --- >>> common/rc | 22 ++++++++++++++++++++-- >>> 1 file changed, 20 insertions(+), 2 deletions(-) >>> >>> diff --git a/common/rc b/common/rc >>> index 4c053a5..e36ee24 100644 >>> --- a/common/rc >>> +++ b/common/rc >>> @@ -669,6 +669,21 @@ _scratch_cleanup_files() >>> esac >>> } >>> >>> +_scratch_shutdown() >>> +{ >>> + >>> + case $FSTYP in >>> + overlay) >> >> Looks good, >> but first you need to check for [ -z "$OVL_BASE_SCRATCH_MNT" ] >> meaning that tester is using "old" overlay config and we are not allowed to >> mess with base fs. > > Agreed, we don't want to shutdown the root fs accidently. If "$OVL_BASE_SCRATCH_MNT” is nothing, godown command should fail for improper param, right? I don’t clearly know how it makes rootfs shutdown. you mean I need to check for [ "$OVL_BASE_SCRATCH_MNT” = “/“ ] ? > >> >>> + src/godown -f $OVL_BASE_SCRATCH_MNT 2>&1 \ >>> + || _notrun "Underlying filesystem does not support shutdown" >>> + ;; >>> + *) >>> + src/godown -f $SCRATCH_MNT 2>&1 \ >>> + || _notrun "$FSTYP does not support shutdown" >>> + ;; >>> + esac >>> +} > > I think this _scratch_shutdown() should be folded into > _require_scratch_shutdown(). The name _scratch_shutdown indicates it > shuts down the scratch device, and call _notrun from it would be a > surprise to users. > >>> + >>> _scratch_mkfs() >>> { >>> local mkfs_cmd="" >>> @@ -2087,6 +2102,10 @@ _require_xfs_io_command() >>> "utimes" ) >>> testio=`$XFS_IO_PROG -f -c "utimes" 0 0 0 0 $testfile 2>&1` >>> ;; >>> + "syncfs") >>> + touch $testfile >>> + testio=`$XFS_IO_PROG -c "syncfs" $testfile 2>&1` >>> + ;; >>> *) > > This part should be in a separate patch. > > Thanks, > Eryu > >>> testio=`$XFS_IO_PROG -c "help $command" 2>&1` >>> esac >>> @@ -2908,8 +2927,7 @@ _require_scratch_shutdown() >>> >>> _scratch_mkfs > /dev/null 2>&1 >>> _scratch_mount >>> - src/godown -f $SCRATCH_MNT 2>&1 \ >>> - || _notrun "$FSTYP does not support shutdown" >>> + _scratch_shutdown >>> _scratch_unmount >>> } >>> >>> — >>> 1.8.3.1 Thanks, cgxu -- 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
On Wed, Dec 6, 2017 at 2:59 AM, Chengguang Xu <cgxu519@icloud.com> wrote: > >> 在 2017年12月6日，上午1:14，Eryu Guan <eguan@redhat.com> 写道： >> >> On Tue, Dec 05, 2017 at 06:13:35PM +0200, Amir Goldstein wrote: >>> On Tue, Dec 5, 2017 at 5:50 PM, Chengguang Xu <cgxu519@icloud.com> wrote: >>>> 1. Add a check case in _require_xfs_io_command() to support syncfs. >>>> 2. Introduce a helper to support scratch shutdown for overlayfs. >>>> >>>> Signed-off-by: Chengguang Xu <cgxu519@icloud.com> >>>> --- >>>> common/rc | 22 ++++++++++++++++++++-- >>>> 1 file changed, 20 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/common/rc b/common/rc >>>> index 4c053a5..e36ee24 100644 >>>> --- a/common/rc >>>> +++ b/common/rc >>>> @@ -669,6 +669,21 @@ _scratch_cleanup_files() >>>> esac >>>> } >>>> >>>> +_scratch_shutdown() >>>> +{ >>>> + >>>> + case $FSTYP in >>>> + overlay) >>> >>> Looks good, >>> but first you need to check for [ -z "$OVL_BASE_SCRATCH_MNT" ] >>> meaning that tester is using "old" overlay config and we are not allowed to >>> mess with base fs. >> >> Agreed, we don't want to shutdown the root fs accidently. > > If "$OVL_BASE_SCRATCH_MNT” is nothing, godown command should fail for improper param, right? > I don’t clearly know how it makes rootfs shutdown. > you mean I need to check for [ "$OVL_BASE_SCRATCH_MNT” = “/“ ] ? > Sorry, I meant you need to check if OVL_BASE_SCRATCH_DEV is not empty. OVL_BASE_SCRATCH_MNT will always be valid dir, but either user set it with old configuration, e.g.: SCRATCH_DEV=/my/ovl/root FSTYP=overlay OR xfstests mounted a base fs with new overlay configuration, e.g.: SCRATCH_DEV=/dev/my-ovl-bdev SCRATCH_MNT=/my/ovl/root FSTYP=xfs If you shutdown OVL_BASE_SCRATCH_MNT in the first case, you may shutdown the fs of the host running the test. Amir. -- 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
> > 在 2017年12月6日，下午2:40，Amir Goldstein <amir73il@gmail.com> 写道： > > On Wed, Dec 6, 2017 at 2:59 AM, Chengguang Xu <cgxu519@icloud.com> wrote: >> >>> 在 2017年12月6日，上午1:14，Eryu Guan <eguan@redhat.com> 写道： >>> >>> On Tue, Dec 05, 2017 at 06:13:35PM +0200, Amir Goldstein wrote: >>>> On Tue, Dec 5, 2017 at 5:50 PM, Chengguang Xu <cgxu519@icloud.com> wrote: >>>>> 1. Add a check case in _require_xfs_io_command() to support syncfs. >>>>> 2. Introduce a helper to support scratch shutdown for overlayfs. >>>>> >>>>> Signed-off-by: Chengguang Xu <cgxu519@icloud.com> >>>>> --- >>>>> common/rc | 22 ++++++++++++++++++++-- >>>>> 1 file changed, 20 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/common/rc b/common/rc >>>>> index 4c053a5..e36ee24 100644 >>>>> --- a/common/rc >>>>> +++ b/common/rc >>>>> @@ -669,6 +669,21 @@ _scratch_cleanup_files() >>>>> esac >>>>> } >>>>> >>>>> +_scratch_shutdown() >>>>> +{ >>>>> + >>>>> + case $FSTYP in >>>>> + overlay) >>>> >>>> Looks good, >>>> but first you need to check for [ -z "$OVL_BASE_SCRATCH_MNT" ] >>>> meaning that tester is using "old" overlay config and we are not allowed to >>>> mess with base fs. >>> >>> Agreed, we don't want to shutdown the root fs accidently. >> >> If "$OVL_BASE_SCRATCH_MNT” is nothing, godown command should fail for improper param, right? >> I don’t clearly know how it makes rootfs shutdown. >> you mean I need to check for [ "$OVL_BASE_SCRATCH_MNT” = “/“ ] ? >> > > Sorry, I meant you need to check if OVL_BASE_SCRATCH_DEV is not empty. > OVL_BASE_SCRATCH_MNT will always be valid dir, but either user set it > with old configuration, e.g.: > SCRATCH_DEV=/my/ovl/root > FSTYP=overlay > > OR xfstests mounted a base fs with new overlay configuration, e.g.: > SCRATCH_DEV=/dev/my-ovl-bdev > SCRATCH_MNT=/my/ovl/root > FSTYP=xfs > > If you shutdown OVL_BASE_SCRATCH_MNT in the first case, you may > shutdown the fs of the host running the test. I’ve got your point now, thanks. Thanks, Chengguang.-- 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 --git a/common/rc b/common/rc index 4c053a5..e36ee24 100644 --- a/common/rc +++ b/common/rc @@ -669,6 +669,21 @@ _scratch_cleanup_files() esac } +_scratch_shutdown() +{ + + case $FSTYP in + overlay) + src/godown -f $OVL_BASE_SCRATCH_MNT 2>&1 \ + || _notrun "Underlying filesystem does not support shutdown" + ;; + *) + src/godown -f $SCRATCH_MNT 2>&1 \ + || _notrun "$FSTYP does not support shutdown" + ;; + esac +} + _scratch_mkfs() { local mkfs_cmd="" @@ -2087,6 +2102,10 @@ _require_xfs_io_command() "utimes" ) testio=`$XFS_IO_PROG -f -c "utimes" 0 0 0 0 $testfile 2>&1` ;; + "syncfs") + touch $testfile + testio=`$XFS_IO_PROG -c "syncfs" $testfile 2>&1` + ;; *) testio=`$XFS_IO_PROG -c "help $command" 2>&1` esac @@ -2908,8 +2927,7 @@ _require_scratch_shutdown() _scratch_mkfs > /dev/null 2>&1 _scratch_mount - src/godown -f $SCRATCH_MNT 2>&1 \ - || _notrun "$FSTYP does not support shutdown" + _scratch_shutdown _scratch_unmount }
1. Add a check case in _require_xfs_io_command() to support syncfs. 2. Introduce a helper to support scratch shutdown for overlayfs. Signed-off-by: Chengguang Xu <cgxu519@icloud.com> --- common/rc | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)