QEMU-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
Cc: qemu-devel@nongnu.org, "Hanna Reitz" <hreitz@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	qemu-block@nongnu.org, "Cleber Rosa" <crosa@redhat.com>,
	"Kevin Wolf" <kwolf@redhat.com>, "John Snow" <jsnow@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Thomas Huth" <thuth@redhat.com>
Subject: Re: [PATCH 14/16] tests: add QEMU_TEST_IO_SKIP for skipping I/O tests
Date: Tue, 12 May 2026 17:06:24 +0100	[thread overview]
Message-ID: <agNPkLRe0Rf4NYcU@redhat.com> (raw)
In-Reply-To: <fa8be3d8-6d76-4811-83ab-69c256e8e44f@oss.qualcomm.com>

On Tue, May 12, 2026 at 08:56:54AM -0700, Pierrick Bouvier wrote:
> On 4/24/2026 8:42 AM, Daniel P. Berrangé wrote:
> > The nature of block I/O tests is such that there can be unexpected false
> > positive failures in certain scenarios that have not been encountered
> > before, and sometimes non-deterministic failures that are hard to
> > reproduce.
> > 
> > Before enabling the I/O tests as gating jobs in CI, there needs to be a
> > mechanism to dynamically mark tests as skipped, without having to commit
> > code changes.
> > 
> > This introduces the QEMU_TEST_IO_SKIP environment variable that is set
> > to a list of FORMAT-OR-PROTOCOL:NAME pairs. The intent is that this
> > variable can be set as a GitLab CI pipeline variable to temporarily
> > disable a test while problems are being debugged.
> > 
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  docs/devel/testing/main.rst      |  7 +++++++
> >  tests/qemu-iotests/testrunner.py | 16 ++++++++++++++++
> >  2 files changed, 23 insertions(+)
> > 
> > diff --git a/docs/devel/testing/main.rst b/docs/devel/testing/main.rst
> > index 797111009a..f779a64415 100644
> > --- a/docs/devel/testing/main.rst
> > +++ b/docs/devel/testing/main.rst
> > @@ -284,6 +284,13 @@ that are specific to certain cache mode.
> >  More options are supported by the ``./check`` script, run ``./check -h`` for
> >  help.
> >  
> > +If a test program is known to be broken, it can be disabled by setting
> > +the ``QEMU_TEST_IO_SKIP`` environment variable with a list of tests to
> > +be skipped. The values are of the form FORMAT-OR-PROTOCOL:NAME, the
> > +leading component can be omitted to skip the test for all formats and
> > +protocols. For example ``export QEMU_TEST_IO_SKIP="luks:149 185 iov-padding``
> > +will skip ``149`` for LUKS only, and ``185`` and ``iov-padding`` for all.
> > +
> >  Writing a new test case
> >  ~~~~~~~~~~~~~~~~~~~~~~~
> >  
> > diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
> > index dbe2dddc32..ecb5d4529f 100644
> > --- a/tests/qemu-iotests/testrunner.py
> > +++ b/tests/qemu-iotests/testrunner.py
> > @@ -145,6 +145,18 @@ def __init__(self, env: TestEnv, tap: bool = False,
> >  
> >          self._stack: contextlib.ExitStack
> >  
> > +        self.skip = {}
> > +        for rule in os.environ.get("QEMU_TEST_IO_SKIP", "").split(" "):
> > +            rule = rule.strip()
> > +            if rule == "":
> > +                continue
> > +            if ":" in rule:
> > +                fmt, name = rule.split(":")
> > +                if fmt in ("", env.imgfmt, env.imgproto):
> > +                    self.skip[name] = True
> > +            else:
> > +                self.skip[rule] = True
> > +
> >      def __enter__(self) -> 'TestRunner':
> >          self._stack = contextlib.ExitStack()
> >          self._stack.enter_context(self.env)
> > @@ -251,6 +263,10 @@ def do_run_test(self, test: str) -> TestResult:
> >                                description='No qualified output '
> >                                            f'(expected {f_reference})')
> >  
> > +        if f_test.name in self.skip:
> > +            return TestResult(status='not run',
> > +                              description='Listed in QEMU_TEST_IO_SKIP')
> > +
> >          args = [str(f_test.resolve())]
> >          env = self.env.prepare_subprocess(args)
> >  
> 
> Why not simply remove the broken tests, and create issues to add them
> again in the future?

In theory that's what our policy today is, but in practice it is
too much of a burden on the release co-ordinator, to expect them
to create such a patch themselves, or wait on a subsys maintainer
todo it for them.

They end up just ignoring brokenness in CI which is a bad practice,
and will prevent us ever making CI truely gating or switching to
using MRs for pull requests. This gives us a super-fast way to skip
flaky tests, while the subsystem maintainers figure out the right
permanent answer.

> Once it's green, in theory, code breaking existing tests should not be
> merged, right? So what would be the usage of this variable?

We have had a pretty decent chunk of non-deterministic test failures,
so there is high likelihood we can merge stuff that passes once and
then subsequently fails some subset of the time. This non-determinism
is one of the key reasons that we currently only have a hand picked
selection of block I/O tests run by meson.

