* [PATCH 1/2] common/rc: Add syncfs check and a helper _scratch_shutdown()
@ 2017-12-05 15:50 Chengguang Xu
2017-12-05 16:13 ` Amir Goldstein
0 siblings, 1 reply; 6+ messages in thread
From: Chengguang Xu @ 2017-12-05 15:50 UTC (permalink / raw)
To: Eryu Guan, Amir Goldstein; +Cc: fstests, overlayfs, Chengguang Xu
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)
+ 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.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] common/rc: Add syncfs check and a helper _scratch_shutdown()
2017-12-05 15:50 [PATCH 1/2] common/rc: Add syncfs check and a helper _scratch_shutdown() Chengguang Xu
@ 2017-12-05 16:13 ` Amir Goldstein
2017-12-05 17:14 ` Eryu Guan
0 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2017-12-05 16:13 UTC (permalink / raw)
To: Chengguang Xu; +Cc: Eryu Guan, fstests, overlayfs
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.
> + 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.8.3.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] common/rc: Add syncfs check and a helper _scratch_shutdown()
2017-12-05 16:13 ` Amir Goldstein
@ 2017-12-05 17:14 ` Eryu Guan
2017-12-06 0:59 ` Chengguang Xu
0 siblings, 1 reply; 6+ messages in thread
From: Eryu Guan @ 2017-12-05 17:14 UTC (permalink / raw)
To: Amir Goldstein, Chengguang Xu; +Cc: fstests, overlayfs
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] common/rc: Add syncfs check and a helper _scratch_shutdown()
2017-12-05 17:14 ` Eryu Guan
@ 2017-12-06 0:59 ` Chengguang Xu
2017-12-06 6:40 ` Amir Goldstein
0 siblings, 1 reply; 6+ messages in thread
From: Chengguang Xu @ 2017-12-06 0:59 UTC (permalink / raw)
To: Eryu Guan; +Cc: Amir Goldstein, fstests, overlayfs
> 在 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] common/rc: Add syncfs check and a helper _scratch_shutdown()
2017-12-06 0:59 ` Chengguang Xu
@ 2017-12-06 6:40 ` Amir Goldstein
2017-12-06 12:26 ` Chengguang Xu
0 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2017-12-06 6:40 UTC (permalink / raw)
To: Chengguang Xu; +Cc: Eryu Guan, fstests, overlayfs
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] common/rc: Add syncfs check and a helper _scratch_shutdown()
2017-12-06 6:40 ` Amir Goldstein
@ 2017-12-06 12:26 ` Chengguang Xu
0 siblings, 0 replies; 6+ messages in thread
From: Chengguang Xu @ 2017-12-06 12:26 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Eryu Guan, fstests, overlayfs
>
> 在 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-06 12:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-05 15:50 [PATCH 1/2] common/rc: Add syncfs check and a helper _scratch_shutdown() Chengguang Xu
2017-12-05 16:13 ` Amir Goldstein
2017-12-05 17:14 ` Eryu Guan
2017-12-06 0:59 ` Chengguang Xu
2017-12-06 6:40 ` Amir Goldstein
2017-12-06 12:26 ` Chengguang Xu
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.