> > 在 2017年12月14日，下午11:02，Eryu Guan <eguan@xxxxxxxxxx> 写道： > > On Thu, Dec 14, 2017 at 04:46:21PM +0200, Amir Goldstein wrote: >> On Thu, Dec 14, 2017 at 4:19 PM, Chengguang Xu <cgxu519@xxxxxxxxxx> wrote: >>> check -overlay overrides SCRATCH_MNT variable to it's own, >>> so adjust target to underlying filesystem(OVL_BASE_SCRATCH_MNT) >>> when running shutdown on overlayfs. >>> >>> Introduce a helper _scratch_shutdown to mainly handle scratch >>> shutdown and convert godown to _scratch_shutdown to support >>> overlayfs running on below cases. >> >> You should do this in 2 patches: >> 1. replace all open coded godown calls with _scratch_shutdown helper >> 2. Add overlay support to _scratch_shutdown helper. > > The description is not clear enough, the updates are not for running > existing shutdown tests to run on overlay, but to avoid test failures > using the bare 'src/godown $SCRATCH_MNT' when _require_scratch_shutdown > accepts overlay to run. e.g. > > +/root/xfstests/src/godown: error on xfsctl(GOINGDOWN) of "/mnt/scratch/ovl-mnt": Inappropriate ioctl for device > > So I think it's proper to do all the required updates in one patch, > instead of introducing (intermediate) false failures, and fixing them in > a follow-up patch. > >> >>> >>> generic/043 >>> generic/044 >>> generic/045 >>> generic/046 >>> generic/047 >>> generic/048 >>> generic/051 >>> generic/052 >>> generic/054 >>> generic/055 >>> generic/392 >>> generic/417 >>> generic/461 >>> generic/468 >> >> Really no need to specify these in change log. They already appear in diffstat. > > Agreed. > >> >>> >>> In order to avoid overlayfs running on improper shutdown >>> cases, add _require_local_device $SCRATCH_DEV to below cases. >> >> I don't understand. >> In all these tests, there is already _require_scratch_shutdown. >> Shouldn't that prevent overlay running improper shutdown? > > generic/042 and generic/050 assume we're testing against a local device > (do mkfs or operate on $SCRATCH_DEV directly), so we need additional > check on $SCRATCH_DEV. Perhaps we need some comments in each test. But I > don't see why generic/388 requires a local device too, maybe I missed > something, that's why we need comments :) > generic/388 causes underlying filesystem corruption and can’t be mounted unless running new mkfs. For local device based filesystems, it’s not a problem but overlayfs does not do real mkfs for device so the cases after this will be all failed. Even worse，there is no check about _scratch_mount result before doing shutdown，so it can cause base fs shutting down unexpectedly. For safety, I’ll add a check in _require_scratch_shutdown, but _scratch_shutdown caller still needs to check mount status carefully in their test. Thanks, Chengguang. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
