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>]").
Thanks,
Eryu
--
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
