Re: [PATCH v3 1/3] common/rc: add scratch shutdown support for overlayfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> 在 2017年12月14日,下午12:32,Eryu Guan <eguan@xxxxxxxxxx> 写道:
> 
> On Thu, Dec 14, 2017 at 11:35:04AM +0800, Chengguang Xu wrote:
>> 
>>> 
>>> 在 2017年12月12日,下午5:16,Eryu Guan <eguan@xxxxxxxxxx> 写道:
>>> 
>>> On Tue, Dec 12, 2017 at 11:09:37AM +0800, Chengguang Xu wrote:
>>>> OverlayFS overrides SCRATCH_MNT variable to it's own,
>>>> so adjust target to underlying filesystem(OVL_BASE_SCRATCH_MNT)
>>>> when running shutdown on overlayfs.
>>>> 
>>>> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxx>
>>>> ---
>>>> 
>>>> Changes since v2:
>>>> 1. Make option for _scratch_shutdown(), so caller can 
>>>> specify option when calling.
>>>> 2. Add comment for why overlay requires special handling.
>>>> 3. Add commit log.
>>>> 
>>>> Changes since v1:
>>>> _scratch_shutdown() does not call notrun.
>>>> 
>>>> common/rc | 37 +++++++++++++++++++++++++++++++++++--
>>>> 1 file changed, 35 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/common/rc b/common/rc
>>>> index 4c053a5..a5c4ace 100644
>>>> --- a/common/rc
>>>> +++ b/common/rc
>>>> @@ -382,6 +382,24 @@ _scratch_cycle_mount()
>>>>    _scratch_mount "$opts"
>>>> }
>>>> 
>>>> +_scratch_shutdown()
>>>> +{
>>>> +	if [ $FSTYP = "overlay" ]; then
>>>> +		# In lagacy overlay usage, it may specify directory as 
>>> 
>>> Trailing whitespace in above line.
>>> 
>>>> +		# SCRATCH_DEV, in this case OVL_BASE_SCRATCH_DEV
>>>> +		# will be null, so check OVL_BASE_SCRATCH_DEV before
>>>> +		# running shutdown to avoid shutting down base fs accidently.
>>>> +		if [ -z $OVL_BASE_SCRATCH_DEV ]; then
>>>> +			echo -n "Ignore shutdown because "
>>>> +			echo "$SCRATCH_DEV is not a block device"
>>> 
>>> Hmm, I think a '_fail "<message>"' would be more appropriate here, so
>>> that test fails and exits immediately. Test should call
>>> _require_scratch_shutdown first before calling _scratch_shutdown, if
>>> not, it indicates a useage error of the helper, we need to prompt the
>>> test writer to call _require_scratch_shutdown first in error message.
>> 
>> 
>> The additional check I added here, is mainly for avoiding accident improper shutdown.
> 
> Yeah, I understand, and the _fail call I suggested is to exit the test
> immediately with a proper error message when such an accident usage is
> detected, while current code just prints a message and continues the
> rest of the test, which is unnecessary.
> 
>> If you think we need to prompt message to indicate should call _require_scratch_shutdown first,
>> I think we had better modify godown to implement this function. What do you think?
> 
> I just want to see a more specific & clearer error message on such a
> usage failure, so people know what to do to correct this error. Similar
> to what we do in common/rc::_require_command (_fail "usage:
> _require_command <command> [<name>]”).

I agree exit the test immediately if you get into this condition. 
But for error message let me keep it as what it is, because the 
reason of the failure is actually config problem not improper 
command usage.

Thanks,
Chengguang.


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




[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux