[PATCH v1 0/8] some testing and CI updates (re-greening)

[PATCH v1 0/8] some testing and CI updates (re-greening)

[Alex Bennée]

Hi,

My first series after a holiday is a bunch of clean-ups for testing.
Currently they all apply on top of Thomas' pull-request-2020-09-02 tag
which is currently in flight. The fixes to shippable won't become
apparent until the main registry has been updates with the new images
but I have tested them locally.

The migration and mips fixes where just quick band-aids so I would
appreciate the appropriate maintainers having a closer look.

With these we get back to a mostly green state although there seem to
be some acceptance tests that need updating.

Alex Bennée (5):
  CODING_STYLE.rst: flesh out our naming conventions.
  tests/docker: add python3-setuptools the docker images
  tests/meson.build: fp tests don't need CONFIG_TCG
  target/mips: simplify gen_compute_imm_branch logic
  migration: use pstrcpy to copy run state

Daniel P. Berrangé (1):
  crypto: fix build with gcrypt enabled

Gerd Hoffmann (1):
  usb-host: restrict workaround to new libusb versions

Paolo Bonzini (1):
  qemu-iotests: move check-block back to Makefiles

 CODING_STYLE.rst                         | 31 +++++++++++++++--
 configure                                |  2 ++
 hw/usb/host-libusb.c                     |  2 +-
 migration/global_state.c                 |  4 +--
 target/mips/translate.c                  | 12 ++-----
 .travis.yml                              |  2 +-
 crypto/meson.build                       | 42 +++++++++++++++++-------
 meson.build                              |  7 ++--
 tests/Makefile.include                   | 15 +++++++--
 tests/docker/dockerfiles/debian10.docker |  1 +
 tests/docker/dockerfiles/debian9.docker  |  1 +
 tests/meson.build                        |  3 +-
 tests/qemu-iotests/meson.build           |  4 ---
 13 files changed, 89 insertions(+), 37 deletions(-)

-- 
2.20.1

[PATCH v1 1/8] CODING_STYLE.rst: flesh out our naming conventions.

[Alex Bennée]

Mention a few of the more common naming conventions we follow in the
code base including common variable names and function prefix and
suffix examples.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - punctuation fixes suggested by Cornelia
  - re-worded section on qemu_ prefix
  - expanded on _locked suffix
v3
  - re-order phrasing around qemu_ prefix
  - drop "while actual..." shortname examples

squash! CODING_STYLE.rst: flesh out our naming conventions.
---
 CODING_STYLE.rst | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/CODING_STYLE.rst b/CODING_STYLE.rst
index 427699e0e42..fd8ce9ceaba 100644
--- a/CODING_STYLE.rst
+++ b/CODING_STYLE.rst
@@ -109,8 +109,35 @@ names are lower_case_with_underscores_ending_with_a_t, like the POSIX
 uint64_t and family.  Note that this last convention contradicts POSIX
 and is therefore likely to be changed.
 
-When wrapping standard library functions, use the prefix ``qemu_`` to alert
-readers that they are seeing a wrapped version; otherwise avoid this prefix.
+Variable Naming Conventions
+---------------------------
+
+A number of short naming conventions exist for variables that use
+common QEMU types. For example, the architecture independent CPUState
+is often held as a ``cs`` pointer variable, whereas the concrete
+CPUArchState is usually held in a pointer called ``env``.
+
+Likewise, in device emulation code the common DeviceState is usually
+called ``dev``.
+
+Function Naming Conventions
+---------------------------
+
+The ``qemu_`` prefix is used for utility functions that are widely
+called from across the code-base. This includes wrapped versions of
+standard library functions (e.g. ``qemu_strtol``) where the prefix is
+added to the library function name to alert readers that they are
+seeing a wrapped version.
+
+Public functions from a file or subsystem (declared in headers) tend
+to have a consistent prefix to show where they came from. For example,
+``tlb_`` for functions from ``cputlb.c`` or ``cpu_`` for functions
+from cpus.c.
+
+If there are two versions of a function to be called with or without a
+lock held, the function that expects the lock to be already held
+usually uses the suffix ``_locked``.
+
 
 Block structure
 ===============
-- 
2.20.1

[PATCH v1 2/8] crypto: fix build with gcrypt enabled

[Alex Bennée]

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

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>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200901133050.381844-2-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 cc9af723580..1f61e363a18 100755
--- a/configure
+++ b/configure
@@ -6953,6 +6953,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 18da7c8541d..f6f5ce1ecd0 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 55c7d2318cd..9b5076452b2 100644
--- a/meson.build
+++ b/meson.build
@@ -116,6 +116,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.20.1

[PATCH v1 3/8] tests/docker: add python3-setuptools the docker images

[Alex Bennée]

We need these now for builds to work.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/dockerfiles/debian10.docker | 1 +
 tests/docker/dockerfiles/debian9.docker  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tests/docker/dockerfiles/debian10.docker b/tests/docker/dockerfiles/debian10.docker
index bcdff04ddfe..e3c11a454ee 100644
--- a/tests/docker/dockerfiles/debian10.docker
+++ b/tests/docker/dockerfiles/debian10.docker
@@ -29,6 +29,7 @@ RUN apt update && \
         pkg-config \
         psmisc \
         python3 \
