Re: [PATCH v4 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日,下午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 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