[PATCH 0/4] tests: do more testing of block drivers in CI

[PATCH 0/4] tests: do more testing of block drivers in CI

[Daniel P. Berrangé]

The recent set of regressions identified in the LUKS block driver
re-inforced that despite having a hugely useful set of I/O tests,
our CI coverage is still letting through too many bugs.

This series expands the meson suites / make targets for running
I/O tests for more drivers, and then creates a CI job for each.

While the first three patches are functionally usable as is, the
last patch for gitlab is very much an RFC. A test pipeline

  https://gitlab.com/berrange/qemu/-/pipelines/2085907042

shows passes for qcow2, qed, vmdk & vpc, but failures for luks,
nbd and raw.

The luks failures all have patchs floating around pending merge.

The nbd failures are problems with non-deterministic output
ordering, or warnings from python deprecations that need
addressing.

The raw failures are problems with python deprecations, and
insufficient permissions checks wrt device mapper acess.

I'm unclear if there are other non-deterministic failures that
would only appear if we start running the tests regularly. That
is a chicken & egg problem though. At some point we have to accept
a (hopefully short) period of instability to tease out the rare
edge cases, in order to get a long term win of better code quality
through CI coverage.

Daniel P. Berrangé (4):
  tests: print reason when I/O test is skipped in TAP mode
  tests: rearrange suites for I/O tests
  tests: add nbd and luks to the I/O test suites
  gitlab: add jobs for thorough block tests

 .gitlab-ci.d/buildtest.yml       | 63 ++++++++++++++++++++++++++++++++
 tests/qemu-iotests/meson.build   | 52 ++++++++++++++++++++------
 tests/qemu-iotests/testrunner.py |  2 +-
 3 files changed, 104 insertions(+), 13 deletions(-)

-- 
2.50.1

[PATCH 1/4] tests: print reason when I/O test is skipped in TAP mode

[Daniel P. Berrangé]

The TAP output on a skipped test:

  ok raw 181 # SKIP

is not informative.

The test program included a reason, and that should be displayed
in TAP mode (it is already shown in non-TAP mode):

  ok raw 181 # SKIP Postcopy is not supported

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/testrunner.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 14cc8492f9..c4ce3e29e9 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -174,7 +174,7 @@ def test_print_one_line(self, test: str,
             elif status == 'fail':
                 print(f'not ok {self.env.imgfmt} {test}')
             elif status == 'not run':
-                print(f'ok {self.env.imgfmt} {test} # SKIP')
+                print(f'ok {self.env.imgfmt} {test} # SKIP {description}')
             return
 
         if lasttime:
-- 
2.50.1

Re: [PATCH 1/4] tests: print reason when I/O test is skipped in TAP mode

[Thomas Huth]

On 08/10/2025 13.35, Daniel P. Berrangé wrote:
> The TAP output on a skipped test:
> 
>    ok raw 181 # SKIP
> 
> is not informative.
> 
> The test program included a reason, and that should be displayed
> in TAP mode (it is already shown in non-TAP mode):
> 
>    ok raw 181 # SKIP Postcopy is not supported
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/qemu-iotests/testrunner.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
> index 14cc8492f9..c4ce3e29e9 100644
> --- a/tests/qemu-iotests/testrunner.py
> +++ b/tests/qemu-iotests/testrunner.py
> @@ -174,7 +174,7 @@ def test_print_one_line(self, test: str,
>               elif status == 'fail':
>                   print(f'not ok {self.env.imgfmt} {test}')
>               elif status == 'not run':
> -                print(f'ok {self.env.imgfmt} {test} # SKIP')
> +                print(f'ok {self.env.imgfmt} {test} # SKIP {description}')
>               return
>   
>           if lasttime:

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 2/4] tests: rearrange suites for I/O tests

[Daniel P. Berrangé]

Currently there are 5 block formats, each classified as either quick,
slow or thorough. This classification then determines what tests we
add to meson suites. The suites are as follows

  * block

    => tests listed by 'check -g auto -n' for 'quick' formats

  * slow, block-slow

    => tests listed by 'check -n' for 'slow' formats or 'check -g auto -n'
       for 'quick' formats

  * thorough, block-thorough

    => tests listed by 'check -n' for 'thorough' formats

The pairs of suites 'slow' / 'block-slow' and 'thorough' / 'block-thorough'
match in terms of what tests are enabled. The only difference is whether
non-block related tests are also in the suite.

There are two problems with this

 * If a format is classified as 'quick', we don't expose any
   meson suite for running *all* tests, only the 'auto' tests.

   eg there is no suite to run all qcow2 tests, only 'quick'
   tests can be run via meson, even if using 'SPEED=slow' we
   still filter to only 'auto' tests.

 * There is no suite that allows running all tests for a given
   format.

   eg there is no suite to run only 'raw' tests - you can only
   use 'block-slow' which runs both raw and "auto" qcow2 tests.

   eg there is no suite to run only 'vpc' tests - you can only
   use 'block-thorough' which runs qed, vmdk & vpc tests.

This patch suggests that 'block-slow' and 'block-thorough' are
not actually compelling use cases, and should be dropped. ie it
is not expected that people need to run all VPC, VMDK and QED
tests at the same time. Instead a more useful feature is the
ability to run all tests for a given format. Further the 'auto'
filtering should only apply in the default 'block' target/suite.

IOW, with this patch we get the follows meson suites:

 * 'block' - 'auto' tests for any format listed as 'quick'
             Currently just qcow2 'auto' tests
 * 'block-$FORMAT' - ALL tests for the given $FORMAT, for each
                     of qcow2, raw, qed, vmdk & vpc
 * 'slow' - ALL tests for formats tagged with 'quick' or 'slow'
 * 'thorough' - ALL tests formats tagged with 'thorough'

This corresponds to the following make targets.

 * 'make check-block'

    => runs only 'auto' qcow2 tests  (unchanged)

 * 'make check-block SPEED=thorough'

    => runs all 'qed', 'vmdk', 'vpc' tests (unchanged)

 * 'make check-block SPEED=slow'

    => runs all 'raw' tests (unchanged)
    => runs all 'qcow2' tests (previously was only 'auto' tests)

 * 'make check-block-qcow2'

     => runs all qcow2 tests (new feature)

 * 'make check-block-raw'

     => runs all raw tests (new feature)

 * 'make check-block-vpc'

     => runs all vpc tests (new feature)

 * 'make check-block-qed'

     => runs all qed tests (new feature)

 * 'make check-block-vmdk'

     => runs all vmdk tests (new feature)

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/meson.build | 48 ++++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index fad340ad59..939a14ffae 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -35,23 +35,21 @@ endforeach
 qemu_iotests_check_cmd = files('check')
 
 foreach format, speed: qemu_iotests_formats
-  if speed == 'quick'
-    suites = 'block'
-  else
-    suites = ['block-' + speed, speed]
-  endif
-
-  args = ['-tap', '-' + format]
-  if speed == 'quick'
-      args += ['-g', 'auto']
-  endif
+  listargs = ['-tap', '-' + format]
 
   rc = run_command(
-      [python, qemu_iotests_check_cmd] + args + ['-n'],
+      [python, qemu_iotests_check_cmd] + listargs + ['-n'],
       check: true,
   )
 
   foreach item: rc.stdout().strip().split()
+      suites = ['block-' + format]
+      if speed == 'quick'
+          suites += ['slow']
+      else
+          suites += [speed]
+      endif
+
       args = [qemu_iotests_check_cmd,
               '-tap', '-' + format, item,
               '--source-dir', meson.current_source_dir(),
@@ -68,4 +66,32 @@ foreach format, speed: qemu_iotests_formats
            timeout: 180,
            suite: suites)
   endforeach
+
+  if speed == 'quick'
+      listargs += ['-g', 'auto']
+      suites = ['block']
+
+      rc = run_command(
+          [python, qemu_iotests_check_cmd] + listargs + ['-n'],
+          check: true,
+      )
+
+      foreach item: rc.stdout().strip().split()
+          args = [qemu_iotests_check_cmd,
+                  '-tap', '-' + format, item,
+                  '--source-dir', meson.current_source_dir(),
+                  '--build-dir', meson.current_build_dir()]
+          # Some individual tests take as long as 45 seconds
+          # Bump the timeout to 3 minutes for some headroom
+          # on slow machines to minimize spurious failures
+          test('io-' + format + '-' + item,
+               python,
+               args: args,
+               depends: qemu_iotests_binaries,
+               env: qemu_iotests_env,
+               protocol: 'tap',
+               timeout: 180,
+               suite: suites)
+      endforeach
+  endif
 endforeach
-- 
2.50.1

Re: [PATCH 2/4] tests: rearrange suites for I/O tests

[Kevin Wolf]

Am 08.10.2025 um 13:35 hat Daniel P. Berrangé geschrieben:
> Currently there are 5 block formats, each classified as either quick,
> slow or thorough. This classification then determines what tests we
> add to meson suites. The suites are as follows
> 
>   * block
> 
>     => tests listed by 'check -g auto -n' for 'quick' formats
> 
>   * slow, block-slow
> 
>     => tests listed by 'check -n' for 'slow' formats or 'check -g auto -n'
>        for 'quick' formats
> 
>   * thorough, block-thorough
> 
>     => tests listed by 'check -n' for 'thorough' formats
> 
> The pairs of suites 'slow' / 'block-slow' and 'thorough' / 'block-thorough'
> match in terms of what tests are enabled. The only difference is whether
> non-block related tests are also in the suite.
> 
> There are two problems with this
> 
>  * If a format is classified as 'quick', we don't expose any
>    meson suite for running *all* tests, only the 'auto' tests.
> 
>    eg there is no suite to run all qcow2 tests, only 'quick'
>    tests can be run via meson, even if using 'SPEED=slow' we
>    still filter to only 'auto' tests.
> 
>  * There is no suite that allows running all tests for a given
>    format.
> 
>    eg there is no suite to run only 'raw' tests - you can only
>    use 'block-slow' which runs both raw and "auto" qcow2 tests.
> 
>    eg there is no suite to run only 'vpc' tests - you can only
>    use 'block-thorough' which runs qed, vmdk & vpc tests.
> 
> This patch suggests that 'block-slow' and 'block-thorough' are
> not actually compelling use cases, and should be dropped. ie it
> is not expected that people need to run all VPC, VMDK and QED
> tests at the same time. Instead a more useful feature is the
> ability to run all tests for a given format. Further the 'auto'
> filtering should only apply in the default 'block' target/suite.
> 
> IOW, with this patch we get the follows meson suites:
> 
>  * 'block' - 'auto' tests for any format listed as 'quick'
>              Currently just qcow2 'auto' tests
>  * 'block-$FORMAT' - ALL tests for the given $FORMAT, for each
>                      of qcow2, raw, qed, vmdk & vpc
>  * 'slow' - ALL tests for formats tagged with 'quick' or 'slow'
>  * 'thorough' - ALL tests formats tagged with 'thorough'
> 
> This corresponds to the following make targets.
> 
>  * 'make check-block'
> 
>     => runs only 'auto' qcow2 tests  (unchanged)
> 
>  * 'make check-block SPEED=thorough'
> 
>     => runs all 'qed', 'vmdk', 'vpc' tests (unchanged)

Also all qcow2 and raw tests, right?

Quotes are a bit inconsistent here in the commit message. Sometimes
you use quotes for format names like here, but in other places you use
quotes for 'auto' and leave format names without quotes.

>  * 'make check-block SPEED=slow'
> 
>     => runs all 'raw' tests (unchanged)
>     => runs all 'qcow2' tests (previously was only 'auto' tests)
> 
>  * 'make check-block-qcow2'
> 
>      => runs all qcow2 tests (new feature)
> 
>  * 'make check-block-raw'
> 
>      => runs all raw tests (new feature)
> 
>  * 'make check-block-vpc'
> 
>      => runs all vpc tests (new feature)
> 
>  * 'make check-block-qed'
> 
>      => runs all qed tests (new feature)
> 
>  * 'make check-block-vmdk'
> 
>      => runs all vmdk tests (new feature)
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/meson.build | 48 ++++++++++++++++++++++++++--------
>  1 file changed, 37 insertions(+), 11 deletions(-)
> 
> diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
> index fad340ad59..939a14ffae 100644
> --- a/tests/qemu-iotests/meson.build
> +++ b/tests/qemu-iotests/meson.build
> @@ -35,23 +35,21 @@ endforeach
>  qemu_iotests_check_cmd = files('check')
>  
>  foreach format, speed: qemu_iotests_formats
> -  if speed == 'quick'
> -    suites = 'block'
> -  else
> -    suites = ['block-' + speed, speed]
> -  endif
> -
> -  args = ['-tap', '-' + format]
> -  if speed == 'quick'
> -      args += ['-g', 'auto']
> -  endif
> +  listargs = ['-tap', '-' + format]
>  
>    rc = run_command(
> -      [python, qemu_iotests_check_cmd] + args + ['-n'],
> +      [python, qemu_iotests_check_cmd] + listargs + ['-n'],
>        check: true,
>    )
>  
>    foreach item: rc.stdout().strip().split()
> +      suites = ['block-' + format]
> +      if speed == 'quick'
> +          suites += ['slow']
> +      else
> +          suites += [speed]
> +      endif
> +
>        args = [qemu_iotests_check_cmd,
>                '-tap', '-' + format, item,
>                '--source-dir', meson.current_source_dir(),
> @@ -68,4 +66,32 @@ foreach format, speed: qemu_iotests_formats
>             timeout: 180,
>             suite: suites)
>    endforeach
> +
> +  if speed == 'quick'
> +      listargs += ['-g', 'auto']
> +      suites = ['block']
> +
> +      rc = run_command(
> +          [python, qemu_iotests_check_cmd] + listargs + ['-n'],
> +          check: true,
> +      )
> +
> +      foreach item: rc.stdout().strip().split()
> +          args = [qemu_iotests_check_cmd,
> +                  '-tap', '-' + format, item,
> +                  '--source-dir', meson.current_source_dir(),
> +                  '--build-dir', meson.current_build_dir()]
> +          # Some individual tests take as long as 45 seconds
> +          # Bump the timeout to 3 minutes for some headroom
> +          # on slow machines to minimize spurious failures
> +          test('io-' + format + '-' + item,
> +               python,
> +               args: args,
> +               depends: qemu_iotests_binaries,
> +               env: qemu_iotests_env,
> +               protocol: 'tap',
> +               timeout: 180,
> +               suite: suites)
> +      endforeach
> +  endif
>  endforeach

The code itself looks good to me.

Kevin

[PATCH 3/4] tests: add nbd and luks to the I/O test suites

[Daniel P. Berrangé]

This introduces new suits for running I/O tests on NBD and LUKS
drivers, giving new make targets

 * make check-block-luks
 * make check-block-nbd

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index 939a14ffae..5735d67c8c 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -23,7 +23,9 @@ qemu_iotests_formats = {
   'raw': 'slow',
   'qed': 'thorough',
   'vmdk': 'thorough',
-  'vpc': 'thorough'
+  'vpc': 'thorough',
+  'nbd': 'thorough',
+  'luks': 'thorough',
 }
 
 foreach k, v : emulators
-- 
2.50.1

Re: [PATCH 3/4] tests: add nbd and luks to the I/O test suites

[Thomas Huth]

On 08/10/2025 13.35, Daniel P. Berrangé wrote:
> This introduces new suits for running I/O tests on NBD and LUKS
> drivers, giving new make targets
> 
>   * make check-block-luks
>   * make check-block-nbd
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/qemu-iotests/meson.build | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
> index 939a14ffae..5735d67c8c 100644
> --- a/tests/qemu-iotests/meson.build
> +++ b/tests/qemu-iotests/meson.build
> @@ -23,7 +23,9 @@ qemu_iotests_formats = {
>     'raw': 'slow',
>     'qed': 'thorough',
>     'vmdk': 'thorough',
> -  'vpc': 'thorough'
> +  'vpc': 'thorough',
> +  'nbd': 'thorough',
> +  'luks': 'thorough',
>   }

Before we do that, I'd first see a solution for the problem that I described 
in my series here:

https://lore.kernel.org/qemu-devel/20250910153727.226217-1-thuth@redhat.com/

which, by the way, contains a patch that is very similar to yours here.

Also not sure whether we should add "nbd" to the "formats" list - it's a 
protocol, and not a format, isn't it?

  Thomas

Re: [PATCH 3/4] tests: add nbd and luks to the I/O test suites

[Daniel P. Berrangé]

On Wed, Oct 08, 2025 at 01:55:12PM +0200, Thomas Huth wrote:
> On 08/10/2025 13.35, Daniel P. Berrangé wrote:
> > This introduces new suits for running I/O tests on NBD and LUKS
> > drivers, giving new make targets
> > 
> >   * make check-block-luks
> >   * make check-block-nbd
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   tests/qemu-iotests/meson.build | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
> > index 939a14ffae..5735d67c8c 100644
> > --- a/tests/qemu-iotests/meson.build
> > +++ b/tests/qemu-iotests/meson.build
> > @@ -23,7 +23,9 @@ qemu_iotests_formats = {
> >     'raw': 'slow',
> >     'qed': 'thorough',
> >     'vmdk': 'thorough',
> > -  'vpc': 'thorough'
> > +  'vpc': 'thorough',
> > +  'nbd': 'thorough',
> > +  'luks': 'thorough',
> >   }
> 
> Before we do that, I'd first see a solution for the problem that I described
> in my series here:
> 
> https://lore.kernel.org/qemu-devel/20250910153727.226217-1-thuth@redhat.com/
> 
> which, by the way, contains a patch that is very similar to yours here.

IIUC, the problem you're concerned with is that 'make check SPEED=thorough'
is running too much, and you want to stop running skipped tests directly.

My view is that running "make check SPEED=thorough" is undesirable in
general, even before either of our patch series. I'd say it is almost
never what people actually want to use, and is only picked because of
the lack of a better option. That's why I thought 'make check-block-qed'
(and equiva for other formats) was a better option, as it gives a make
target that matches a specific testing use case. With that in mind, IMHO
it is less important if 'make check SPEED=thorough' waste a bit of time
launched irrelevant tests.

> Also not sure whether we should add "nbd" to the "formats" list - it's a
> protocol, and not a format, isn't it?

Yes, technically there are two distinct axis  formats vs protocols, but
from the POV of running the 'check' script the boundary is rather blurred.

You can run './check -nbd' and './check -qcow2', or both combined. The main
limit that you can only pick a single format and single protocol at a time.

IMHO for test suites it is preferrable to keep a flat namespace, rather
than creating a matrix of suites for protocol vs format combniations.

Perhaps the meson.build variable should just be renamed from _formats
to something else.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 3/4] tests: add nbd and luks to the I/O test suites