+        python3-setuptools \
         python3-sphinx \
         texinfo \
         $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  -f2)
diff --git a/tests/docker/dockerfiles/debian9.docker b/tests/docker/dockerfiles/debian9.docker
index 0f0ebe530af..3edb5147ef3 100644
--- a/tests/docker/dockerfiles/debian9.docker
+++ b/tests/docker/dockerfiles/debian9.docker
@@ -28,4 +28,5 @@ RUN apt update && \
         pkg-config \
         psmisc \
         python3 \
+        python3-setuptools \
         $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  -f2)
-- 
2.20.1

[PATCH v1 4/8] usb-host: restrict workaround to new libusb versions

[Alex Bennée]

From: Gerd Hoffmann <kraxel@redhat.com>

Fixes build failures with old kernels (USBDEVFS_GET_SPEED missing),
on the assumtion that distros with old kernels also have old libusb.

Reported-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200902081445.3291-1-kraxel@redhat.com>
---
 hw/usb/host-libusb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 08604f787fd..c5d38cb09c0 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -942,7 +942,7 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd)
     usb_host_ep_update(s);
 
     libusb_speed = libusb_get_device_speed(dev);
-#ifdef CONFIG_LINUX
+#if LIBUSB_API_VERSION >= 0x01000107 && defined(CONFIG_LINUX)
     if (hostfd && libusb_speed == 0) {
         /*
          * Workaround libusb bug: libusb_get_device_speed() does not
-- 
2.20.1

[PATCH v1 5/8] qemu-iotests: move check-block back to Makefiles

[Alex Bennée]

From: Paolo Bonzini <pbonzini@redhat.com>

check-block has its own test harness, unlike every other test.  If
we capture its output, as is in general nicer to do without V=1,
there will be no sign of progress.  So for lack of a better option
just move the invocation of the test back to Makefile rules.

As a side effect, this will also fix "make check" in --disable-tools
builds, as they were trying to run qemu-iotests without having
made qemu-img before.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200902080046.10412-1-pbonzini@redhat.com>
---
 meson.build                    |  2 --
 tests/Makefile.include         | 15 ++++++++++++---
 tests/qemu-iotests/meson.build |  4 ----
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/meson.build b/meson.build
index 9b5076452b2..599306f59ad 100644
--- a/meson.build
+++ b/meson.build
@@ -1100,11 +1100,9 @@ if have_tools
              dependencies: [authz, block, crypto, io, qom, qemuutil], install: true)
   qemu_io = executable('qemu-io', files('qemu-io.c'),
              dependencies: [block, qemuutil], install: true)
-  qemu_block_tools += [qemu_img, qemu_io]
   if targetos != 'windows'
     qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
                dependencies: [block, qemuutil], install: true)
-    qemu_block_tools += [qemu_nbd]
   endif
 
   subdir('storage-daemon')
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 9ac8f5b86a6..08301f5bc9b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -468,7 +468,6 @@ check-tcg: $(RUN_TCG_TARGET_RULES)
 .PHONY: clean-tcg
 clean-tcg: $(CLEAN_TCG_TARGET_RULES)
 
-
 # Python venv for running tests
 
 .PHONY: check-venv check-acceptance
@@ -523,8 +522,18 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
 # Consolidated targets
 
 .PHONY: check-block check-unit check check-clean get-vm-images
-check-block:
-check-build: build-unit
+check:
+
+ifeq ($(CONFIG_TOOLS)$(CONFIG_POSIX),yy)
+QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) = tests/qemu-iotests/socket_scm_helper$(EXESUF)
+check: check-block
+check-block: $(SRC_PATH)/tests/check-block.sh qemu-img$(EXESUF) \
+		qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y) \
+		$(patsubst %-softmmu,qemu-system-%,$(filter %-softmmu,$(TARGET_DIRS)))
+	@$<
+endif
+
+check-build: build-unit $(QEMU_IOTESTS_HELPERS-y)
 
 check-clean:
 	rm -rf $(check-unit-y) tests/*.o tests/*/*.o $(QEMU_IOTESTS_HELPERS-y)
diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index 3de09fb8fab..60470936b45 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -4,7 +4,3 @@ if 'CONFIG_LINUX' in config_host
 else
     socket_scm_helper = []
 endif
-test('qemu-iotests', sh, args: [files('../check-block.sh')],
-     depends: [qemu_block_tools, emulators, socket_scm_helper],
-     suite: 'block', timeout: 10000)
-
-- 
2.20.1

[PATCH v1 6/8] tests/meson.build: fp tests don't need CONFIG_TCG

[Alex Bennée]

As the tests build only softfloat.c no actual TCG machinary is neede
to test them (as is evidenced by GCC check-softfloat). Might as well
fix the wording on Travis while at it.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .travis.yml       | 2 +-
 tests/meson.build | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 1d0ade0a133..65341634d02 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -138,7 +138,7 @@ jobs:
 
 
     # Just build tools and run minimal unit and softfloat checks
-    - name: "GCC check-softfloat (user)"
+    - name: "GCC check-unit and check-softfloat"
       env:
         - BASE_CONFIG="--enable-tools"
         - CONFIG="--disable-user --disable-system"
diff --git a/tests/meson.build b/tests/meson.build
index fe2c6d8e6b6..bdcc5d75293 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -7,8 +7,9 @@ test('decodetree', sh,
      workdir: meson.current_source_dir() / 'decode',
      suite: 'decodetree')
 
+subdir('fp')
+
 if 'CONFIG_TCG' in config_host
-  subdir('fp')
   if 'CONFIG_PLUGIN' in config_host
     subdir('plugin')
   endif
-- 
2.20.1

[PATCH v1 7/8] target/mips: simplify gen_compute_imm_branch logic

[Alex Bennée]

One of the Travis builds was complaining about:

  qemu/include/tcg/tcg.h:437:12: error: ‘cond’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
       return (TCGCond)(c ^ 1);
  ../target/mips/translate.c:20031:13: note: ‘cond’ was declared here
       TCGCond cond;

Rather than figure out exactly which one was causing the complaint I
just defaulted to TCG_COND_ALWAYS and allowed that state to double up
for the now defunct bcond_compute variable.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/mips/translate.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 899b90ae0ff..398edf72898 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20028,8 +20028,7 @@ static void gen_pool32axf_nanomips_insn(CPUMIPSState *env, DisasContext *ctx)
 static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
                                    int rt, int32_t imm, int32_t offset)
 {
-    TCGCond cond;
-    int bcond_compute = 0;
+    TCGCond cond = TCG_COND_ALWAYS;
     TCGv t0 = tcg_temp_new();
     TCGv t1 = tcg_temp_new();
 
@@ -20046,7 +20045,6 @@ static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
             /* Treat as NOP */
             goto out;
         } else {
-            bcond_compute = 1;
             cond = TCG_COND_EQ;
         }
         break;
