All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zorro Lang <zlang@kernel.org>
To: Qu Wenruo <wqu@suse.com>
Cc: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org,
	 Long An <lan@suse.com>
Subject: Re: [PATCH v2] fstests: btrfs/340: add support for older kernels/progs
Date: Sat, 16 May 2026 22:03:55 +0800	[thread overview]
Message-ID: <agh3zgP9C9EPn_HS@zlang-mailbox> (raw)
In-Reply-To: <816ad05a-4c36-480b-8166-d3c94597fb95@suse.com>

On Sat, May 16, 2026 at 08:48:55AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2026/5/16 02:25, Zorro Lang 写道:
> > On Thu, May 14, 2026 at 06:34:39PM +0930, Qu Wenruo wrote:
> > > The test case will fail on older distros like SLE12-SP5, there are two
> > > problems on the test case itself:
> > > 
> > > - Requires "--sync" option from "btrfs qgroup show" subcommand
> > >    That option was introduecd in v4.9.1, thus there are still some older
> > >    distros using that very old version.
> > > 
> > > - Not all kernels support quick inherit
> > >    The kernel commit introducing the quick inherit is 6.9, thus some
> > >    supported LTS kernels do not have that commit.
> > > 
> > >    In that case qgroup will just be marked inconsistent, this will cause
> > >    the later "btrfs qgroup show" command to output a warning due to the
> > >    inconsistent flag.
> > > 
> > > Fix both problems by:
> > > 
> > > - Require "--sync" option from "btrfs qgroup show" subcommand
> > >    So older distros will just skip the test.
> > > 
> > > - Redirect all output from "btrfs qgroup show" to $seqres.full
> > >    So older kernels can still pass the test case by marking qgroup
> > >    inconsitent.
> > > 
> > > Reported-by: Long An <lan@suse.com>
> > > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > > ---
> > > Changelog:
> > > v2:
> > > - Fix the mismatch between the commit message and the test case change
> > >    Forgot to amend the changes before sending.
> > > 
> > > - Fix a bug in the stderr redirect
> > >    The stdout redirect should be put last.
> > 
> > Sorry, did I miss part of the discussion? Why did v2 switch to notrun for
> > older tools instead of using the compatible `btrfs filesystem sync` from v1?
> > Was there something wrong with the v1 method :)
> 
> The main concern is the progs used in those tests can be really old, I mean
> there is no supported LTS kernel from that time period anymore.
> 
> With that old progs, I'm not confident enough to trust the progs to do all
> the qgroup numbers verification.
> 
> Nor do I even have an environment at hand to compile that old progs.
> 
> That's why in v2 I switch to skip the test case completely if the old progs
> doesn't even support "btrfs qgroup show --sync".

Oh, that explanation makes sense. I trust your judgment on this as a core btrfs
developer. If there are no other review points from the btrfs list, I'll drop v1
and merge v2. Thanks!

Reviewed-by: Zorro Lang <zlang@kernel.org>

> 
> Thanks,
> Qu
> 
> > 
> > Thanks,
> > Zorro
> > 
> > > ---
> > >   tests/btrfs/340 | 8 +++++++-
> > >   1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/btrfs/340 b/tests/btrfs/340
> > > index 872f790cb975..eff76d0b633c 100755
> > > --- a/tests/btrfs/340
> > > +++ b/tests/btrfs/340
> > > @@ -16,6 +16,7 @@ _fixed_by_kernel_commit 68d4b3fa18d7 \
> > >   # For the automatic fsck at unmount.
> > >   _require_scratch
> > >   _require_btrfs_qgroup_report
> > > +_require_btrfs_command qgroup show --sync
> > >   # This will imply mkfs and enable regular qgroup.
> > >   _require_scratch_qgroup
> > > @@ -39,7 +40,12 @@ _btrfs qgroup show -p --sync $SCRATCH_MNT >> $seqres.full
> > >   # without marking qgroup inconsistent.
> > >   _btrfs subv snap -i 1/1 $SCRATCH_MNT/subv1 $SCRATCH_MNT/snap1
> > >   echo "After quick inherit:" >> $seqres.full
> > > -_btrfs qgroup show -p --sync $SCRATCH_MNT >> $seqres.full
> > > +# Redirect all output to seqres.full including stderr.
> > > +# For older kernels without quick inherit support, it will mark
> > > +# qgroup inconsistent, which will output a warning about the inconsistent
> > > +# numbers. Here we rely on the final btrfs check to catch any qgroup number
> > > +# errors, thus no need to bother that possible warning.
> > > +_btrfs qgroup show -p --sync $SCRATCH_MNT >> $seqres.full 2>&1
> > >   echo "Silence is golden"
> > >   _exit 0
> > > -- 
> > > 2.54.0
> > > 
> > > 
> 
> 

      reply	other threads:[~2026-05-16 14:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-14  9:04 [PATCH v2] fstests: btrfs/340: add support for older kernels/progs Qu Wenruo
2026-05-15 16:55 ` Zorro Lang
2026-05-15 23:18   ` Qu Wenruo
2026-05-16 14:03     ` Zorro Lang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agh3zgP9C9EPn_HS@zlang-mailbox \
    --to=zlang@kernel.org \
    --cc=fstests@vger.kernel.org \
    --cc=lan@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.