[Thomas Huth]

On 08/10/2025 14.55, Daniel P. Berrangé wrote:
> On Wed, Oct 08, 2025 at 01:55:12PM +0200, Thomas Huth wrote:
>> On 08/10/2025 13.35, Daniel P. Berrangé wrote:
>>> This introduces new suits for running I/O tests on NBD and LUKS
>>> drivers, giving new make targets
>>>
>>>    * make check-block-luks
>>>    * make check-block-nbd
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>> ---
>>>    tests/qemu-iotests/meson.build | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
>>> index 939a14ffae..5735d67c8c 100644
>>> --- a/tests/qemu-iotests/meson.build
>>> +++ b/tests/qemu-iotests/meson.build
>>> @@ -23,7 +23,9 @@ qemu_iotests_formats = {
>>>      'raw': 'slow',
>>>      'qed': 'thorough',
>>>      'vmdk': 'thorough',
>>> -  'vpc': 'thorough'
>>> +  'vpc': 'thorough',
>>> +  'nbd': 'thorough',
>>> +  'luks': 'thorough',
>>>    }
>>
>> Before we do that, I'd first see a solution for the problem that I described
>> in my series here:
>>
>> https://lore.kernel.org/qemu-devel/20250910153727.226217-1-thuth@redhat.com/
>>
>> which, by the way, contains a patch that is very similar to yours here.
> 
> IIUC, the problem you're concerned with is that 'make check SPEED=thorough'
> is running too much, and you want to stop running skipped tests directly.
> 
> My view is that running "make check SPEED=thorough" is undesirable in
> general, even before either of our patch series. I'd say it is almost
> never what people actually want to use, and is only picked because of
> the lack of a better option. That's why I thought 'make check-block-qed'
> (and equiva for other formats) was a better option, as it gives a make
> target that matches a specific testing use case. With that in mind, IMHO
> it is less important if 'make check SPEED=thorough' waste a bit of time
> launched irrelevant tests.

