[PATCH v2 0/2] crypto: fix build with gcrypt

[PATCH v2 0/2] crypto: fix build with gcrypt

[Daniel P. Berrangé]

The build system failed to add gcrypt flags and also didn't link to
gnutls in all scenarios.  This was missed because of the lack of CI
coverage for various build scenarios

Changed in v2:

 - Change way we add library dependencies in meson rules
   to fix linux-user build with gcrypt/gnutls too.
 - Extend CI coverage to test 1 system and 1 linux-user
   build with each crypto combination, not merely tools.

Daniel P. Berrangé (2):
  crypto: fix build with gcrypt enabled
  gitlab: expand test coverage for crypto builds

 .gitlab-ci.yml                          | 69 +++++++++++++++++++++++++
 configure                               |  2 +
 crypto/meson.build                      | 42 +++++++++++----
 meson.build                             |  5 ++
 tests/docker/dockerfiles/centos7.docker |  2 +
 tests/docker/dockerfiles/centos8.docker |  1 +
 6 files changed, 110 insertions(+), 11 deletions(-)

-- 
2.26.2

[PATCH v2 1/2] crypto: fix build with gcrypt enabled

[Daniel P. Berrangé]

If nettle is disabled and gcrypt enabled, the compiler and linker flags
needed for gcrypt are not passed.

Gnutls was also not added as a dependancy when gcrypt is enabled.

Attempting to add the library dependencies at the same time as the
source dependencies is error prone, as there are alot of different
rules for picking which sources to use, and some of the source files
use code level conditionals intead. It is thus clearer to add the
library dependencies separately.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 configure          |  2 ++
 crypto/meson.build | 42 +++++++++++++++++++++++++++++++-----------
 meson.build        |  5 +++++
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index 8dc981684b..3dc2431722 100755
--- a/configure
+++ b/configure
@@ -6979,6 +6979,8 @@ if test "$gcrypt" = "yes" ; then
   if test "$gcrypt_hmac" = "yes" ; then
     echo "CONFIG_GCRYPT_HMAC=y" >> $config_host_mak
   fi
+  echo "GCRYPT_CFLAGS=$gcrypt_cflags" >> $config_host_mak
+  echo "GCRYPT_LIBS=$gcrypt_libs" >> $config_host_mak
 fi
 if test "$nettle" = "yes" ; then
   echo "CONFIG_NETTLE=y" >> $config_host_mak
diff --git a/crypto/meson.build b/crypto/meson.build
index 18da7c8541..f6f5ce1ecd 100644
--- a/crypto/meson.build
+++ b/crypto/meson.build
@@ -23,24 +23,35 @@ crypto_ss.add(files(
   'tlssession.c',
 ))
 
-if 'CONFIG_GCRYPT' in config_host
-  wo_nettle = files('hash-gcrypt.c', 'pbkdf-gcrypt.c')
+if 'CONFIG_NETTLE' in config_host
+  crypto_ss.add(files('hash-nettle.c', 'hmac-nettle.c', 'pbkdf-nettle.c'))
+elif 'CONFIG_GCRYPT' in config_host
+  crypto_ss.add(files('hash-gcrypt.c', 'pbkdf-gcrypt.c'))
+  if 'CONFIG_GCRYPT_HMAC' in config_host
+    crypto_ss.add(files('hmac-gcrypt.c'))
+  else
+    crypto_ss.add(files('hmac-glib.c'))
+  endif
 else
-  wo_nettle = files('hash-glib.c', 'pbkdf-stub.c')
-endif
-if 'CONFIG_GCRYPT_HMAC' not in config_host
-  wo_nettle += files('hmac-glib.c')
+  crypto_ss.add(files('hash-glib.c', 'hmac-glib.c', 'pbkdf-stub.c'))
 endif
-crypto_ss.add(when: [nettle, 'CONFIG_NETTLE'],
-             if_true: files('hash-nettle.c', 'hmac-nettle.c', 'pbkdf-nettle.c'),
-             if_false: wo_nettle)
 
 crypto_ss.add(when: 'CONFIG_SECRET_KEYRING', if_true: files('secret_keyring.c'))
 crypto_ss.add(when: 'CONFIG_QEMU_PRIVATE_XTS', if_true: files('xts.c'))
