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 :) 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