Sounds like you're only thinking about running iotests here. But what I 
generally want to do: Run *all* tests at once, in parallel, on as many CPUs 
as my host system provides, i.e. also qtests and functional tests in 
parallel with the iotests. That's what you get with "make check 
SPEED=thorough" only right now. And the output of that list is quite 
cluttered with a lot of skipped iotests, which will only get much worse if 
we add more formats to qemu_iotests_formats without any other patches.

  Thomas

Re: [PATCH 3/4] tests: add nbd and luks to the I/O test suites

[Eric Blake]

On Wed, Oct 08, 2025 at 01:55:09PM +0100, Daniel P. Berrangé wrote:
> > Before we do that, I'd first see a solution for the problem that I described
> > in my series here:
> > 
> > https://lore.kernel.org/qemu-devel/20250910153727.226217-1-thuth@redhat.com/
> > 
> > which, by the way, contains a patch that is very similar to yours here.
> 
> IIUC, the problem you're concerned with is that 'make check SPEED=thorough'
> is running too much, and you want to stop running skipped tests directly.

Or at least reduce the output (easier to focus on the success/failures
if less effort is spent on telling us about skips).

> 
> My view is that running "make check SPEED=thorough" is undesirable in
> general, even before either of our patch series. I'd say it is almost
> never what people actually want to use, and is only picked because of
> the lack of a better option. That's why I thought 'make check-block-qed'
> (and equiva for other formats) was a better option, as it gives a make
> target that matches a specific testing use case. With that in mind, IMHO
> it is less important if 'make check SPEED=thorough' waste a bit of time
> launched irrelevant tests.