-crypto_ss.add(when: 'CONFIG_GCRYPT_HMAC', if_true: files('hmac-gcrypt.c'))
 crypto_ss.add(when: 'CONFIG_AF_ALG', if_true: files('afalg.c', 'cipher-afalg.c', 'hash-afalg.c'))
 crypto_ss.add(when: 'CONFIG_GNUTLS', if_true: files('tls-cipher-suites.c'))
 
+if 'CONFIG_NETTLE' in config_host
+  crypto_ss.add(nettle)
+elif 'CONFIG_GCRYPT' in config_host
+  crypto_ss.add(gcrypt)
+endif
+
+if 'CONFIG_GNUTLS' in config_host
+  crypto_ss.add(gnutls)
+endif
+
+
 crypto_ss = crypto_ss.apply(config_host, strict: false)
 libcrypto = static_library('crypto', crypto_ss.sources() + genh,
                            dependencies: [crypto_ss.dependencies()],
@@ -52,12 +63,21 @@ crypto = declare_dependency(link_whole: libcrypto,
 
 util_ss.add(files('aes.c'))
 util_ss.add(files('init.c'))
+
 if 'CONFIG_GCRYPT' in config_host
   util_ss.add(files('random-gcrypt.c'))
 elif 'CONFIG_GNUTLS' in config_host
-  util_ss.add(files('random-gnutls.c'), gnutls)
+  util_ss.add(files('random-gnutls.c'))
 elif 'CONFIG_RNG_NONE' in config_host
   util_ss.add(files('random-none.c'))
 else
   util_ss.add(files('random-platform.c'))
 endif
+
+if 'CONFIG_GCRYPT' in config_host
+  util_ss.add(gcrypt)
+endif
+
+if 'CONFIG_GNUTLS' in config_host
+  util_ss.add(gnutls)
+endif
diff --git a/meson.build b/meson.build
index 1e7aee85e3..bc6aac4ce7 100644
--- a/meson.build
+++ b/meson.build
@@ -114,6 +114,11 @@ urcubp = not_found
 if 'CONFIG_TRACE_UST' in config_host
   urcubp = declare_dependency(link_args: config_host['URCU_BP_LIBS'].split())
 endif
+gcrypt = not_found
+if 'CONFIG_GCRYPT' in config_host
+  gcrypt = declare_dependency(compile_args: config_host['GCRYPT_CFLAGS'].split(),
+                              link_args: config_host['GCRYPT_LIBS'].split())
+endif
 nettle = not_found
 if 'CONFIG_NETTLE' in config_host
   nettle = declare_dependency(compile_args: config_host['NETTLE_CFLAGS'].split(),
-- 
2.26.2

[PATCH v2 2/2] gitlab: expand test coverage for crypto builds

[Daniel P. Berrangé]

Most jobs test the latest nettle library. This adds explicit coverage
for latest gcrypt using Fedora, and old gcrypt and nettle using
CentOS-7. The latter does a minimal tools-only build, as we only need to
validate that the crypto code builds and unit tests pass. Finally a job
disabling both nettle and gcrypt is provided to validate that gnutls
still works.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 .gitlab-ci.yml                          | 69 +++++++++++++++++++++++++
 tests/docker/dockerfiles/centos7.docker |  2 +
 tests/docker/dockerfiles/centos8.docker |  1 +
 3 files changed, 72 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index b7967b9a13..a74b16ff04 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -130,6 +130,7 @@ build-system-fedora:
   <<: *native_build_job_definition
   variables:
     IMAGE: fedora
+    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
     TARGETS: tricore-softmmu unicore32-softmmu microblaze-softmmu mips-softmmu
       xtensa-softmmu m68k-softmmu riscv32-softmmu ppc-softmmu sparc64-softmmu
     MAKE_CHECK_ARGS: check-build
@@ -160,6 +161,7 @@ build-system-centos:
   <<: *native_build_job_definition
   variables:
     IMAGE: centos8
+    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
     TARGETS: ppc64-softmmu lm32-softmmu or1k-softmmu s390x-softmmu
       x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
     MAKE_CHECK_ARGS: check-build
@@ -196,6 +198,7 @@ build-disabled:
       --disable-guest-agent --disable-curses --disable-libxml2 --disable-tpm
       --disable-qom-cast-debug --disable-spice --disable-vhost-vsock
       --disable-vhost-net --disable-vhost-crypto --disable-vhost-user
+      --disable-nettle --disable-gcrypt --disable-gnutls
     TARGETS: i386-softmmu ppc64-softmmu mips64-softmmu i386-linux-user
     MAKE_CHECK_ARGS: check-qtest SPEED=slow
 
@@ -271,3 +274,69 @@ build-tci:
       done
     - QTEST_QEMU_BINARY="./qemu-system-x86_64" ./tests/qtest/pxe-test
     - QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
+
+# Most jobs test latest gcrypt or nettle builds
+#
+# These jobs test old gcrypt and nettle from RHEL7
+# which had some API differences.
+build-crypto-old-nettle:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: centos7
+    TARGETS: x86_64-softmmu x86_64-linux-user
+    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
+    MAKE_CHECK_ARGS: check-build
+  artifacts:
+    paths:
+      - build
+
+check-crypto-old-nettle:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-crypto-old-nettle
+      artifacts: true
+  variables:
+    IMAGE: centos7
+    MAKE_CHECK_ARGS: check
+
+
+build-crypto-old-gcrypt:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: centos7
+    TARGETS: x86_64-softmmu x86_64-linux-user
+    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
+    MAKE_CHECK_ARGS: check-build
+  artifacts:
+    paths:
+      - build
+
+check-crypto-old-gcrypt:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-crypto-old-gcrypt
+      artifacts: true
+  variables:
+    IMAGE: centos7
+    MAKE_CHECK_ARGS: check
+
+
+build-crypto-only-gnutls:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: centos7
+    TARGETS: x86_64-softmmu x86_64-linux-user
+    CONFIGURE_ARGS: --disable-nettle --disable-gcrypt --enable-gnutls
+    MAKE_CHECK_ARGS: check-build
+  artifacts:
+    paths:
+      - build
+
+check-crypto-only-gnutls:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-crypto-only-gnutls
+      artifacts: true
+  variables:
+    IMAGE: centos7
+    MAKE_CHECK_ARGS: check
diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker
index e197acdc3c..46277773bf 100644
--- a/tests/docker/dockerfiles/centos7.docker
+++ b/tests/docker/dockerfiles/centos7.docker
@@ -15,9 +15,11 @@ ENV PACKAGES \
     gettext \
     git \
     glib2-devel \
+    gnutls-devel \
     libaio-devel \
     libepoxy-devel \
     libfdt-devel \
+    libgcrypt-devel \
     librdmacm-devel \
     libzstd-devel \
     lzo-devel \
diff --git a/tests/docker/dockerfiles/centos8.docker b/tests/docker/dockerfiles/centos8.docker
index 9852c5b9ee..f435616d6a 100644
--- a/tests/docker/dockerfiles/centos8.docker
+++ b/tests/docker/dockerfiles/centos8.docker
@@ -13,6 +13,7 @@ ENV PACKAGES \
     glib2-devel \
     libaio-devel \
     libepoxy-devel \
+    libgcrypt-devel \
     lzo-devel \
     make \
     mesa-libEGL-devel \
-- 
2.26.2

Re: [PATCH v2 1/2] crypto: fix build with gcrypt enabled

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> If nettle is disabled and gcrypt enabled, the compiler and linker flags
> needed for gcrypt are not passed.
>
> Gnutls was also not added as a dependancy when gcrypt is enabled.
>
> Attempting to add the library dependencies at the same time as the
> source dependencies is error prone, as there are alot of different
> rules for picking which sources to use, and some of the source files
> use code level conditionals intead. It is thus clearer to add the
> library dependencies separately.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

That seems to fix some of the container based builds.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  configure          |  2 ++
>  crypto/meson.build | 42 +++++++++++++++++++++++++++++++-----------
>  meson.build        |  5 +++++
>  3 files changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/configure b/configure
> index 8dc981684b..3dc2431722 100755
> --- a/configure
> +++ b/configure
> @@ -6979,6 +6979,8 @@ if test "$gcrypt" = "yes" ; then
>    if test "$gcrypt_hmac" = "yes" ; then
>      echo "CONFIG_GCRYPT_HMAC=y" >> $config_host_mak
>    fi
> +  echo "GCRYPT_CFLAGS=$gcrypt_cflags" >> $config_host_mak
> +  echo "GCRYPT_LIBS=$gcrypt_libs" >> $config_host_mak
>  fi
>  if test "$nettle" = "yes" ; then
>    echo "CONFIG_NETTLE=y" >> $config_host_mak
> diff --git a/crypto/meson.build b/crypto/meson.build
> index 18da7c8541..f6f5ce1ecd 100644
> --- a/crypto/meson.build
> +++ b/crypto/meson.build
> @@ -23,24 +23,35 @@ crypto_ss.add(files(
>    'tlssession.c',
>  ))
>  
> -if 'CONFIG_GCRYPT' in config_host
> -  wo_nettle = files('hash-gcrypt.c', 'pbkdf-gcrypt.c')
> +if 'CONFIG_NETTLE' in config_host
> +  crypto_ss.add(files('hash-nettle.c', 'hmac-nettle.c', 'pbkdf-nettle.c'))
> +elif 'CONFIG_GCRYPT' in config_host
> +  crypto_ss.add(files('hash-gcrypt.c', 'pbkdf-gcrypt.c'))
> +  if 'CONFIG_GCRYPT_HMAC' in config_host
> +    crypto_ss.add(files('hmac-gcrypt.c'))
> +  else
> +    crypto_ss.add(files('hmac-glib.c'))
> +  endif
>  else
> -  wo_nettle = files('hash-glib.c', 'pbkdf-stub.c')
> -endif
> -if 'CONFIG_GCRYPT_HMAC' not in config_host
> -  wo_nettle += files('hmac-glib.c')
> +  crypto_ss.add(files('hash-glib.c', 'hmac-glib.c', 'pbkdf-stub.c'))
>  endif
> -crypto_ss.add(when: [nettle, 'CONFIG_NETTLE'],
> -             if_true: files('hash-nettle.c', 'hmac-nettle.c', 'pbkdf-nettle.c'),
> -             if_false: wo_nettle)
>  
>  crypto_ss.add(when: 'CONFIG_SECRET_KEYRING', if_true: files('secret_keyring.c'))
>  crypto_ss.add(when: 'CONFIG_QEMU_PRIVATE_XTS', if_true: files('xts.c'))
> -crypto_ss.add(when: 'CONFIG_GCRYPT_HMAC', if_true: files('hmac-gcrypt.c'))
>  crypto_ss.add(when: 'CONFIG_AF_ALG', if_true: files('afalg.c', 'cipher-afalg.c', 'hash-afalg.c'))
>  crypto_ss.add(when: 'CONFIG_GNUTLS', if_true: files('tls-cipher-suites.c'))
>  
> +if 'CONFIG_NETTLE' in config_host
> +  crypto_ss.add(nettle)
> +elif 'CONFIG_GCRYPT' in config_host
> +  crypto_ss.add(gcrypt)
> +endif
> +
> +if 'CONFIG_GNUTLS' in config_host
> +  crypto_ss.add(gnutls)
> +endif
> +
> +
>  crypto_ss = crypto_ss.apply(config_host, strict: false)
>  libcrypto = static_library('crypto', crypto_ss.sources() + genh,
>                             dependencies: [crypto_ss.dependencies()],
> @@ -52,12 +63,21 @@ crypto = declare_dependency(link_whole: libcrypto,
>  
>  util_ss.add(files('aes.c'))
>  util_ss.add(files('init.c'))
> +
>  if 'CONFIG_GCRYPT' in config_host
>    util_ss.add(files('random-gcrypt.c'))
>  elif 'CONFIG_GNUTLS' in config_host
> -  util_ss.add(files('random-gnutls.c'), gnutls)
> +  util_ss.add(files('random-gnutls.c'))
>  elif 'CONFIG_RNG_NONE' in config_host
>    util_ss.add(files('random-none.c'))
>  else
>    util_ss.add(files('random-platform.c'))
>  endif
> +
> +if 'CONFIG_GCRYPT' in config_host
> +  util_ss.add(gcrypt)
> +endif
> +
> +if 'CONFIG_GNUTLS' in config_host
> +  util_ss.add(gnutls)
> +endif
> diff --git a/meson.build b/meson.build
> index 1e7aee85e3..bc6aac4ce7 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -114,6 +114,11 @@ urcubp = not_found
>  if 'CONFIG_TRACE_UST' in config_host
>    urcubp = declare_dependency(link_args: config_host['URCU_BP_LIBS'].split())
>  endif
> +gcrypt = not_found
> +if 'CONFIG_GCRYPT' in config_host
> +  gcrypt = declare_dependency(compile_args: config_host['GCRYPT_CFLAGS'].split(),
> +                              link_args: config_host['GCRYPT_LIBS'].split())
> +endif
>  nettle = not_found
>  if 'CONFIG_NETTLE' in config_host
>    nettle = declare_dependency(compile_args: config_host['NETTLE_CFLAGS'].split(),


-- 
Alex Bennée

Re: [PATCH v2 2/2] gitlab: expand test coverage for crypto builds

[Philippe Mathieu-Daudé]

On 9/1/20 3:30 PM, Daniel P. Berrangé wrote:
> Most jobs test the latest nettle library. This adds explicit coverage
> for latest gcrypt using Fedora, and old gcrypt and nettle using
> CentOS-7. The latter does a minimal tools-only build, as we only need to
> validate that the crypto code builds and unit tests pass. Finally a job
> disabling both nettle and gcrypt is provided to validate that gnutls
> still works.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  .gitlab-ci.yml                          | 69 +++++++++++++++++++++++++
>  tests/docker/dockerfiles/centos7.docker |  2 +
>  tests/docker/dockerfiles/centos8.docker |  1 +
>  3 files changed, 72 insertions(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index b7967b9a13..a74b16ff04 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -130,6 +130,7 @@ build-system-fedora:
>    <<: *native_build_job_definition
>    variables:
>      IMAGE: fedora
> +    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
>      TARGETS: tricore-softmmu unicore32-softmmu microblaze-softmmu mips-softmmu
>        xtensa-softmmu m68k-softmmu riscv32-softmmu ppc-softmmu sparc64-softmmu
>      MAKE_CHECK_ARGS: check-build
> @@ -160,6 +161,7 @@ build-system-centos:
>    <<: *native_build_job_definition
>    variables:
>      IMAGE: centos8
> +    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
>      TARGETS: ppc64-softmmu lm32-softmmu or1k-softmmu s390x-softmmu
>        x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
>      MAKE_CHECK_ARGS: check-build
> @@ -196,6 +198,7 @@ build-disabled:
>        --disable-guest-agent --disable-curses --disable-libxml2 --disable-tpm
>        --disable-qom-cast-debug --disable-spice --disable-vhost-vsock
>        --disable-vhost-net --disable-vhost-crypto --disable-vhost-user
> +      --disable-nettle --disable-gcrypt --disable-gnutls
>      TARGETS: i386-softmmu ppc64-softmmu mips64-softmmu i386-linux-user
>      MAKE_CHECK_ARGS: check-qtest SPEED=slow
>  
> @@ -271,3 +274,69 @@ build-tci:
>        done
>      - QTEST_QEMU_BINARY="./qemu-system-x86_64" ./tests/qtest/pxe-test
>      - QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
> +
> +# Most jobs test latest gcrypt or nettle builds
> +#
> +# These jobs test old gcrypt and nettle from RHEL7
> +# which had some API differences.
> +build-crypto-old-nettle:
> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: centos7
> +    TARGETS: x86_64-softmmu x86_64-linux-user
> +    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
> +    MAKE_CHECK_ARGS: check-build
> +  artifacts:
> +    paths:
> +      - build
> +
> +check-crypto-old-nettle:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-crypto-old-nettle
> +      artifacts: true
> +  variables:
> +    IMAGE: centos7
> +    MAKE_CHECK_ARGS: check
> +
> +

I'd copy the same comment for each library... In case
we add more jobs in the middle.

> +build-crypto-old-gcrypt:
> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: centos7
> +    TARGETS: x86_64-softmmu x86_64-linux-user
> +    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
> +    MAKE_CHECK_ARGS: check-build
> +  artifacts:
> +    paths:
> +      - build
> +
> +check-crypto-old-gcrypt:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-crypto-old-gcrypt
> +      artifacts: true
> +  variables:
> +    IMAGE: centos7
> +    MAKE_CHECK_ARGS: check
> +
> +
> +build-crypto-only-gnutls:

Aren't these 'old' jobs too (centos 7, not 8)?

> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: centos7
> +    TARGETS: x86_64-softmmu x86_64-linux-user
> +    CONFIGURE_ARGS: --disable-nettle --disable-gcrypt --enable-gnutls
> +    MAKE_CHECK_ARGS: check-build
> +  artifacts:
> +    paths:
> +      - build
> +
> +check-crypto-only-gnutls:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-crypto-only-gnutls
> +      artifacts: true
> +  variables:
> +    IMAGE: centos7
> +    MAKE_CHECK_ARGS: check
> diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker
> index e197acdc3c..46277773bf 100644
> --- a/tests/docker/dockerfiles/centos7.docker
> +++ b/tests/docker/dockerfiles/centos7.docker
> @@ -15,9 +15,11 @@ ENV PACKAGES \
>      gettext \
>      git \
>      glib2-devel \
> +    gnutls-devel \
>      libaio-devel \
>      libepoxy-devel \
>      libfdt-devel \
> +    libgcrypt-devel \
>      librdmacm-devel \
>      libzstd-devel \
>      lzo-devel \

We should try to keep the same set of packages installed (if possible)
in the older distrib supported and in the more recent one. Not sure
what the best way to do that though.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> diff --git a/tests/docker/dockerfiles/centos8.docker b/tests/docker/dockerfiles/centos8.docker
> index 9852c5b9ee..f435616d6a 100644
> --- a/tests/docker/dockerfiles/centos8.docker
> +++ b/tests/docker/dockerfiles/centos8.docker
> @@ -13,6 +13,7 @@ ENV PACKAGES \
>      glib2-devel \
>      libaio-devel \
>      libepoxy-devel \
> +    libgcrypt-devel \
>      lzo-devel \
>      make \
>      mesa-libEGL-devel \
> 

Re: [PATCH v2 2/2] gitlab: expand test coverage for crypto builds

[Daniel P. Berrangé]

On Tue, Sep 01, 2020 at 05:10:20PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/1/20 3:30 PM, Daniel P. Berrangé wrote:
> > Most jobs test the latest nettle library. This adds explicit coverage
> > for latest gcrypt using Fedora, and old gcrypt and nettle using
> > CentOS-7. The latter does a minimal tools-only build, as we only need to
> > validate that the crypto code builds and unit tests pass. Finally a job
> > disabling both nettle and gcrypt is provided to validate that gnutls
> > still works.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  .gitlab-ci.yml                          | 69 +++++++++++++++++++++++++
> >  tests/docker/dockerfiles/centos7.docker |  2 +
> >  tests/docker/dockerfiles/centos8.docker |  1 +
> >  3 files changed, 72 insertions(+)
> > 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index b7967b9a13..a74b16ff04 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -130,6 +130,7 @@ build-system-fedora:
> >    <<: *native_build_job_definition
> >    variables:
> >      IMAGE: fedora
> > +    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
> >      TARGETS: tricore-softmmu unicore32-softmmu microblaze-softmmu mips-softmmu
> >        xtensa-softmmu m68k-softmmu riscv32-softmmu ppc-softmmu sparc64-softmmu
> >      MAKE_CHECK_ARGS: check-build
> > @@ -160,6 +161,7 @@ build-system-centos:
> >    <<: *native_build_job_definition
> >    variables:
> >      IMAGE: centos8
> > +    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
> >      TARGETS: ppc64-softmmu lm32-softmmu or1k-softmmu s390x-softmmu
> >        x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
> >      MAKE_CHECK_ARGS: check-build
> > @@ -196,6 +198,7 @@ build-disabled:
> >        --disable-guest-agent --disable-curses --disable-libxml2 --disable-tpm
> >        --disable-qom-cast-debug --disable-spice --disable-vhost-vsock
> >        --disable-vhost-net --disable-vhost-crypto --disable-vhost-user
> > +      --disable-nettle --disable-gcrypt --disable-gnutls
> >      TARGETS: i386-softmmu ppc64-softmmu mips64-softmmu i386-linux-user
> >      MAKE_CHECK_ARGS: check-qtest SPEED=slow
> >  
> > @@ -271,3 +274,69 @@ build-tci:
> >        done
> >      - QTEST_QEMU_BINARY="./qemu-system-x86_64" ./tests/qtest/pxe-test
> >      - QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
> > +
> > +# Most jobs test latest gcrypt or nettle builds
> > +#
> > +# These jobs test old gcrypt and nettle from RHEL7
> > +# which had some API differences.
> > +build-crypto-old-nettle:
> > +  <<: *native_build_job_definition
> > +  variables:
> > +    IMAGE: centos7
> > +    TARGETS: x86_64-softmmu x86_64-linux-user
> > +    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
> > +    MAKE_CHECK_ARGS: check-build
> > +  artifacts:
> > +    paths:
> > +      - build
> > +
> > +check-crypto-old-nettle:
> > +  <<: *native_test_job_definition
> > +  needs:
> > +    - job: build-crypto-old-nettle
> > +      artifacts: true
> > +  variables:
> > +    IMAGE: centos7
> > +    MAKE_CHECK_ARGS: check
> > +
> > +
> 
> I'd copy the same comment for each library... In case
> we add more jobs in the middle.
> 
> > +build-crypto-old-gcrypt:
> > +  <<: *native_build_job_definition
> > +  variables:
> > +    IMAGE: centos7
> > +    TARGETS: x86_64-softmmu x86_64-linux-user
> > +    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
> > +    MAKE_CHECK_ARGS: check-build
> > +  artifacts:
> > +    paths:
> > +      - build
> > +
> > +check-crypto-old-gcrypt:
> > +  <<: *native_test_job_definition
> > +  needs:
> > +    - job: build-crypto-old-gcrypt
> > +      artifacts: true
> > +  variables:
> > +    IMAGE: centos7
> > +    MAKE_CHECK_ARGS: check
> > +
> > +
> > +build-crypto-only-gnutls:
> 
> Aren't these 'old' jobs too (centos 7, not 8)?

It doesn't matter what distro this job builds on - centos 7 was
essentially just a cut+paste choice. The key point is this is only
enabling GNUTLS - the age of gnutls/gcrypt/nettle doesn't matter.

> 
> > +  <<: *native_build_job_definition
> > +  variables:
> > +    IMAGE: centos7
> > +    TARGETS: x86_64-softmmu x86_64-linux-user
> > +    CONFIGURE_ARGS: --disable-nettle --disable-gcrypt --enable-gnutls
> > +    MAKE_CHECK_ARGS: check-build
> > +  artifacts:
> > +    paths:
> > +      - build
> > +
> > +check-crypto-only-gnutls:
> > +  <<: *native_test_job_definition
> > +  needs:
> > +    - job: build-crypto-only-gnutls
> > +      artifacts: true
> > +  variables:
> > +    IMAGE: centos7
> > +    MAKE_CHECK_ARGS: check

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