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

Daniel P. Berrangé posted 4 patches 1 month, 1 week ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Thomas Huth <thuth@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
[PATCH 3/4] tests: add nbd and luks to the I/O test suites
Posted by Daniel P. Berrangé 1 month, 1 week ago
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
Posted by Eric Blake 1 month, 1 week ago
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


Re: [PATCH 3/4] tests: add nbd and luks to the I/O test suites
Posted by Thomas Huth 1 month, 1 week ago
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
Posted by Daniel P. Berrangé 1 month, 1 week ago
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
Posted by Eric Blake 1 month, 1 week ago
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
Posted by Kevin Wolf 1 month ago
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
Posted by Eric Blake 1 month ago
On Tue, Oct 14, 2025 at 10:50:31AM +0200, Kevin Wolf wrote:
> 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?

There are some tests that run differently under '-nbd -qcow2' than
under '-nbd -file'; but you are also correct that any qcow2 test that
relies on image resizing can't work with nbd.

> 
> 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).

Yeah, that does sound better...

> 
> 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.

...but I'm not opposed to inertia sticking to a slightly inaccurate
name over the pain of churn just to change the name.  It's not
something that Coccinelle can automate.

-- 
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
Posted by Thomas Huth 1 month, 1 week ago
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