The two ideas (adding more test targets, reducing output on skipped
tests) seem orthogonal and independently useful, but it does create
the question on how to resolve the conflicts on what lands first ;)

> 
> > Also not sure whether we should add "nbd" to the "formats" list - it's a
> > protocol, and not a format, isn't it?
> 
> Yes, technically there are two distinct axis  formats vs protocols, but
> from the POV of running the 'check' script the boundary is rather blurred.
> 
> You can run './check -nbd' and './check -qcow2', or both combined. The main
> limit that you can only pick a single format and single protocol at a time.

In fact, I've seen times where './check -nbd -raw' passes but './check
-nbd -qcow2' fails, because that combination enables different sets of
tests.  So we probably STILL aren't giving CI everything possible to
test by having just one dimension of easy-to-name test subsets, but it
is still better than no CI nbd tests at all.

> 
> IMHO for test suites it is preferrable to keep a flat namespace, rather
> than creating a matrix of suites for protocol vs format combniations.
> 
> Perhaps the meson.build variable should just be renamed from _formats
> to something else.

Renaming makes sense to me; would _bds be a reasonable name (since
both protocols and formats are a BDS)?  I'm open to other naming
ideas, as well.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH 3/4] tests: add nbd and luks to the I/O test suites

[Kevin Wolf]

Am 08.10.2025 um 17:55 hat Eric Blake geschrieben:
> > > Also not sure whether we should add "nbd" to the "formats" list - it's a
> > > protocol, and not a format, isn't it?
> > 
> > Yes, technically there are two distinct axis  formats vs protocols, but
> > from the POV of running the 'check' script the boundary is rather blurred.
> > 
> > You can run './check -nbd' and './check -qcow2', or both combined. The main
> > limit that you can only pick a single format and single protocol at a time.
> 
> In fact, I've seen times where './check -nbd -raw' passes but './check
> -nbd -qcow2' fails, because that combination enables different sets of
> tests.  So we probably STILL aren't giving CI everything possible to
> test by having just one dimension of easy-to-name test subsets, but it
> is still better than no CI nbd tests at all.

How valid is -nbd -qcow2 even? Wasn't there the fundamental problem that
NBD devices can't grow?

Running various image formats may be useful for other protocols, but I'm
not sure that NBD is one of them.

> > IMHO for test suites it is preferrable to keep a flat namespace, rather
> > than creating a matrix of suites for protocol vs format combniations.
> > 
> > Perhaps the meson.build variable should just be renamed from _formats
> > to something else.
> 
> Renaming makes sense to me; would _bds be a reasonable name (since
> both protocols and formats are a BDS)?  I'm open to other naming
> ideas, as well.

Well, bdrv if anything (block driver rather than BlockDriverState).

But we also have -fuse, which isn't really a block driver, but it just
means that we're using the file protocol on top of a FUSE export...

Probably not worth renaming from one imperfect name to another imperfect
one.

Kevin

Re: [PATCH 3/4] tests: add nbd and luks to the I/O test suites

[Eric Blake]

On Wed, Oct 08, 2025 at 12:35:51PM +0100, Daniel P. Berrangé wrote:
> This introduces new suits for running I/O tests on NBD and LUKS

suites

> drivers, giving new make targets
> 
>  * make check-block-luks
>  * make check-block-nbd
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/meson.build | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

[PATCH RFC/WIP 4/4] gitlab: add jobs for thorough block tests

[Daniel P. Berrangé]

CI is only exercising the qcow2 'auto' tests currently. As a result we
get no exposure of changes which cause regressions in other block format
drivers.

This adds new CI jobs for each block format, that will run the target
'make check-block-$FORMAT'. The jobs are separate so that we have the
ability to make each formats gating or not, depending on their level
of reliability.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 .gitlab-ci.d/buildtest.yml | 63 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 0502094b9a..e4ee48cf9e 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -176,6 +176,69 @@ build-system-centos:
       x86_64-softmmu rx-softmmu sh4-softmmu
     MAKE_CHECK_ARGS: check-build
 
+check-block-raw-centos:
+  extends: .native_test_job_template
+  needs:
+    - job: build-system-centos
+      artifacts: true
+  variables:
+    IMAGE: centos9
+    MAKE_CHECK_ARGS: check-block-raw
+
+check-block-qcow2-centos:
+  extends: .native_test_job_template
+  needs:
+    - job: build-system-centos
+      artifacts: true
+  variables:
+    IMAGE: centos9
+    MAKE_CHECK_ARGS: check-block-qcow2
+
+check-block-vpc-centos:
+  extends: .native_test_job_template
+  needs:
+    - job: build-system-centos
+      artifacts: true
+  variables:
+    IMAGE: centos9
+    MAKE_CHECK_ARGS: check-block-vpc
+
+check-block-vmdk-centos:
+  extends: .native_test_job_template
+  needs:
+    - job: build-system-centos
+      artifacts: true
+  variables:
+    IMAGE: centos9
+    MAKE_CHECK_ARGS: check-block-vmdk
+
+check-block-qed-centos:
+  extends: .native_test_job_template
+  needs:
+    - job: build-system-centos
+      artifacts: true
+  variables:
+    IMAGE: centos9
+    MAKE_CHECK_ARGS: check-block-qed
+
+check-block-luks-centos:
+  extends: .native_test_job_template
+  needs:
+    - job: build-system-centos
+      artifacts: true
+  variables:
+    IMAGE: centos9
+    MAKE_CHECK_ARGS: check-block-luks
+
+check-block-nbd-centos:
+  extends: .native_test_job_template
+  needs:
+    - job: build-system-centos
+      artifacts: true
+  variables:
+    IMAGE: centos9
+    MAKE_CHECK_ARGS: check-block-nbd
+
 # Previous QEMU release. Used for cross-version migration tests.
 build-previous-qemu:
   extends: .native_build_job_template
-- 
2.50.1

Re: [PATCH RFC/WIP 4/4] gitlab: add jobs for thorough block tests

[Eric Blake]

On Wed, Oct 08, 2025 at 12:35:52PM +0100, Daniel P. Berrangé wrote:
> CI is only exercising the qcow2 'auto' tests currently. As a result we
> get no exposure of changes which cause regressions in other block format
> drivers.
> 
> This adds new CI jobs for each block format, that will run the target
> 'make check-block-$FORMAT'. The jobs are separate so that we have the
> ability to make each formats gating or not, depending on their level
> of reliability.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  .gitlab-ci.d/buildtest.yml | 63 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)

The idea makes sense to me.

Did you have a link a URL of a CI run that failed tests on NBD, where
I might be able to help make the tests more deterministic?

> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index 0502094b9a..e4ee48cf9e 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -176,6 +176,69 @@ build-system-centos:
>        x86_64-softmmu rx-softmmu sh4-softmmu
>      MAKE_CHECK_ARGS: check-build
>  