@@ -20065,7 +20063,6 @@ static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
             tcg_gen_shri_tl(t0, t0, imm);
             tcg_gen_andi_tl(t0, t0, 1);
             tcg_gen_movi_tl(t1, 0);
-            bcond_compute = 1;
             if (opc == NM_BBEQZC) {
                 cond = TCG_COND_EQ;
             } else {
@@ -20080,7 +20077,6 @@ static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
         } else if (rt == 0 && imm != 0) {
             /* Unconditional branch */
         } else {
-            bcond_compute = 1;
             cond = TCG_COND_NE;
         }
         break;
@@ -20088,24 +20084,20 @@ static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
         if (rt == 0 && imm == 0) {
             /* Unconditional branch */
         } else  {
-            bcond_compute = 1;
             cond = TCG_COND_GE;
         }
         break;
     case NM_BLTIC:
-        bcond_compute = 1;
         cond = TCG_COND_LT;
         break;
     case NM_BGEIUC:
         if (rt == 0 && imm == 0) {
             /* Unconditional branch */
         } else  {
-            bcond_compute = 1;
             cond = TCG_COND_GEU;
         }
         break;
     case NM_BLTIUC:
-        bcond_compute = 1;
         cond = TCG_COND_LTU;
         break;
     default:
@@ -20118,7 +20110,7 @@ static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
     clear_branch_hflags(ctx);
     ctx->base.is_jmp = DISAS_NORETURN;
 
