Message ID | 20240423220656.4994-1-anand.jain@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [GIT,PULL] fstests: btrfs changes staged-20240418 | expand |
On Wed, Apr 24, 2024 at 06:06:43AM +0800, Anand Jain wrote: > (I just realized that the previous attempt to send this PR failed. Resending it now.) > > Zorro, > > Several of the btrfs test cases were failing due to a change in the golden > output. The commits here fix them. These patches are on top of the last PR > branch staged-20240414. Hi Anand, I found lots of patches in this branch doesn't have RVB. That's not safe, if we always do things like that. We need one single peer review at least, that requirement is low enough I think. Better to ping btrfs-list or fstests-list or particular reviewers to get review, if some patches missed RVB. Thanks, Zorro > > Thank you. > > The following changes since commit 943bbbc1ce0a3f8af862a7f9f11ecec00146edfe: > > btrfs: remove useless comments (2024-04-14 08:38:14 +0800) > > are available in the Git repository at: > > https://github.com/asj/fstests.git staged-20240418 > > for you to fetch changes up to 6fc18c4142c9470013dae598cdc29a2f67887a94: > > fstests: btrfs: use _btrfs for 'subvolume snapshot' command (2024-04-18 20:16:21 +0800) > > ---------------------------------------------------------------- > Qu Wenruo (2): > fstests: btrfs: rename _run_btrfs_util_prog to _btrfs > fstests: btrfs: use _btrfs for 'subvolume snapshot' command > > common/btrfs | 15 ++++++++------- > tests/btrfs/001 | 2 +- > tests/btrfs/001.out | 1 - > tests/btrfs/004 | 2 +- > tests/btrfs/007 | 6 +++--- > tests/btrfs/011 | 10 +++++----- > tests/btrfs/017 | 6 +++--- > tests/btrfs/022 | 6 +++--- > tests/btrfs/025 | 20 ++++++++++---------- > tests/btrfs/028 | 2 +- > tests/btrfs/030 | 12 ++++++------ > tests/btrfs/034 | 12 ++++++------ > tests/btrfs/038 | 20 ++++++++++---------- > tests/btrfs/039 | 12 ++++++------ > tests/btrfs/040 | 12 ++++++------ > tests/btrfs/041 | 2 +- > tests/btrfs/042 | 10 +++++----- > tests/btrfs/043 | 12 ++++++------ > tests/btrfs/044 | 12 ++++++------ > tests/btrfs/045 | 12 ++++++------ > tests/btrfs/046 | 14 +++++++------- > tests/btrfs/048 | 16 ++++++++-------- > tests/btrfs/050 | 6 +++--- > tests/btrfs/051 | 6 +++--- > tests/btrfs/052 | 2 +- > tests/btrfs/053 | 12 ++++++------ > tests/btrfs/054 | 18 +++++++++--------- > tests/btrfs/057 | 6 +++--- > tests/btrfs/058 | 4 ++-- > tests/btrfs/077 | 12 ++++++------ > tests/btrfs/080 | 2 +- > tests/btrfs/083 | 12 ++++++------ > tests/btrfs/084 | 12 ++++++------ > tests/btrfs/085 | 4 ++-- > tests/btrfs/087 | 12 ++++++------ > tests/btrfs/090 | 2 +- > tests/btrfs/091 | 8 ++++---- > tests/btrfs/092 | 12 ++++++------ > tests/btrfs/094 | 12 ++++++------ > tests/btrfs/097 | 12 ++++++------ > tests/btrfs/099 | 4 ++-- > tests/btrfs/100 | 6 +++--- > tests/btrfs/101 | 6 +++--- > tests/btrfs/104 | 10 +++++----- > tests/btrfs/105 | 14 +++++++------- > tests/btrfs/108 | 6 +++--- > tests/btrfs/109 | 6 +++--- > tests/btrfs/110 | 16 ++++++++-------- > tests/btrfs/111 | 20 ++++++++++---------- > tests/btrfs/117 | 18 +++++++++--------- > tests/btrfs/118 | 8 ++++---- > tests/btrfs/119 | 6 +++--- > tests/btrfs/120 | 4 ++-- > tests/btrfs/121 | 2 +- > tests/btrfs/122 | 10 +++++----- > tests/btrfs/123 | 2 +- > tests/btrfs/124 | 10 +++++----- > tests/btrfs/125 | 18 +++++++++--------- > tests/btrfs/126 | 4 ++-- > tests/btrfs/127 | 12 ++++++------ > tests/btrfs/128 | 12 ++++++------ > tests/btrfs/129 | 12 ++++++------ > tests/btrfs/130 | 2 +- > tests/btrfs/139 | 6 +++--- > tests/btrfs/152 | 14 ++++++-------- > tests/btrfs/152.out | 2 -- > tests/btrfs/153 | 4 ++-- > tests/btrfs/161 | 4 ++-- > tests/btrfs/162 | 6 +++--- > tests/btrfs/163 | 12 ++++++------ > tests/btrfs/164 | 12 ++++++------ > tests/btrfs/166 | 2 +- > tests/btrfs/167 | 2 +- > tests/btrfs/168 | 6 ++---- > tests/btrfs/168.out | 2 -- > tests/btrfs/169 | 6 ++---- > tests/btrfs/169.out | 2 -- > tests/btrfs/170 | 3 +-- > tests/btrfs/170.out | 1 - > tests/btrfs/187 | 6 ++---- > tests/btrfs/187.out | 2 -- > tests/btrfs/188 | 6 ++---- > tests/btrfs/188.out | 2 -- > tests/btrfs/189 | 6 ++---- > tests/btrfs/189.out | 2 -- > tests/btrfs/191 | 6 ++---- > tests/btrfs/191.out | 2 -- > tests/btrfs/200 | 6 ++---- > tests/btrfs/200.out | 2 -- > tests/btrfs/202 | 3 +-- > tests/btrfs/202.out | 1 - > tests/btrfs/203 | 6 ++---- > tests/btrfs/203.out | 2 -- > tests/btrfs/218 | 2 +- > tests/btrfs/226 | 3 +-- > tests/btrfs/226.out | 1 - > tests/btrfs/272 | 14 +++++++------- > tests/btrfs/273 | 6 +++--- > tests/btrfs/276 | 2 +- > tests/btrfs/276.out | 1 - > tests/btrfs/278 | 14 +++++++------- > tests/btrfs/280 | 2 +- > tests/btrfs/280.out | 1 - > tests/btrfs/281 | 3 +-- > tests/btrfs/281.out | 1 - > tests/btrfs/283 | 3 +-- > tests/btrfs/283.out | 1 - > tests/btrfs/287 | 6 ++---- > tests/btrfs/287.out | 2 -- > tests/btrfs/293 | 4 ++-- > tests/btrfs/293.out | 2 -- > tests/btrfs/300 | 2 +- > tests/btrfs/300.out | 1 - > tests/btrfs/302 | 3 +-- > tests/btrfs/302.out | 1 - > tests/btrfs/314 | 3 +-- > tests/btrfs/314.out | 2 -- > tests/btrfs/320 | 16 ++++++++-------- > 118 files changed, 375 insertions(+), 435 deletions(-) >
On Wed, Apr 24, 2024 at 05:12:43PM +0800, Zorro Lang wrote: > On Wed, Apr 24, 2024 at 06:06:43AM +0800, Anand Jain wrote: > > (I just realized that the previous attempt to send this PR failed. Resending it now.) > > > > Zorro, > > > > Several of the btrfs test cases were failing due to a change in the golden > > output. The commits here fix them. These patches are on top of the last PR > > branch staged-20240414. > > Hi Anand, > > I found lots of patches in this branch doesn't have RVB. That's not safe, if > we always do things like that. We need one single peer review at least, that > requirement is low enough I think. > > Better to ping btrfs-list or fstests-list or particular reviewers to get > review, if some patches missed RVB. Anand is maintainer within fstests and I guess he reviews the patches when putting them to the branch for merge. Filipe is mentioned as reviewer but please don't expect him to reivew each and every patch. I have a feeling that you're following process of merging patches that is maybe modeled after linux kernel, with the multiple branches and even merge window (mentioned in previsous PR), but this is IMO inadequate for a testsuite where we need quick fixups to test cases to be released in a much shorter turnaround. I was expecting that if there was a dedicated maintainer for a filesystem then things would go smoothly and we could skip formalities because the maintainer is expected to do reviews that count too.
On Wed, Apr 24, 2024 at 02:14:36PM +0200, David Sterba wrote: > On Wed, Apr 24, 2024 at 05:12:43PM +0800, Zorro Lang wrote: > > On Wed, Apr 24, 2024 at 06:06:43AM +0800, Anand Jain wrote: > > > (I just realized that the previous attempt to send this PR failed. Resending it now.) > > > > > > Zorro, > > > > > > Several of the btrfs test cases were failing due to a change in the golden > > > output. The commits here fix them. These patches are on top of the last PR > > > branch staged-20240414. > > > > Hi Anand, > > > > I found lots of patches in this branch doesn't have RVB. That's not safe, if > > we always do things like that. We need one single peer review at least, that > > requirement is low enough I think. > > > > Better to ping btrfs-list or fstests-list or particular reviewers to get > > review, if some patches missed RVB. > > Anand is maintainer within fstests and I guess he reviews the patches > when putting them to the branch for merge. Filipe is mentioned as > reviewer but please don't expect him to reivew each and every patch. Hi David, Sure, mostly I trust the pull request from Anand. But this time those patches are from himself, can we say "I've reviewed the patches from myself"? Is that an acceptable work process? In other words, should maintainers have the privilege to merge his own patches without any other review? I'm a bit confused, if that's acceptable by all of you, especially we just through the "xz backdoor". > > I have a feeling that you're following process of merging patches that > is maybe modeled after linux kernel, with the multiple branches and > even merge window (mentioned in previsous PR), but this is IMO > inadequate for a testsuite where we need quick fixups to test cases to > be released in a much shorter turnaround. I try to learn it, but won't follow it totally. Due to fstests is not linux project, it's small and fast, we can accept quick fixes, but not without control. I'm not removing all the patches from his PR. I've merged those patches which got reviewed. For those un-reviewed patches, I'll try to deal with them in these 2 days, and try to help to catch the next release. > > I was expecting that if there was a dedicated maintainer for a > filesystem then things would go smoothly and we could skip formalities > because the maintainer is expected to do reviews that count too. Anand is already particular for btrfs part of fstests. But this time the patches are from him. Anyway, I'll help to review his patches, and merge them if no more other opinions from btrfs-list. Thanks, Zorro >
On Wed, Apr 24, 2024 at 06:06:43AM +0800, Anand Jain wrote: > (I just realized that the previous attempt to send this PR failed. Resending it now.) > > Zorro, > > Several of the btrfs test cases were failing due to a change in the golden > output. The commits here fix them. These patches are on top of the last PR > branch staged-20240414. Hi Anand, Thanks for working on this! Now I've merged all 12 patches with below changes: I added your RVB to: fstests: btrfs: use _btrfs for 'subvolume snapshot' command due to I trust you've reviewed it as you'd like to push it. Then I reviewed those 4 patches from you (refer to mail list): generic: move btrfs clone device testcase to the generic group common/verity: fix btrfs-corrupt-block -v option btrfs/290: fix btrfs_corrupt_block options common/btrfs: refactor _require_btrfs_corrupt_block to check option and merged these 4 patches with the review points (to save time). Please check the "patches-in-queue" branch of upstream fstests. If you (or other btrfs folks) feel anything wrong, feel free to tell me. If no more changes are needed, you'll see them in next release :) Thanks, Zorro > > Thank you. > > The following changes since commit 943bbbc1ce0a3f8af862a7f9f11ecec00146edfe: > > btrfs: remove useless comments (2024-04-14 08:38:14 +0800) > > are available in the Git repository at: > > https://github.com/asj/fstests.git staged-20240418 > > for you to fetch changes up to 6fc18c4142c9470013dae598cdc29a2f67887a94: > > fstests: btrfs: use _btrfs for 'subvolume snapshot' command (2024-04-18 20:16:21 +0800) > > ---------------------------------------------------------------- > Qu Wenruo (2): > fstests: btrfs: rename _run_btrfs_util_prog to _btrfs > fstests: btrfs: use _btrfs for 'subvolume snapshot' command > > common/btrfs | 15 ++++++++------- > tests/btrfs/001 | 2 +- > tests/btrfs/001.out | 1 - > tests/btrfs/004 | 2 +- > tests/btrfs/007 | 6 +++--- > tests/btrfs/011 | 10 +++++----- > tests/btrfs/017 | 6 +++--- > tests/btrfs/022 | 6 +++--- > tests/btrfs/025 | 20 ++++++++++---------- > tests/btrfs/028 | 2 +- > tests/btrfs/030 | 12 ++++++------ > tests/btrfs/034 | 12 ++++++------ > tests/btrfs/038 | 20 ++++++++++---------- > tests/btrfs/039 | 12 ++++++------ > tests/btrfs/040 | 12 ++++++------ > tests/btrfs/041 | 2 +- > tests/btrfs/042 | 10 +++++----- > tests/btrfs/043 | 12 ++++++------ > tests/btrfs/044 | 12 ++++++------ > tests/btrfs/045 | 12 ++++++------ > tests/btrfs/046 | 14 +++++++------- > tests/btrfs/048 | 16 ++++++++-------- > tests/btrfs/050 | 6 +++--- > tests/btrfs/051 | 6 +++--- > tests/btrfs/052 | 2 +- > tests/btrfs/053 | 12 ++++++------ > tests/btrfs/054 | 18 +++++++++--------- > tests/btrfs/057 | 6 +++--- > tests/btrfs/058 | 4 ++-- > tests/btrfs/077 | 12 ++++++------ > tests/btrfs/080 | 2 +- > tests/btrfs/083 | 12 ++++++------ > tests/btrfs/084 | 12 ++++++------ > tests/btrfs/085 | 4 ++-- > tests/btrfs/087 | 12 ++++++------ > tests/btrfs/090 | 2 +- > tests/btrfs/091 | 8 ++++---- > tests/btrfs/092 | 12 ++++++------ > tests/btrfs/094 | 12 ++++++------ > tests/btrfs/097 | 12 ++++++------ > tests/btrfs/099 | 4 ++-- > tests/btrfs/100 | 6 +++--- > tests/btrfs/101 | 6 +++--- > tests/btrfs/104 | 10 +++++----- > tests/btrfs/105 | 14 +++++++------- > tests/btrfs/108 | 6 +++--- > tests/btrfs/109 | 6 +++--- > tests/btrfs/110 | 16 ++++++++-------- > tests/btrfs/111 | 20 ++++++++++---------- > tests/btrfs/117 | 18 +++++++++--------- > tests/btrfs/118 | 8 ++++---- > tests/btrfs/119 | 6 +++--- > tests/btrfs/120 | 4 ++-- > tests/btrfs/121 | 2 +- > tests/btrfs/122 | 10 +++++----- > tests/btrfs/123 | 2 +- > tests/btrfs/124 | 10 +++++----- > tests/btrfs/125 | 18 +++++++++--------- > tests/btrfs/126 | 4 ++-- > tests/btrfs/127 | 12 ++++++------ > tests/btrfs/128 | 12 ++++++------ > tests/btrfs/129 | 12 ++++++------ > tests/btrfs/130 | 2 +- > tests/btrfs/139 | 6 +++--- > tests/btrfs/152 | 14 ++++++-------- > tests/btrfs/152.out | 2 -- > tests/btrfs/153 | 4 ++-- > tests/btrfs/161 | 4 ++-- > tests/btrfs/162 | 6 +++--- > tests/btrfs/163 | 12 ++++++------ > tests/btrfs/164 | 12 ++++++------ > tests/btrfs/166 | 2 +- > tests/btrfs/167 | 2 +- > tests/btrfs/168 | 6 ++---- > tests/btrfs/168.out | 2 -- > tests/btrfs/169 | 6 ++---- > tests/btrfs/169.out | 2 -- > tests/btrfs/170 | 3 +-- > tests/btrfs/170.out | 1 - > tests/btrfs/187 | 6 ++---- > tests/btrfs/187.out | 2 -- > tests/btrfs/188 | 6 ++---- > tests/btrfs/188.out | 2 -- > tests/btrfs/189 | 6 ++---- > tests/btrfs/189.out | 2 -- > tests/btrfs/191 | 6 ++---- > tests/btrfs/191.out | 2 -- > tests/btrfs/200 | 6 ++---- > tests/btrfs/200.out | 2 -- > tests/btrfs/202 | 3 +-- > tests/btrfs/202.out | 1 - > tests/btrfs/203 | 6 ++---- > tests/btrfs/203.out | 2 -- > tests/btrfs/218 | 2 +- > tests/btrfs/226 | 3 +-- > tests/btrfs/226.out | 1 - > tests/btrfs/272 | 14 +++++++------- > tests/btrfs/273 | 6 +++--- > tests/btrfs/276 | 2 +- > tests/btrfs/276.out | 1 - > tests/btrfs/278 | 14 +++++++------- > tests/btrfs/280 | 2 +- > tests/btrfs/280.out | 1 - > tests/btrfs/281 | 3 +-- > tests/btrfs/281.out | 1 - > tests/btrfs/283 | 3 +-- > tests/btrfs/283.out | 1 - > tests/btrfs/287 | 6 ++---- > tests/btrfs/287.out | 2 -- > tests/btrfs/293 | 4 ++-- > tests/btrfs/293.out | 2 -- > tests/btrfs/300 | 2 +- > tests/btrfs/300.out | 1 - > tests/btrfs/302 | 3 +-- > tests/btrfs/302.out | 1 - > tests/btrfs/314 | 3 +-- > tests/btrfs/314.out | 2 -- > tests/btrfs/320 | 16 ++++++++-------- > 118 files changed, 375 insertions(+), 435 deletions(-) >
On 4/24/24 23:49, Zorro Lang wrote: > On Wed, Apr 24, 2024 at 06:06:43AM +0800, Anand Jain wrote: >> (I just realized that the previous attempt to send this PR failed. Resending it now.) >> >> Zorro, >> >> Several of the btrfs test cases were failing due to a change in the golden >> output. The commits here fix them. These patches are on top of the last PR >> branch staged-20240414. > > Hi Anand, > > Thanks for working on this! Now I've merged all 12 patches with below changes: > > I added your RVB to: > fstests: btrfs: use _btrfs for 'subvolume snapshot' command > due to I trust you've reviewed it as you'd like to push it. > Yes, it's fine. > Then I reviewed those 4 patches from you (refer to mail list): > generic: move btrfs clone device testcase to the generic group > common/verity: fix btrfs-corrupt-block -v option > btrfs/290: fix btrfs_corrupt_block options > common/btrfs: refactor _require_btrfs_corrupt_block to check option > > and merged these 4 patches with the review points (to save time). > > Please check the "patches-in-queue" branch of upstream fstests. If you > (or other btrfs folks) feel anything wrong, feel free to tell me. If > no more changes are needed, you'll see them in next release :) > Checked patches in the patches-in-queue; they look good. Thanks, Anand > Thanks, > Zorro