> +
> +check-block-nbd-centos:
> +  extends: .native_test_job_template
> +  needs:
> +    - job: build-system-centos
> +      artifacts: true
> +  variables:
> +    IMAGE: centos9
> +    MAKE_CHECK_ARGS: check-block-nbd
> +
>  # Previous QEMU release. Used for cross-version migration tests.
>  build-previous-qemu:
>    extends: .native_build_job_template
> -- 
> 2.50.1
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH RFC/WIP 4/4] gitlab: add jobs for thorough block tests

[Daniel P. Berrangé]

On Wed, Oct 08, 2025 at 10:57:45AM -0500, Eric Blake wrote:
> On Wed, Oct 08, 2025 at 12:35:52PM +0100, Daniel P. Berrangé wrote:
> > CI is only exercising the qcow2 'auto' tests currently. As a result we
> > get no exposure of changes which cause regressions in other block format
> > drivers.
> > 
> > This adds new CI jobs for each block format, that will run the target
> > 'make check-block-$FORMAT'. The jobs are separate so that we have the
> > ability to make each formats gating or not, depending on their level
> > of reliability.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  .gitlab-ci.d/buildtest.yml | 63 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> 
> The idea makes sense to me.
> 
> Did you have a link a URL of a CI run that failed tests on NBD, where
> I might be able to help make the tests more deterministic?

Its the one in the cover letter - the nbd job is

  https://gitlab.com/berrange/qemu/-/jobs/11633680715


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH RFC/WIP 4/4] gitlab: add jobs for thorough block tests

[Eric Blake]

On Wed, Oct 08, 2025 at 05:23:10PM +0100, Daniel P. Berrangé wrote:
> On Wed, Oct 08, 2025 at 10:57:45AM -0500, Eric Blake wrote:
> > On Wed, Oct 08, 2025 at 12:35:52PM +0100, Daniel P. Berrangé wrote:
> > > CI is only exercising the qcow2 'auto' tests currently. As a result we
> > > get no exposure of changes which cause regressions in other block format
> > > drivers.
> > > 
> > > This adds new CI jobs for each block format, that will run the target
> > > 'make check-block-$FORMAT'. The jobs are separate so that we have the
> > > ability to make each formats gating or not, depending on their level
> > > of reliability.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >  .gitlab-ci.d/buildtest.yml | 63 ++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 63 insertions(+)
> > 
> > The idea makes sense to me.
> > 
> > Did you have a link a URL of a CI run that failed tests on NBD, where
> > I might be able to help make the tests more deterministic?
> 
> Its the one in the cover letter - the nbd job is
> 
>   https://gitlab.com/berrange/qemu/-/jobs/11633680715

I finally managed to run 'git bisect' on an old enough version to
track down two of the NBD failures, broken since v9.0.0:

https://lists.gnu.org/archive/html/qemu-devel/2025-10/msg03063.html

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH RFC/WIP 4/4] gitlab: add jobs for thorough block tests

[Thomas Huth]

On 08/10/2025 13.35, Daniel P. Berrangé wrote:
> CI is only exercising the qcow2 'auto' tests currently. As a result we
> get no exposure of changes which cause regressions in other block format
> drivers.
> 
> This adds new CI jobs for each block format, that will run the target
> 'make check-block-$FORMAT'. The jobs are separate so that we have the
> ability to make each formats gating or not, depending on their level
> of reliability.

Some of the jobs seem to be quite quick, though, only 5 minutes and less:

  https://gitlab.com/berrange/qemu/-/jobs/11633680698
  https://gitlab.com/berrange/qemu/-/jobs/11633680704
  https://gitlab.com/berrange/qemu/-/jobs/11633680694

I think I'd prefer to see them rather merged into one job, to avoid that we 
waste too much time with setting up the container each time. We can still 
sort out flaky parts into separate jobs later if necessary.

  Thomas

Re: [PATCH RFC/WIP 4/4] gitlab: add jobs for thorough block tests

[Daniel P. Berrangé]

On Fri, Oct 10, 2025 at 01:55:28PM +0200, Thomas Huth wrote:
> On 08/10/2025 13.35, Daniel P. Berrangé wrote:
> > CI is only exercising the qcow2 'auto' tests currently. As a result we
> > get no exposure of changes which cause regressions in other block format
> > drivers.
> > 
> > This adds new CI jobs for each block format, that will run the target
> > 'make check-block-$FORMAT'. The jobs are separate so that we have the
> > ability to make each formats gating or not, depending on their level
> > of reliability.
> 
> Some of the jobs seem to be quite quick, though, only 5 minutes and less:
> 
>  https://gitlab.com/berrange/qemu/-/jobs/11633680698
>  https://gitlab.com/berrange/qemu/-/jobs/11633680704
>  https://gitlab.com/berrange/qemu/-/jobs/11633680694
> 
> I think I'd prefer to see them rather merged into one job, to avoid that we
> waste too much time with setting up the container each time. We can still
> sort out flaky parts into separate jobs later if necessary.

The container setup / job prep is not a significant time sink  - just
at the 1 minute mark. I wanted to have separate jobs per backend so
that the pipeline dashboard gives an informative summary of which
block backends are impacted by the change, rather than having to
search the logs to discover it.

Flaky parts shouldn't be separate jobs  - the tests should either be
fixed or removed, because unreliable tests just end up being ignored,
leading to regressions being missed. 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 0/4] tests: do more testing of block drivers in CI

[Daniel P. Berrangé]

On Wed, Oct 08, 2025 at 12:35:48PM +0100, Daniel P. Berrangé wrote:
> The recent set of regressions identified in the LUKS block driver
> re-inforced that despite having a hugely useful set of I/O tests,
> our CI coverage is still letting through too many bugs.
> 
> This series expands the meson suites / make targets for running
> I/O tests for more drivers, and then creates a CI job for each.
> 
> While the first three patches are functionally usable as is, the
> last patch for gitlab is very much an RFC. A test pipeline
> 
>   https://gitlab.com/berrange/qemu/-/pipelines/2085907042
> 
> shows passes for qcow2, qed, vmdk & vpc, but failures for luks,
> nbd and raw.
> 
> The luks failures all have patchs floating around pending merge.
> 
> The nbd failures are problems with non-deterministic output
> ordering, or warnings from python deprecations that need
> addressing.
> 
> The raw failures are problems with python deprecations, and
> insufficient permissions checks wrt device mapper acess.

The raw / nbd deprecations are:

/usr/lib64/python3.9/asyncio/trsock.py:20: DeprecationWarning: Using sendmsg() method on sockets returned from get_extra_info('socket') will be prohibited in asyncio 3.9. Please report your use case to bugs.python.org.
  warnings.warn(
DeprecationWarning: Enable tracemalloc to get the object allocation traceback


This comes from send_fd_scm in qmp_client.py where it does

        sock = self._writer.transport.get_extra_info('socket')

        if sock.family != socket.AF_UNIX:
            raise QMPError("Sending file descriptors requires a UNIX socket.")

        if not hasattr(sock, 'sendmsg'):
            # We need to void the warranty sticker.
            # Access to sendmsg is scheduled for removal in Python 3.11.
            # Find the real backing socket to use it anyway.
            sock = sock._sock  # pylint: disable=protected-access

        sock.sendmsg(
            [b' '],
            [(socket.SOL_SOCKET, socket.SCM_RIGHTS, struct.pack('@i', fd))]
        )


As that comment says, the 'sendmsg' method has been removed
from the TransportSock object returned by 'get_extra_info',
with asyncio offering no replacement. The code above grabs
the private sock._sock handle, which is rather gross, but
we lack better options.

The problem in CI is that I picked CentOS 9 which has py 3.9
which still has the 'sendmsg' method. On newer distros
'sendmsg' is gone, so we grab the private sock._sock and
so dno't see a deprecation message.

IMHO we need to revert this previous change, so we avoid
calling the deprecated method at all:

  commit a57cb3e23d5ac918a69d0aab918470ff0b429ff9
  Author: John Snow <jsnow@redhat.com>
  Date:   Thu Nov 18 15:46:20 2021 -0500

    python/aqmp: fix send_fd_scm for python 3.6.x
    
    3.6 doesn't play keepaway with the socket object, so we don't need to go
    fishing for it on this version. In fact, so long as 'sendmsg' is still
    available, it's probably preferable to just use that method and only go
    fishing for forbidden details when we absolutely have to.
    
    Reported-by: Thomas Huth <thuth@redhat.com>
    Signed-off-by: John Snow <jsnow@redhat.com>
    Reviewed-by: Willian Rampazzo <willianr@redhat.com>
    Message-id: 20211118204620.1897674-8-jsnow@redhat.com
    Signed-off-by: John Snow <jsnow@redhat.com>

  diff --git a/python/qemu/aqmp/qmp_client.py b/python/qemu/aqmp/qmp_client.py
  index f987da02eb..8105e29fa8 100644
  --- a/python/qemu/aqmp/qmp_client.py
  +++ b/python/qemu/aqmp/qmp_client.py
  @@ -639,9 +639,12 @@ def send_fd_scm(self, fd: int) -> None:
           if sock.family != socket.AF_UNIX:
               raise AQMPError("Sending file descriptors requires a UNIX socket.")
 
  -        # Void the warranty sticker.
  -        # Access to sendmsg in asyncio is scheduled for removal in Python 3.11.
  -        sock = sock._sock  # pylint: disable=protected-access
  +        if not hasattr(sock, 'sendmsg'):
  +            # We need to void the warranty sticker.
  +            # Access to sendmsg is scheduled for removal in Python 3.11.
  +            # Find the real backing socket to use it anyway.
  +            sock = sock._sock  # pylint: disable=protected-access
  +

The need for 3.6 compat no longer exists since we bumped
min python, so that justification for that original
commit is now obsolete.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|