While I've tested this series and haven't see any non-determinstic
failures in what I'm proposing to enable thus far, I think there is
still a pretty high chance we'll uncover some more.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|



  reply	other threads:[~2026-05-12 16:07 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-24 15:41 [PATCH 00/16] tests: do more testing of block drivers Daniel P. Berrangé
2026-04-24 15:41 ` [PATCH 01/16] python: bump qemu.qmp to v0.0.6 Daniel P. Berrangé
2026-05-12 15:37   ` Pierrick Bouvier
2026-04-24 15:41 ` [PATCH 02/16] gitlab: ensure all meson jobs capture build/meson-logs by default Daniel P. Berrangé
2026-05-12 15:38   ` Pierrick Bouvier
2026-04-24 15:41 ` [PATCH 03/16] tests: print reason when I/O test is skipped in TAP mode Daniel P. Berrangé
2026-05-12 15:38   ` Pierrick Bouvier
2026-04-24 15:41 ` [PATCH 04/16] tests: remove redundant meson suite for iotests Daniel P. Berrangé
2026-05-12 15:42   ` Pierrick Bouvier
2026-04-24 15:41 ` [PATCH 05/16] tests: ensure all qcow2 I/O tests are able to be run via make Daniel P. Berrangé
2026-04-24 15:41 ` [PATCH 06/16] scripts/mtest2make: ensure output has stable sorting Daniel P. Berrangé
2026-04-24 15:41 ` [PATCH 07/16] scripts/mtest2make: support optional tests grouping Daniel P. Berrangé
2026-05-12 15:45   ` Pierrick Bouvier
2026-05-13 10:08     ` Daniel P. Berrangé
2026-05-13 15:49       ` Pierrick Bouvier
2026-05-13 17:15         ` Daniel P. Berrangé
2026-05-13 17:23           ` Pierrick Bouvier
2026-05-13 17:26             ` Daniel P. Berrangé
2026-05-13 17:32               ` Pierrick Bouvier
2026-04-24 15:41 ` [PATCH 08/16] tests: add a meson suite / make target per block I/O tests format Daniel P. Berrangé
2026-05-12 15:46   ` Pierrick Bouvier
2026-04-24 15:41 ` [PATCH 09/16] docs/devel/testing: expand documentation for 'make check-block' Daniel P. Berrangé
2026-05-12 15:47   ` Pierrick Bouvier
2026-04-24 15:41 ` [PATCH 10/16] tests: add nbd and luks to the I/O test suites Daniel P. Berrangé
2026-05-12 15:47   ` Pierrick Bouvier
2026-04-24 15:41 ` [PATCH 11/16] tests: use 'driver' as collective term for either format or protocol Daniel P. Berrangé
2026-05-12 15:52   ` Pierrick Bouvier
2026-04-24 15:42 ` [PATCH 12/16] tests: validate dmsetup result in test 128 Daniel P. Berrangé
2026-05-12 15:53   ` Pierrick Bouvier
2026-05-13 10:11     ` Daniel P. Berrangé
2026-05-13 15:51       ` Pierrick Bouvier
2026-04-24 15:42 ` [PATCH 13/16] tests: fix check for sudo access in LUKS I/O test Daniel P. Berrangé
2026-05-12 15:54   ` Pierrick Bouvier
2026-04-24 15:42 ` [PATCH 14/16] tests: add QEMU_TEST_IO_SKIP for skipping I/O tests Daniel P. Berrangé
2026-05-12 15:56   ` Pierrick Bouvier
2026-05-12 16:06     ` Daniel P. Berrangé [this message]
2026-05-12 16:19       ` Pierrick Bouvier
2026-05-12 16:36         ` Daniel P. Berrangé
2026-05-12 16:47           ` Pierrick Bouvier
2026-05-12 16:53             ` Daniel P. Berrangé
2026-05-12 17:09               ` Pierrick Bouvier
2026-05-12 17:24                 ` Daniel P. Berrangé
2026-05-12 17:53                   ` Pierrick Bouvier
2026-05-12 18:46                     ` Daniel P. Berrangé
2026-05-12 18:52                       ` Pierrick Bouvier
2026-05-12 19:00                         ` Daniel P. Berrangé
2026-05-12 19:12                           ` Pierrick Bouvier
2026-05-13 14:11                             ` Fabiano Rosas
2026-05-13 14:58                               ` Daniel P. Berrangé
2026-05-13  6:18           ` Thomas Huth
2026-05-13 15:53             ` Pierrick Bouvier
2026-04-24 15:42 ` [PATCH 15/16] gitlab: add jobs for thorough block tests Daniel P. Berrangé
2026-05-12 15:59   ` Pierrick Bouvier
2026-04-24 15:42 ` [PATCH 16/16] gitlab: remove I/O tests from build-tcg-disabled job Daniel P. Berrangé
2026-04-25  6:53   ` Thomas Huth
2026-05-12 15:47   ` Pierrick Bouvier
2026-05-12 13:53 ` [PATCH 00/16] tests: do more testing of block drivers Daniel P. Berrangé

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=agNPkLRe0Rf4NYcU@redhat.com \
    --to=berrange@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=crosa@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=pierrick.bouvier@oss.qualcomm.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox