fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fstests: btrfs/022: Disable snapshot ioctl in fsstress
@ 2020-02-06  5:32 Qu Wenruo
  2020-02-06 15:47 ` Josef Bacik
  0 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2020-02-06  5:32 UTC (permalink / raw)
  To: fstests, linux-btrfs

Since commit fd0830929573 ("fsstress: add the ability to create
snapshots") adds the ability for fsstress to create/delete snapshot and
subvolume, test case btrfs/022 fails as _btrfs_get_subvolid can't
handle multiple subvolumes under the same path.

So manually disable snapshot/subvolume creation and deletion ioctl in this
test case. Other qgroup test cases aren't affected.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/022 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/btrfs/022 b/tests/btrfs/022
index 5348d3ed..d0ae6b01 100755
--- a/tests/btrfs/022
+++ b/tests/btrfs/022
@@ -43,6 +43,7 @@ _basic_test()
 		$seqres.full 2>&1
 	[ $? -eq 0 ] || _fail "couldn't find our subvols quota group"
 	run_check $FSSTRESS_PROG -d $SCRATCH_MNT/a -w -p 1 -n 2000 \
+		-f snapshot=0 -f subvol_create=0 -f subvol_delete=0 \
 		$FSSTRESS_AVOID
 	_run_btrfs_util_prog subvolume snapshot $SCRATCH_MNT/a \
 		$SCRATCH_MNT/b
@@ -66,6 +67,7 @@ _rescan_test()
 	_run_btrfs_util_prog quota enable $SCRATCH_MNT/a
 	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT a)
 	run_check $FSSTRESS_PROG -d $SCRATCH_MNT/a -w -p 1 -n 2000 \
+		-f snapshot=0 -f subvol_create=0 -f subvol_delete=0 \
 		$FSSTRESS_AVOID
 	sync
 	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] fstests: btrfs/022: Disable snapshot ioctl in fsstress
  2020-02-06  5:32 [PATCH] fstests: btrfs/022: Disable snapshot ioctl in fsstress Qu Wenruo
@ 2020-02-06 15:47 ` Josef Bacik
  2020-02-07  0:35   ` Qu Wenruo
  0 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2020-02-06 15:47 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs

On 2/6/20 12:32 AM, Qu Wenruo wrote:
> Since commit fd0830929573 ("fsstress: add the ability to create
> snapshots") adds the ability for fsstress to create/delete snapshot and
> subvolume, test case btrfs/022 fails as _btrfs_get_subvolid can't
> handle multiple subvolumes under the same path.
> 
> So manually disable snapshot/subvolume creation and deletion ioctl in this
> test case. Other qgroup test cases aren't affected.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Why not just fix _btrfs_get_subvolid?  You can use egrep to make sure the name 
matches exactly.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fstests: btrfs/022: Disable snapshot ioctl in fsstress
  2020-02-06 15:47 ` Josef Bacik
@ 2020-02-07  0:35   ` Qu Wenruo
  2020-02-07  0:38     ` Josef Bacik
  0 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2020-02-07  0:35 UTC (permalink / raw)
  To: Josef Bacik, fstests, linux-btrfs



On 2020/2/6 下午11:47, Josef Bacik wrote:
> On 2/6/20 12:32 AM, Qu Wenruo wrote:
>> Since commit fd0830929573 ("fsstress: add the ability to create
>> snapshots") adds the ability for fsstress to create/delete snapshot and
>> subvolume, test case btrfs/022 fails as _btrfs_get_subvolid can't
>> handle multiple subvolumes under the same path.
>>
>> So manually disable snapshot/subvolume creation and deletion ioctl in
>> this
>> test case. Other qgroup test cases aren't affected.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Why not just fix _btrfs_get_subvolid?  You can use egrep to make sure
> the name matches exactly.  Thanks,

Because we have other requirement, like limit tests.

If we have other snapshots/subvolumes, they don't have the same limit,
thus unable to test qgroup properly.

Thanks,
Qu
> 
> Josef

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fstests: btrfs/022: Disable snapshot ioctl in fsstress
  2020-02-07  0:35   ` Qu Wenruo
@ 2020-02-07  0:38     ` Josef Bacik
  2020-02-07  0:38       ` Qu Wenruo
  2020-02-07  1:39       ` Qu Wenruo
  0 siblings, 2 replies; 6+ messages in thread
From: Josef Bacik @ 2020-02-07  0:38 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs

On 2/6/20 7:35 PM, Qu Wenruo wrote:
> 
> 
> On 2020/2/6 下午11:47, Josef Bacik wrote:
>> On 2/6/20 12:32 AM, Qu Wenruo wrote:
>>> Since commit fd0830929573 ("fsstress: add the ability to create
>>> snapshots") adds the ability for fsstress to create/delete snapshot and
>>> subvolume, test case btrfs/022 fails as _btrfs_get_subvolid can't
>>> handle multiple subvolumes under the same path.
>>>
>>> So manually disable snapshot/subvolume creation and deletion ioctl in
>>> this
>>> test case. Other qgroup test cases aren't affected.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>
>> Why not just fix _btrfs_get_subvolid?  You can use egrep to make sure
>> the name matches exactly.  Thanks,
> 
> Because we have other requirement, like limit tests.
> 
> If we have other snapshots/subvolumes, they don't have the same limit,
> thus unable to test qgroup properly.
> 

That's fair, but we should also fix _btrfs_get_subvolid since we know it doesn't 
work in this case.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fstests: btrfs/022: Disable snapshot ioctl in fsstress
  2020-02-07  0:38     ` Josef Bacik
@ 2020-02-07  0:38       ` Qu Wenruo
  2020-02-07  1:39       ` Qu Wenruo
  1 sibling, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2020-02-07  0:38 UTC (permalink / raw)
  To: Josef Bacik, fstests, linux-btrfs



On 2020/2/7 上午8:38, Josef Bacik wrote:
> On 2/6/20 7:35 PM, Qu Wenruo wrote:
>>
>>
>> On 2020/2/6 下午11:47, Josef Bacik wrote:
>>> On 2/6/20 12:32 AM, Qu Wenruo wrote:
>>>> Since commit fd0830929573 ("fsstress: add the ability to create
>>>> snapshots") adds the ability for fsstress to create/delete snapshot and
>>>> subvolume, test case btrfs/022 fails as _btrfs_get_subvolid can't
>>>> handle multiple subvolumes under the same path.
>>>>
>>>> So manually disable snapshot/subvolume creation and deletion ioctl in
>>>> this
>>>> test case. Other qgroup test cases aren't affected.
>>>>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>
>>> Why not just fix _btrfs_get_subvolid?  You can use egrep to make sure
>>> the name matches exactly.  Thanks,
>>
>> Because we have other requirement, like limit tests.
>>
>> If we have other snapshots/subvolumes, they don't have the same limit,
>> thus unable to test qgroup properly.
>>
> 
> That's fair, but we should also fix _btrfs_get_subvolid since we know it
> doesn't work in this case.  Thanks,

Sure, another patch will address this soon.

Thanks,
Qu

> 
> Josef

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fstests: btrfs/022: Disable snapshot ioctl in fsstress
  2020-02-07  0:38     ` Josef Bacik
  2020-02-07  0:38       ` Qu Wenruo
@ 2020-02-07  1:39       ` Qu Wenruo
  1 sibling, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2020-02-07  1:39 UTC (permalink / raw)
  To: Josef Bacik, Qu Wenruo, fstests, linux-btrfs


[-- Attachment #1.1: Type: text/plain, Size: 1962 bytes --]



On 2020/2/7 上午8:38, Josef Bacik wrote:
> On 2/6/20 7:35 PM, Qu Wenruo wrote:
>>
>>
>> On 2020/2/6 下午11:47, Josef Bacik wrote:
>>> On 2/6/20 12:32 AM, Qu Wenruo wrote:
>>>> Since commit fd0830929573 ("fsstress: add the ability to create
>>>> snapshots") adds the ability for fsstress to create/delete snapshot and
>>>> subvolume, test case btrfs/022 fails as _btrfs_get_subvolid can't
>>>> handle multiple subvolumes under the same path.
>>>>
>>>> So manually disable snapshot/subvolume creation and deletion ioctl in
>>>> this
>>>> test case. Other qgroup test cases aren't affected.
>>>>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>
>>> Why not just fix _btrfs_get_subvolid?  You can use egrep to make sure
>>> the name matches exactly.  Thanks,
>>
>> Because we have other requirement, like limit tests.
>>
>> If we have other snapshots/subvolumes, they don't have the same limit,
>> thus unable to test qgroup properly.
>>
> 
> That's fair, but we should also fix _btrfs_get_subvolid since we know it
> doesn't work in this case.  Thanks,

My bad. It's not the limit test, it doesn't utilize fsstress at all.

It's completely the bad greping for qgroup ids.

We could have the following subvolume layouts in btrfs qgroup show output:
subvol a id=256
subvol b id=306
qgroupid         rfer         excl
--------         ----         ----
0/5             16384        16384
0/256        13914112        16384
...
0/263         3080192      2306048 << 306 matches here first
...
0/306        13914112        16384 << Then match here

Although disabling snapshot/subvolume creation solves the problem since
there will be no other subvolumes to start with, we're still not that safe.

The root fix is to grep qgroupid by "0/$subvolid", not just $subvolid.

I'll do a proer fix, and keep the snapshot/subvolume creation to take
advantage of your enhanced fsstress.

Thanks,
Qu
> 
> Josef


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-07  1:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-06  5:32 [PATCH] fstests: btrfs/022: Disable snapshot ioctl in fsstress Qu Wenruo
2020-02-06 15:47 ` Josef Bacik
2020-02-07  0:35   ` Qu Wenruo
2020-02-07  0:38     ` Josef Bacik
2020-02-07  0:38       ` Qu Wenruo
2020-02-07  1:39       ` Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).