-    if (bcond_compute == 0) {
+    if (cond == TCG_COND_ALWAYS) {
         /* Uncoditional compact branch */
         gen_goto_tb(ctx, 0, ctx->btarget);
     } else {
-- 
2.20.1

[PATCH v1 8/8] migration: use pstrcpy to copy run state

[Alex Bennée]

The gcov build triggered:

  ../../migration/global_state.c:47:5: error: ‘strncpy’ specified
      bound 100 equals destination size [-Werror=stringop-truncation]
      strncpy((char *)global_state.runstate

As we shouldn't be using strncpy anyway lets use the suggested
pstrcpy.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 migration/global_state.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/global_state.c b/migration/global_state.c
index 25311479a4b..5fbe6d1ff07 100644
--- a/migration/global_state.c
+++ b/migration/global_state.c
@@ -44,8 +44,8 @@ void global_state_store_running(void)
 {
     const char *state = RunState_str(RUN_STATE_RUNNING);
     assert(strlen(state) < sizeof(global_state.runstate));
-    strncpy((char *)global_state.runstate,
-           state, sizeof(global_state.runstate));
+    pstrcpy((char *)global_state.runstate, sizeof(global_state.runstate),
+            state);
 }
 
 bool global_state_received(void)
-- 
2.20.1

Re: [PATCH v1 0/8] some testing and CI updates (re-greening)

[Daniel P. Berrangé]

On Thu, Sep 03, 2020 at 12:20:59PM +0100, Alex Bennée wrote:
> Hi,
> 
> My first series after a holiday is a bunch of clean-ups for testing.
> Currently they all apply on top of Thomas' pull-request-2020-09-02 tag
> which is currently in flight. The fixes to shippable won't become
> apparent until the main registry has been updates with the new images
> but I have tested them locally.
> 
> The migration and mips fixes where just quick band-aids so I would
> appreciate the appropriate maintainers having a closer look.
> 
> With these we get back to a mostly green state although there seem to
> be some acceptance tests that need updating.
> 
> Alex Bennée (5):
>   CODING_STYLE.rst: flesh out our naming conventions.
>   tests/docker: add python3-setuptools the docker images
>   tests/meson.build: fp tests don't need CONFIG_TCG
>   target/mips: simplify gen_compute_imm_branch logic
>   migration: use pstrcpy to copy run state
> 
> Daniel P. Berrangé (1):
>   crypto: fix build with gcrypt enabled

Any reason you only picked 1 of the two pathces in that series.
The second patch adds CI to validate that the first patch is
actually correct

https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg00279.html

> 
> Gerd Hoffmann (1):
>   usb-host: restrict workaround to new libusb versions
> 
> Paolo Bonzini (1):
>   qemu-iotests: move check-block back to Makefiles
> 
>  CODING_STYLE.rst                         | 31 +++++++++++++++--
>  configure                                |  2 ++
>  hw/usb/host-libusb.c                     |  2 +-
>  migration/global_state.c                 |  4 +--
>  target/mips/translate.c                  | 12 ++-----
>  .travis.yml                              |  2 +-
>  crypto/meson.build                       | 42 +++++++++++++++++-------
>  meson.build                              |  7 ++--
>  tests/Makefile.include                   | 15 +++++++--
>  tests/docker/dockerfiles/debian10.docker |  1 +
>  tests/docker/dockerfiles/debian9.docker  |  1 +
>  tests/meson.build                        |  3 +-
>  tests/qemu-iotests/meson.build           |  4 ---
>  13 files changed, 89 insertions(+), 37 deletions(-)
> 
> -- 
> 2.20.1
> 

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 v1 3/8] tests/docker: add python3-setuptools the docker images

[Thomas Huth]

On 03/09/2020 13.21, Alex Bennée wrote:
> We need these now for builds to work.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/docker/dockerfiles/debian10.docker | 1 +
>  tests/docker/dockerfiles/debian9.docker  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/tests/docker/dockerfiles/debian10.docker b/tests/docker/dockerfiles/debian10.docker
> index bcdff04ddfe..e3c11a454ee 100644
> --- a/tests/docker/dockerfiles/debian10.docker
> +++ b/tests/docker/dockerfiles/debian10.docker
> @@ -29,6 +29,7 @@ RUN apt update && \
>          pkg-config \
>          psmisc \
>          python3 \
> +        python3-setuptools \
>          python3-sphinx \
>          texinfo \
>          $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  -f2)
> diff --git a/tests/docker/dockerfiles/debian9.docker b/tests/docker/dockerfiles/debian9.docker
> index 0f0ebe530af..3edb5147ef3 100644
> --- a/tests/docker/dockerfiles/debian9.docker
> +++ b/tests/docker/dockerfiles/debian9.docker
> @@ -28,4 +28,5 @@ RUN apt update && \
>          pkg-config \
>          psmisc \
>          python3 \
> +        python3-setuptools \
>          $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  -f2)
> 

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

Re: [PATCH v1 6/8] tests/meson.build: fp tests don't need CONFIG_TCG

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1506 bytes --]

Il gio 3 set 2020, 13:21 Alex Bennée <alex.bennee@linaro.org> ha scritto:

> As the tests build only softfloat.c no actual TCG machinary is neede
> to test them (as is evidenced by GCC check-softfloat). Might as well
> fix the wording on Travis while at it.
>

The reason is that softfloat is not built at all into QEMU if !CONFIG_TCG.
We similarly skip block layer tests if !CONFIG_SOFTMMU && !CONFIG_TOOLS.

Paolo


> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  .travis.yml       | 2 +-
>  tests/meson.build | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 1d0ade0a133..65341634d02 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -138,7 +138,7 @@ jobs:
>
>
>      # Just build tools and run minimal unit and softfloat checks
> -    - name: "GCC check-softfloat (user)"
> +    - name: "GCC check-unit and check-softfloat"
>        env:
>          - BASE_CONFIG="--enable-tools"
>          - CONFIG="--disable-user --disable-system"
> diff --git a/tests/meson.build b/tests/meson.build
> index fe2c6d8e6b6..bdcc5d75293 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -7,8 +7,9 @@ test('decodetree', sh,
>       workdir: meson.current_source_dir() / 'decode',
>       suite: 'decodetree')
>
> +subdir('fp')
> +
>  if 'CONFIG_TCG' in config_host
> -  subdir('fp')
>    if 'CONFIG_PLUGIN' in config_host
>      subdir('plugin')
>    endif
> --
> 2.20.1
>
>

[-- Attachment #2: Type: text/html, Size: 2390 bytes --]

Re: [PATCH v1 8/8] migration: use pstrcpy to copy run state

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]

Il gio 3 set 2020, 13:21 Alex Bennée <alex.bennee@linaro.org> ha scritto:

> The gcov build triggered:
>
>   ../../migration/global_state.c:47:5: error: ‘strncpy’ specified
>       bound 100 equals destination size [-Werror=stringop-truncation]
>       strncpy((char *)global_state.runstate
>
> As we shouldn't be using strncpy anyway lets use the suggested
> pstrcpy.
>

This is wrong, we want the all-zeros behavior of strncpy that pstrcpy lacks.

Paolo


>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  migration/global_state.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/migration/global_state.c b/migration/global_state.c
> index 25311479a4b..5fbe6d1ff07 100644
> --- a/migration/global_state.c
> +++ b/migration/global_state.c
> @@ -44,8 +44,8 @@ void global_state_store_running(void)
>  {
>      const char *state = RunState_str(RUN_STATE_RUNNING);
>      assert(strlen(state) < sizeof(global_state.runstate));
> -    strncpy((char *)global_state.runstate,
> -           state, sizeof(global_state.runstate));
> +    pstrcpy((char *)global_state.runstate, sizeof(global_state.runstate),
> +            state);
>  }
>
>  bool global_state_received(void)
> --
> 2.20.1
>
>

[-- Attachment #2: Type: text/html, Size: 2026 bytes --]

Re: [PATCH v1 1/8] CODING_STYLE.rst: flesh out our naming conventions.

[Paolo Bonzini]

On 03/09/20 13:21, Alex Bennée wrote:
> Mention a few of the more common naming conventions we follow in the
> code base including common variable names and function prefix and
> suffix examples.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>   - punctuation fixes suggested by Cornelia
>   - re-worded section on qemu_ prefix
>   - expanded on _locked suffix
> v3
>   - re-order phrasing around qemu_ prefix
>   - drop "while actual..." shortname examples
> 
> squash! CODING_STYLE.rst: flesh out our naming conventions.
> ---
>  CODING_STYLE.rst | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/CODING_STYLE.rst b/CODING_STYLE.rst
> index 427699e0e42..fd8ce9ceaba 100644
> --- a/CODING_STYLE.rst
> +++ b/CODING_STYLE.rst
> @@ -109,8 +109,35 @@ names are lower_case_with_underscores_ending_with_a_t, like the POSIX
>  uint64_t and family.  Note that this last convention contradicts POSIX
>  and is therefore likely to be changed.
>  
> -When wrapping standard library functions, use the prefix ``qemu_`` to alert
> -readers that they are seeing a wrapped version; otherwise avoid this prefix.
> +Variable Naming Conventions
> +---------------------------
> +
> +A number of short naming conventions exist for variables that use
> +common QEMU types. For example, the architecture independent CPUState
> +is often held as a ``cs`` pointer variable, whereas the concrete
> +CPUArchState is usually held in a pointer called ``env``.
> +
> +Likewise, in device emulation code the common DeviceState is usually
> +called ``dev``.
> +
> +Function Naming Conventions
> +---------------------------
> +
> +The ``qemu_`` prefix is used for utility functions that are widely
> +called from across the code-base. This includes wrapped versions of
> +standard library functions (e.g. ``qemu_strtol``) where the prefix is
> +added to the library function name to alert readers that they are
> +seeing a wrapped version.

I think we're slowly moving away from "qemu_" except for wrappers, so I
would move this paragraph last, wording it like this:

---
Wrapped version of standard library or GLib functions use a ``qemu_``
prefix to alert readers that they are seeing a wrapped version, for
example ``qemu_strtol`` or ``qemu_mutex_lock``.  Other utility functions
that are widely called from across the codebase should not have any
prefix, for example ``pstrcpy`` or bit manipulation functions such as
``find_first_bit``.

The ``qemu_`` prefix is also used for functions that modify global
emulator state, for example ``qemu_add_vm_change_state_handler``.
However, if there is an obvious subsystem-specific prefix it should be
used instead.
---

Paolo

> +Public functions from a file or subsystem (declared in headers) tend
> +to have a consistent prefix to show where they came from. For example,
> +``tlb_`` for functions from ``cputlb.c`` or ``cpu_`` for functions
> +from cpus.c.
> +
> +If there are two versions of a function to be called with or without a
> +lock held, the function that expects the lock to be already held
> +usually uses the suffix ``_locked``.
> +
>  
>  Block structure
>  ===============
> 

Re: [PATCH v1 0/8] some testing and CI updates (re-greening)

[Thomas Huth]

On 03/09/2020 13.34, Daniel P. Berrangé wrote:
> On Thu, Sep 03, 2020 at 12:20:59PM +0100, Alex Bennée wrote:
>> Hi,
>>
>> My first series after a holiday is a bunch of clean-ups for testing.
>> Currently they all apply on top of Thomas' pull-request-2020-09-02 tag
>> which is currently in flight. The fixes to shippable won't become
>> apparent until the main registry has been updates with the new images
>> but I have tested them locally.
>>
>> The migration and mips fixes where just quick band-aids so I would
>> appreciate the appropriate maintainers having a closer look.
>>
>> With these we get back to a mostly green state although there seem to
>> be some acceptance tests that need updating.
>>
>> Alex Bennée (5):
>>   CODING_STYLE.rst: flesh out our naming conventions.
>>   tests/docker: add python3-setuptools the docker images
>>   tests/meson.build: fp tests don't need CONFIG_TCG
>>   target/mips: simplify gen_compute_imm_branch logic
>>   migration: use pstrcpy to copy run state
>>
>> Daniel P. Berrangé (1):
>>   crypto: fix build with gcrypt enabled
> 
> Any reason you only picked 1 of the two pathces in that series.
> The second patch adds CI to validate that the first patch is
> actually correct
 I guess Alex dropped it because there is a (simple) conflict with one
of my patches ("gitlab/travis: Rework the disabled features tests") ...
should be easy to fix, though.

I'm also preparing yet another patch series for improving the gitlab-CI
(with the cross-compiler patches), so I can also put your crypto gitlab
patch in there if you like.

 Thomas

Re: [PATCH v1 0/8] some testing and CI updates (re-greening)

[Alex Bennée]

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

> On Thu, Sep 03, 2020 at 12:20:59PM +0100, Alex Bennée wrote:
>> Hi,
>> 
>> My first series after a holiday is a bunch of clean-ups for testing.
>> Currently they all apply on top of Thomas' pull-request-2020-09-02 tag
>> which is currently in flight. The fixes to shippable won't become
>> apparent until the main registry has been updates with the new images
>> but I have tested them locally.
>> 
>> The migration and mips fixes where just quick band-aids so I would
>> appreciate the appropriate maintainers having a closer look.
>> 
>> With these we get back to a mostly green state although there seem to
>> be some acceptance tests that need updating.
>> 
>> Alex Bennée (5):
>>   CODING_STYLE.rst: flesh out our naming conventions.
>>   tests/docker: add python3-setuptools the docker images
>>   tests/meson.build: fp tests don't need CONFIG_TCG
>>   target/mips: simplify gen_compute_imm_branch logic
>>   migration: use pstrcpy to copy run state
>> 
>> Daniel P. Berrangé (1):
>>   crypto: fix build with gcrypt enabled
>
> Any reason you only picked 1 of the two pathces in that series.
> The second patch adds CI to validate that the first patch is
> actually correct
>
> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg00279.html

Oops, i got dropped when I re-based on Thomas' tree and I erroneously
thought it was because it conflicted with itself rather than another
change. I'll include it in v2.

>
>> 
>> Gerd Hoffmann (1):
>>   usb-host: restrict workaround to new libusb versions
>> 
>> Paolo Bonzini (1):
>>   qemu-iotests: move check-block back to Makefiles
>> 
>>  CODING_STYLE.rst                         | 31 +++++++++++++++--
>>  configure                                |  2 ++
>>  hw/usb/host-libusb.c                     |  2 +-
>>  migration/global_state.c                 |  4 +--
>>  target/mips/translate.c                  | 12 ++-----
>>  .travis.yml                              |  2 +-
>>  crypto/meson.build                       | 42 +++++++++++++++++-------
>>  meson.build                              |  7 ++--
>>  tests/Makefile.include                   | 15 +++++++--
>>  tests/docker/dockerfiles/debian10.docker |  1 +
>>  tests/docker/dockerfiles/debian9.docker  |  1 +
>>  tests/meson.build                        |  3 +-
>>  tests/qemu-iotests/meson.build           |  4 ---
>>  13 files changed, 89 insertions(+), 37 deletions(-)
>> 
>> -- 
>> 2.20.1
>> 
>
> Regards,
> Daniel


-- 
Alex Bennée

Re: [PATCH v1 7/8] target/mips: simplify gen_compute_imm_branch logic

[Richard Henderson]

On 9/3/20 4:21 AM, Alex Bennée wrote:
> One of the Travis builds was complaining about:
> 
>   qemu/include/tcg/tcg.h:437:12: error: ‘cond’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>        return (TCGCond)(c ^ 1);
>   ../target/mips/translate.c:20031:13: note: ‘cond’ was declared here
>        TCGCond cond;
> 
> Rather than figure out exactly which one was causing the complaint I
> just defaulted to TCG_COND_ALWAYS and allowed that state to double up
> for the now defunct bcond_compute variable.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  target/mips/translate.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)

This looks like a good cleanup.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v1 2/8] crypto: fix build with gcrypt enabled

