diff mbox

[3/3] qemu-iotests: require CONFIG_LINUX_AIO for test 087

Message ID 20170721034730.25612-4-crosa@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Cleber Rosa July 21, 2017, 3:47 a.m. UTC
One of the "sub-"tests of test 087 requires CONFIG_LINUX_AIO.

As a PoC/RFC, this goes the easy route and skips the test as a whole
when that feature is missing.  Other approaches include splitting
the test and adding extra filtering.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/qemu-iotests/087 | 1 +
 1 file changed, 1 insertion(+)

Comments

Jing Liu July 24, 2017, 6:44 a.m. UTC | #1
Hi Cleber,


On 2017/7/21 上午11:47, Cleber Rosa wrote:
> One of the "sub-"tests of test 087 requires CONFIG_LINUX_AIO.
>
> As a PoC/RFC, this goes the easy route and skips the test as a whole
> when that feature is missing.  Other approaches include splitting
> the test and adding extra filtering.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/qemu-iotests/087 | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
> index f8e4903..a2fb7de 100755
> --- a/tests/qemu-iotests/087
> +++ b/tests/qemu-iotests/087
> @@ -34,6 +34,7 @@ status=1	# failure is the default!
>   _supported_fmt qcow2
>   _supported_proto file
>   _supported_os Linux
> +_require_feature CONFIG_LINUX_AIO
I tested that CONFIG_NETTLE_KDF is also a necessary for 087.

+_require_feature CONFIG_NETTLE_KDF


