On Thu, Jan 4, 2018 at 12:20 AM, Eryu Guan <eguan@xxxxxxxxxx> wrote:
> On Thu, Jan 04, 2018 at 10:09:08AM +0200, Amir Goldstein wrote:
>> On Thu, Jan 4, 2018 at 3:18 AM, harshads <harshads@xxxxxxxxxx> wrote:
>> > Add tests to verify Ext4 online resizing feature with bigalloc feature
>> > enabled. We test various resizing scenarios with different cluster
>> > sizes.
>> >
>> > Signed-off-by: Harshad Shirwadkar <harshads@xxxxxxxxxx>
>> > ---
>> > common/rc | 23 ++++++++
>> > tests/ext4/030 | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> > tests/ext4/030.out | 148 +++++++++++++++++++++++++++++++++++++++++++++++++
>> > tests/ext4/group | 1 +
>> > 4 files changed, 330 insertions(+)
>> > create mode 100755 tests/ext4/030
>> > create mode 100644 tests/ext4/030.out
>> >
>> > diff --git a/common/rc b/common/rc
>> > index 9216efdb..052dadae 100644
>> > --- a/common/rc
>> > +++ b/common/rc
>> > @@ -1845,6 +1845,29 @@ _require_scratch_ext4_feature()
>> > _scratch_unmount
>> > }
>> >
>> > +# Check whether the specified feature whether it is supported by
>> > +# mkfs.ext4 and the kernel by using a sparse file image.
>> > +_require_ext4_feature()
>>
>> 1. please explain why this loop variant is needed
As discussed on a previous thread, the loop variant will avoid the
need to mkfs twice on scratch device (once in rule and once in test).
Also, this particular test doesn't need scratch device at all. So, if
I decide to use _require_scratch_ext4_feature, I will have to do
_require_scratch just for the rule even though the actual test doesn't
need scratch. So, having loop variant helps.
>> 2. it would be great if you could also change callers to
>> _require_scratch_ext4_feature
>> to use _require_scratch_feature and plug
>> _require_scratch_ext4_feature in there
Okay, that sounds good.
>> 3. probably best to post this as a separate patch from the test itself
Okay.
>
> I haven't went through the whole patch yet, just want to point out that
> the _require_scratch_ext4_feature helper has been added in commit
> be341e36fd02 ("common: rework _require_ext4_mkfs_feature"), but not
> enabled in the more generic _require_scratch_feature helper yet.
>
> It'd be good to plug the ext4 helper to the generic helper in a separate
> patch, as Amir suggested, and perhaps converting all existing callers of
> the ext4 helper to the generic helper.
>
> Thanks,
> Eryu
>
>>
>> > +{
>> > + if [ -z "$1" ]; then
>> > + echo "Usage: _require_loop_ext4_feature feature"
>>
>> So which is it, _require_loop_ext4_feature or _require_ext4_feature?
>> First one sounds better to me, given that you explain why the loop
>> variant is needed.
>> If it is needed, will it be useful to have for other fs?
>> Then better implement _require_loop_feature and make ext4
>> a specific case.
Thanks for pointing this out. I agree that _require_loop_ext4_feature
is better. Sure, I'll add a _require_loop_feature. Alright, I'll do
_require_loop stuff in a different patch.
- Harshad.
>>
>> Cheers,
>> Amir.
--
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