[Richard Henderson]

On 9/3/20 4:21 AM, Alex Bennée wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> 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>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20200901133050.381844-2-berrange@redhat.com>
> ---
>  configure          |  2 ++
>  crypto/meson.build | 42 +++++++++++++++++++++++++++++++-----------
>  meson.build        |  5 +++++
>  3 files changed, 38 insertions(+), 11 deletions(-)

Tested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v1 3/8] tests/docker: add python3-setuptools the docker images

[Philippe Mathieu-Daudé]

On 9/3/20 1:21 PM, Alex Bennée wrote:
> We need these now for builds to work.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  tests/docker/dockerfiles/debian10.docker | 1 +
>  tests/docker/dockerfiles/debian9.docker  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/tests/docker/dockerfiles/debian10.docker b/tests/docker/dockerfiles/debian10.docker
> index bcdff04ddfe..e3c11a454ee 100644
> --- a/tests/docker/dockerfiles/debian10.docker
> +++ b/tests/docker/dockerfiles/debian10.docker
> @@ -29,6 +29,7 @@ RUN apt update && \
>          pkg-config \
>          psmisc \
>          python3 \
> +        python3-setuptools \
>          python3-sphinx \
>          texinfo \
>          $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  -f2)
> diff --git a/tests/docker/dockerfiles/debian9.docker b/tests/docker/dockerfiles/debian9.docker
> index 0f0ebe530af..3edb5147ef3 100644
> --- a/tests/docker/dockerfiles/debian9.docker
> +++ b/tests/docker/dockerfiles/debian9.docker
> @@ -28,4 +28,5 @@ RUN apt update && \
>          pkg-config \
>          psmisc \
>          python3 \
> +        python3-setuptools \
>          $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  -f2)
> 

Re: [PATCH v1 8/8] migration: use pstrcpy to copy run state

[Philippe Mathieu-Daudé]

On 9/3/20 2:13 PM, Paolo Bonzini wrote:
> Il gio 3 set 2020, 13:21 Alex Bennée <alex.bennee@linaro.org
> <mailto:alex.bennee@linaro.org>> ha scritto:
> 
>     The gcov build triggered:
> 
>       ../../migration/global_state.c:47:5: error: ‘strncpy’ specified
>           bound 100 equals destination size [-Werror=stringop-truncation]
>           strncpy((char *)global_state.runstate
> 
>     As we shouldn't be using strncpy anyway lets use the suggested
>     pstrcpy.
> 
> 
> This is wrong, we want the all-zeros behavior of strncpy that pstrcpy lacks.

FWIW links to previous discussions:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg584216.html

> 
> Paolo

Re: [PATCH v1 8/8] migration: use pstrcpy to copy run state

[Alex Bennée]

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

> On 9/3/20 2:13 PM, Paolo Bonzini wrote:
>> Il gio 3 set 2020, 13:21 Alex Bennée <alex.bennee@linaro.org
>> <mailto:alex.bennee@linaro.org>> ha scritto:
>> 
>>     The gcov build triggered:
>> 
>>       ../../migration/global_state.c:47:5: error: ‘strncpy’ specified
>>           bound 100 equals destination size [-Werror=stringop-truncation]
>>           strncpy((char *)global_state.runstate
>> 
>>     As we shouldn't be using strncpy anyway lets use the suggested
>>     pstrcpy.
>> 
>> 
>> This is wrong, we want the all-zeros behavior of strncpy that pstrcpy lacks.
>
> FWIW links to previous discussions:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg584216.html

Hmm I wonder why gprof interfered with the assert. Either way I'll drop
the patch.

>
>> 
>> Paolo


-- 
Alex Bennée

Re: [PATCH v1 6/8] tests/meson.build: fp tests don't need CONFIG_TCG

[Alex Bennée]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il gio 3 set 2020, 13:21 Alex Bennée <alex.bennee@linaro.org> ha scritto:
>
>> As the tests build only softfloat.c no actual TCG machinary is neede
>> to test them (as is evidenced by GCC check-softfloat). Might as well
>> fix the wording on Travis while at it.
>>
>
> The reason is that softfloat is not built at all into QEMU if !CONFIG_TCG.
> We similarly skip block layer tests if !CONFIG_SOFTMMU &&
> !CONFIG_TOOLS.

It's not built anyway if you don't call the test. Are you saying a
--disable-system and --disable-user build is invalid for running unit
tests? That is what check-softfloat is doing as it doesn't involve
softfloat built into any qemu binary.

>
> Paolo
>
>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  .travis.yml       | 2 +-
>>  tests/meson.build | 3 ++-
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/.travis.yml b/.travis.yml
>> index 1d0ade0a133..65341634d02 100644
>> --- a/.travis.yml
>> +++ b/.travis.yml
>> @@ -138,7 +138,7 @@ jobs:
>>
>>
>>      # Just build tools and run minimal unit and softfloat checks
>> -    - name: "GCC check-softfloat (user)"
>> +    - name: "GCC check-unit and check-softfloat"
>>        env:
>>          - BASE_CONFIG="--enable-tools"
>>          - CONFIG="--disable-user --disable-system"
>> diff --git a/tests/meson.build b/tests/meson.build
>> index fe2c6d8e6b6..bdcc5d75293 100644
>> --- a/tests/meson.build
>> +++ b/tests/meson.build
>> @@ -7,8 +7,9 @@ test('decodetree', sh,
>>       workdir: meson.current_source_dir() / 'decode',
>>       suite: 'decodetree')
>>
>> +subdir('fp')
>> +
>>  if 'CONFIG_TCG' in config_host
>> -  subdir('fp')
>>    if 'CONFIG_PLUGIN' in config_host
>>      subdir('plugin')
>>    endif
>> --
>> 2.20.1
>>
>>


-- 
Alex Bennée

Re: [PATCH v1 6/8] tests/meson.build: fp tests don't need CONFIG_TCG

[Philippe Mathieu-Daudé]

On 9/7/20 11:11 AM, Alex Bennée wrote:
> 
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> Il gio 3 set 2020, 13:21 Alex Bennée <alex.bennee@linaro.org> ha scritto:
>>
>>> As the tests build only softfloat.c no actual TCG machinary is neede
>>> to test them (as is evidenced by GCC check-softfloat). Might as well
>>> fix the wording on Travis while at it.
>>>
>>
>> The reason is that softfloat is not built at all into QEMU if !CONFIG_TCG.
>> We similarly skip block layer tests if !CONFIG_SOFTMMU &&
>> !CONFIG_TOOLS.
> 
> It's not built anyway if you don't call the test. Are you saying a
> --disable-system and --disable-user build is invalid for running unit
> tests? That is what check-softfloat is doing as it doesn't involve
> softfloat built into any qemu binary.

FYI my tools build dir is configured as:

'--enable-trace-backends=log' '--disable-docs' '--enable-debug'
'--disable-system' '--disable-user' '--enable-tools'

and it still works after the Meson conversion.

> 
>>
>> Paolo
>>
>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>  .travis.yml       | 2 +-
>>>  tests/meson.build | 3 ++-
>>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/.travis.yml b/.travis.yml
>>> index 1d0ade0a133..65341634d02 100644
>>> --- a/.travis.yml
>>> +++ b/.travis.yml
>>> @@ -138,7 +138,7 @@ jobs:
>>>
>>>
>>>      # Just build tools and run minimal unit and softfloat checks
>>> -    - name: "GCC check-softfloat (user)"
>>> +    - name: "GCC check-unit and check-softfloat"
>>>        env:
>>>          - BASE_CONFIG="--enable-tools"
>>>          - CONFIG="--disable-user --disable-system"
>>> diff --git a/tests/meson.build b/tests/meson.build
>>> index fe2c6d8e6b6..bdcc5d75293 100644
>>> --- a/tests/meson.build
>>> +++ b/tests/meson.build
>>> @@ -7,8 +7,9 @@ test('decodetree', sh,
>>>       workdir: meson.current_source_dir() / 'decode',
>>>       suite: 'decodetree')
>>>
>>> +subdir('fp')
>>> +
>>>  if 'CONFIG_TCG' in config_host
>>> -  subdir('fp')
>>>    if 'CONFIG_PLUGIN' in config_host
>>>      subdir('plugin')
>>>    endif
>>> --
>>> 2.20.1
>>>
>>>
> 
> 

Re: [PATCH v1 6/8] tests/meson.build: fp tests don't need CONFIG_TCG

[Alex Bennée]

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

> On 9/7/20 11:11 AM, Alex Bennée wrote:
>> 
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> Il gio 3 set 2020, 13:21 Alex Bennée <alex.bennee@linaro.org> ha scritto:
>>>
>>>> As the tests build only softfloat.c no actual TCG machinary is neede
>>>> to test them (as is evidenced by GCC check-softfloat). Might as well
>>>> fix the wording on Travis while at it.
>>>>
>>>
>>> The reason is that softfloat is not built at all into QEMU if !CONFIG_TCG.
>>> We similarly skip block layer tests if !CONFIG_SOFTMMU &&
>>> !CONFIG_TOOLS.
>> 
>> It's not built anyway if you don't call the test. Are you saying a
>> --disable-system and --disable-user build is invalid for running unit
>> tests? That is what check-softfloat is doing as it doesn't involve
>> softfloat built into any qemu binary.
>
> FYI my tools build dir is configured as:
>
> '--enable-trace-backends=log' '--disable-docs' '--enable-debug'
> '--disable-system' '--disable-user' '--enable-tools'
>
> and it still works after the Meson conversion.

But check-softfloat doesn't work with that right?

-- 
Alex Bennée

Re: [PATCH v1 6/8] tests/meson.build: fp tests don't need CONFIG_TCG

[Philippe Mathieu-Daudé]

On 9/7/20 11:55 AM, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> On 9/7/20 11:11 AM, Alex Bennée wrote:
>>>
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>
>>>> Il gio 3 set 2020, 13:21 Alex Bennée <alex.bennee@linaro.org> ha scritto:
>>>>
>>>>> As the tests build only softfloat.c no actual TCG machinary is neede
>>>>> to test them (as is evidenced by GCC check-softfloat). Might as well
>>>>> fix the wording on Travis while at it.
>>>>>
>>>>
>>>> The reason is that softfloat is not built at all into QEMU if !CONFIG_TCG.
>>>> We similarly skip block layer tests if !CONFIG_SOFTMMU &&
>>>> !CONFIG_TOOLS.
>>>
>>> It's not built anyway if you don't call the test. Are you saying a
>>> --disable-system and --disable-user build is invalid for running unit
>>> tests? That is what check-softfloat is doing as it doesn't involve
>>> softfloat built into any qemu binary.
>>
>> FYI my tools build dir is configured as:
>>
>> '--enable-trace-backends=log' '--disable-docs' '--enable-debug'
>> '--disable-system' '--disable-user' '--enable-tools'
>>
>> and it still works after the Meson conversion.
> 
> But check-softfloat doesn't work with that right?

Sorry I can't say, I don't test it :/