Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 0/3] fstests: Fix tests which checks for swapfile support
To:     Eryu Guan <guan@eryu.me>
Cc:     fstests@vger.kernel.org, anju@linux.vnet.ibm.com,
        linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org,
        linux-xfs@vger.kernel.org
References: <cover.1604000570.git.riteshh@linux.ibm.com>
 <20201101160357.GE3853@desktop>
From:   Ritesh Harjani <riteshh@linux.ibm.com>
Message-ID: <c2f91a8e-b831-52c5-7e5e-79dcc7175cd3@linux.ibm.com>
Date:   Thu, 26 Nov 2020 09:21:19 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <20201101160357.GE3853@desktop>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 11/1/20 9:33 PM, Eryu Guan wrote:
> On Fri, Oct 30, 2020 at 01:22:50AM +0530, Ritesh Harjani wrote:
>> For more details, pls refer commit msg of each patch.
>>
>> Patch-1: modifies _require_scratch_swapfile() to check swapon only for btrfs
> 
> I don't think it's a good idea, if a new fs without swapfile support is
> tested by fstests, test would get false failure, where it should
> _notrun. And making a generic requirement check to fs-specific doesn't
> seem quite right either.
> 
>> Patch-2: adds a swapfile test for fallocate files for ext4, xfs (assuming
>> both FS supports and thus should pass).
> 
> As Brian mentioned in his review, we're in the process to convert all
> shared tests to generic or fs-specific tests (very slow though), that
> said we don't want new shared tests.
> 
> I think we could whitelist fs types in _require_scratch_swapfile() and
> don't _notrun for such filesystems, something like what we did in
> _fstyp_has_non_default_seek_data_hole(), so that we won't miss silent
> regressions on sucn filesystems, and we'll do sanity check as well on
> other filesystems.

Nice idea. Let me check that part.

> 
>> Patch-3: added to support tests to run when multiple config section present
>> in local.config file.
> 
> I have a patch[1] that should fix the issue 3 years ago, but it never
> got reviewed, would you please check and see if it fixed the bug for you?
> 
> [1] https://patchwork.kernel.org/project/fstests/patch/20171117070022.14002-1-eguan@redhat.com/

Sure, will test it and get back.

Sorry about the delay from my end on this patch series. I got pulled
into something else. Let me work on your suggestions.


-ritesh