>
>   function do_run_qemu()
>   {
Stefan Hajnoczi July 25, 2017, 3:45 p.m. UTC | #2
On Mon, Jul 24, 2017 at 02:44:13PM +0800, Jing Liu wrote:
> On 2017/7/21 上午11:47, Cleber Rosa wrote:
> > One of the "sub-"tests of test 087 requires CONFIG_LINUX_AIO.
> > 
> > As a PoC/RFC, this goes the easy route and skips the test as a whole
> > when that feature is missing.  Other approaches include splitting
> > the test and adding extra filtering.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/qemu-iotests/087 | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
> > index f8e4903..a2fb7de 100755
> > --- a/tests/qemu-iotests/087
> > +++ b/tests/qemu-iotests/087
> > @@ -34,6 +34,7 @@ status=1	# failure is the default!
> >   _supported_fmt qcow2
> >   _supported_proto file
> >   _supported_os Linux
> > +_require_feature CONFIG_LINUX_AIO
> I tested that CONFIG_NETTLE_KDF is also a necessary for 087.
> 
> +_require_feature CONFIG_NETTLE_KDF

Are you sure?  Looks like either nettle or gcrypt is needed:

crypto/Makefile.objs:crypto-obj-$(CONFIG_NETTLE_KDF) += pbkdf-nettle.o
crypto/Makefile.objs:crypto-obj-$(if $(CONFIG_NETTLE_KDF),n,$(CONFIG_GCRYPT_KDF)) += pbkdf-gcrypt.o

But this shows why the compile-time testing of features is ugly:

1. It duplicates build dependency logic into the test cases.

2. The test cases don't care about nettle vs gcrypt and they shouldn't
   have to know about it.  They just care whether LUKS is available or
   not.

As I mentioned earlier, let's test whether the QEMU binaries offer
_features_, not which _config options_ they were built with.
Daniel P. Berrangé July 25, 2017, 3:48 p.m. UTC | #3
On Tue, Jul 25, 2017 at 04:45:46PM +0100, Stefan Hajnoczi wrote:
> On Mon, Jul 24, 2017 at 02:44:13PM +0800, Jing Liu wrote:
> > On 2017/7/21 上午11:47, Cleber Rosa wrote:
> > > One of the "sub-"tests of test 087 requires CONFIG_LINUX_AIO.
> > > 
> > > As a PoC/RFC, this goes the easy route and skips the test as a whole
> > > when that feature is missing.  Other approaches include splitting
> > > the test and adding extra filtering.
> > > 
> > > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > > ---
> > >   tests/qemu-iotests/087 | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
> > > index f8e4903..a2fb7de 100755
> > > --- a/tests/qemu-iotests/087
> > > +++ b/tests/qemu-iotests/087
> > > @@ -34,6 +34,7 @@ status=1	# failure is the default!
> > >   _supported_fmt qcow2
> > >   _supported_proto file
> > >   _supported_os Linux
> > > +_require_feature CONFIG_LINUX_AIO
> > I tested that CONFIG_NETTLE_KDF is also a necessary for 087.
> > 
> > +_require_feature CONFIG_NETTLE_KDF
> 
> Are you sure?  Looks like either nettle or gcrypt is needed:

Correct, it works with either.

> crypto/Makefile.objs:crypto-obj-$(CONFIG_NETTLE_KDF) += pbkdf-nettle.o
> crypto/Makefile.objs:crypto-obj-$(if $(CONFIG_NETTLE_KDF),n,$(CONFIG_GCRYPT_KDF)) += pbkdf-gcrypt.o
> 
> But this shows why the compile-time testing of features is ugly:
> 
> 1. It duplicates build dependency logic into the test cases.
> 
> 2. The test cases don't care about nettle vs gcrypt and they shouldn't
>    have to know about it.  They just care whether LUKS is available or
>    not.

Yeah that knowledge is messy and fragile wrt future changes.


Regards,
Daniel
Jing Liu July 26, 2017, 8:55 a.m. UTC | #4
On 2017/7/25 下午11:48, Daniel P. Berrange wrote:
> On Tue, Jul 25, 2017 at 04:45:46PM +0100, Stefan Hajnoczi wrote:
>> On Mon, Jul 24, 2017 at 02:44:13PM +0800, Jing Liu wrote:
>>> On 2017/7/21 上午11:47, Cleber Rosa wrote:
>>>> One of the "sub-"tests of test 087 requires CONFIG_LINUX_AIO.
>>>>
>>>> As a PoC/RFC, this goes the easy route and skips the test as a whole
>>>> when that feature is missing.  Other approaches include splitting
>>>> the test and adding extra filtering.
>>>>
>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>>> ---
>>>>    tests/qemu-iotests/087 | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
>>>> index f8e4903..a2fb7de 100755
>>>> --- a/tests/qemu-iotests/087
>>>> +++ b/tests/qemu-iotests/087
>>>> @@ -34,6 +34,7 @@ status=1	# failure is the default!
>>>>    _supported_fmt qcow2
>>>>    _supported_proto file
>>>>    _supported_os Linux
>>>> +_require_feature CONFIG_LINUX_AIO
>>> I tested that CONFIG_NETTLE_KDF is also a necessary for 087.
>>>
>>> +_require_feature CONFIG_NETTLE_KDF
>> Are you sure?  Looks like either nettle or gcrypt is needed:
> Correct, it works with either.
Ah, because I just found out nettle which related to KDF.
why can not find out gcrypt.h in qemu?
How to compile config_gcrypt_kdf into qemu?
>
>> crypto/Makefile.objs:crypto-obj-$(CONFIG_NETTLE_KDF) += pbkdf-nettle.o
>> crypto/Makefile.objs:crypto-obj-$(if $(CONFIG_NETTLE_KDF),n,$(CONFIG_GCRYPT_KDF)) += pbkdf-gcrypt.o
>>
>> But this shows why the compile-time testing of features is ugly:
>>
>> 1. It duplicates build dependency logic into the test cases.
>>
>> 2. The test cases don't care about nettle vs gcrypt and they shouldn't
>>     have to know about it.  They just care whether LUKS is available or
>>     not.
> Yeah that knowledge is messy and fragile wrt future changes.
>
>
> Regards,
> Daniel
Daniel P. Berrangé July 26, 2017, 9:49 a.m. UTC | #5
On Wed, Jul 26, 2017 at 04:55:39PM +0800, Jing Liu wrote:
> 
> 
> On 2017/7/25 下午11:48, Daniel P. Berrange wrote:
> > On Tue, Jul 25, 2017 at 04:45:46PM +0100, Stefan Hajnoczi wrote:
> > > On Mon, Jul 24, 2017 at 02:44:13PM +0800, Jing Liu wrote:
> > > > On 2017/7/21 上午11:47, Cleber Rosa wrote:
> > > > > One of the "sub-"tests of test 087 requires CONFIG_LINUX_AIO.
> > > > > 
> > > > > As a PoC/RFC, this goes the easy route and skips the test as a whole
> > > > > when that feature is missing.  Other approaches include splitting
> > > > > the test and adding extra filtering.
> > > > > 
> > > > > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > > > > ---
> > > > >    tests/qemu-iotests/087 | 1 +
> > > > >    1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
> > > > > index f8e4903..a2fb7de 100755
> > > > > --- a/tests/qemu-iotests/087
> > > > > +++ b/tests/qemu-iotests/087
> > > > > @@ -34,6 +34,7 @@ status=1	# failure is the default!
> > > > >    _supported_fmt qcow2
> > > > >    _supported_proto file
> > > > >    _supported_os Linux
> > > > > +_require_feature CONFIG_LINUX_AIO
> > > > I tested that CONFIG_NETTLE_KDF is also a necessary for 087.
> > > > 
> > > > +_require_feature CONFIG_NETTLE_KDF
> > > Are you sure?  Looks like either nettle or gcrypt is needed:
> > Correct, it works with either.
> Ah, because I just found out nettle which related to KDF.
> why can not find out gcrypt.h in qemu?
> How to compile config_gcrypt_kdf into qemu?

QEMU picks either nettle or libgcrypt automatically, dependant on which
of these gnutls is linked to.

You can force override this via  --disable-nettle --enable-gcrypt
(or vica-verca), for sake of testing, but you shouldn't do that
for production builds.


Regards,
Daniel
diff mbox

Patch

diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index f8e4903..a2fb7de 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -34,6 +34,7 @@  status=1	# failure is the default!
 _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
+_require_feature CONFIG_LINUX_AIO
 
 function do_run_qemu()
 {