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 <[email protected]>
> > Signed-off-by: Daniel P. Berrangé <[email protected]>
> > ---
> >  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 via email to