diff mbox series

[v1,31/39] tcg/plugins: enable by default for TCG builds

Message ID 20210706145817.24109-32-alex.bennee@linaro.org
State New
Headers show
Series final pre-PR for 6.1 (testing and plugins) | expand

Commit Message

Alex Bennée July 6, 2021, 2:58 p.m. UTC 
Aside from a minor bloat to file size the ability to have TCG plugins
has no real impact on performance unless a plugin is actively loaded.
Even then the libempty.so plugin shows only a minor degradation in
performance caused by the extra book keeping the TCG has to do to keep
track of instructions. As it's a useful feature lets just enable it by
default and reduce our testing matrix a little.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - properly handle plugins being ""
  - make the test for linker support handle default case
  - move linker tests to before the glib-modules test
---
 docs/devel/tcg-plugins.rst |   3 +-
 configure                  | 125 ++++++++++++++++++++-----------------
 .gitlab-ci.d/buildtest.yml |  23 -------
 3 files changed, 71 insertions(+), 80 deletions(-)

Comments

Thomas Huth July 7, 2021, 4:32 a.m. UTC | #1 
On 06/07/2021 16.58, Alex Bennée wrote:
> Aside from a minor bloat to file size the ability to have TCG plugins
> has no real impact on performance unless a plugin is actively loaded.
> Even then the libempty.so plugin shows only a minor degradation in
> performance caused by the extra book keeping the TCG has to do to keep
> track of instructions. As it's a useful feature lets just enable it by
> default and reduce our testing matrix a little.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>    - properly handle plugins being ""
>    - make the test for linker support handle default case
>    - move linker tests to before the glib-modules test
> ---
>   docs/devel/tcg-plugins.rst |   3 +-
>   configure                  | 125 ++++++++++++++++++++-----------------
>   .gitlab-ci.d/buildtest.yml |  23 -------
>   3 files changed, 71 insertions(+), 80 deletions(-)
> 
> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
> index 18c6581d85..0cd77c77d2 100644
> --- a/docs/devel/tcg-plugins.rst
> +++ b/docs/devel/tcg-plugins.rst
> @@ -71,7 +71,8 @@ API
>   Usage
>   =====
>   
> -The QEMU binary needs to be compiled for plugin support::
> +Any QEMU binary with TCG support has plugins enabled by default.
> +Earlier releases needed to be explicitly enabled with::
>   
>     configure --enable-plugins
>   
> diff --git a/configure b/configure
> index 9d72b31a9f..0ce6c1ff65 100755
> --- a/configure
> +++ b/configure
> @@ -429,7 +429,7 @@ libxml2="auto"
>   debug_mutex="no"
>   libpmem="auto"
>   default_devices="true"
> -plugins="no"
> +plugins="$default_feature"
>   fuzzing="no"
>   rng_none="no"
>   secret_keyring="$default_feature"
> @@ -3085,6 +3085,73 @@ for drv in $audio_drv_list; do
>       esac
>   done
>   
> +##########################################
> +# plugin linker support probe
> +
> +if test "$plugins" != "no"; then
> +
> +    #########################################
> +    # See if --dynamic-list is supported by the linker
> +
> +    ld_dynamic_list="no"
> +    if test "$static" = "no" ; then
> +        cat > $TMPTXT <<EOF
> +{
> +  foo;
> +};
> +EOF
> +
> +        cat > $TMPC <<EOF
> +#include <stdio.h>
> +void foo(void);
> +
> +void foo(void)
> +{
> +  printf("foo\n");
> +}
> +
> +int main(void)
> +{
> +  foo();
> +  return 0;
> +}
> +EOF
> +
> +        if compile_prog "" "-Wl,--dynamic-list=$TMPTXT" ; then
> +            ld_dynamic_list="yes"
> +        fi
> +    fi
> +
> +    #########################################
> +    # See if -exported_symbols_list is supported by the linker
> +
> +    ld_exported_symbols_list="no"
> +    if test "$static" = "no" ; then
> +        cat > $TMPTXT <<EOF
> +  _foo
> +EOF
> +
> +        if compile_prog "" "-Wl,-exported_symbols_list,$TMPTXT" ; then
> +            ld_exported_symbols_list="yes"
> +        fi
> +    fi
> +
> +    if test "$ld_dynamic_list" = "no" &&
> +       test "$ld_exported_symbols_list" = "no" ; then
> +        if test "$plugins" = "yes"; then
> +            error_exit \
> +                "Plugin ($plugins) support requires dynamic linking and specifying a set of symbols " \

Printing "($plugins)" here does not make much sense to me (it will always 
result in "(yes)", won't it?) ... but apart from that, the patch looks fine 
to me.

So with that "($plugins)" removed:

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

Patch

diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 18c6581d85..0cd77c77d2 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -71,7 +71,8 @@  API
 Usage
 =====
 
-The QEMU binary needs to be compiled for plugin support::
+Any QEMU binary with TCG support has plugins enabled by default.
+Earlier releases needed to be explicitly enabled with::
 
   configure --enable-plugins
 
diff --git a/configure b/configure
index 9d72b31a9f..0ce6c1ff65 100755
--- a/configure
+++ b/configure
@@ -429,7 +429,7 @@  libxml2="auto"
 debug_mutex="no"
 libpmem="auto"
 default_devices="true"
-plugins="no"
+plugins="$default_feature"
 fuzzing="no"
 rng_none="no"
 secret_keyring="$default_feature"
@@ -3085,6 +3085,73 @@  for drv in $audio_drv_list; do
     esac
 done
 
+##########################################
+# plugin linker support probe
+
+if test "$plugins" != "no"; then
+
+    #########################################
+    # See if --dynamic-list is supported by the linker
+
+    ld_dynamic_list="no"
+    if test "$static" = "no" ; then
+        cat > $TMPTXT <<EOF
+{
+  foo;
+};
+EOF
+
+        cat > $TMPC <<EOF
+#include <stdio.h>
+void foo(void);
+
+void foo(void)
+{
+  printf("foo\n");
+}
+
+int main(void)
+{
+  foo();
+  return 0;
+}
+EOF
+
+        if compile_prog "" "-Wl,--dynamic-list=$TMPTXT" ; then
+            ld_dynamic_list="yes"
+        fi
+    fi
+
+    #########################################
+    # See if -exported_symbols_list is supported by the linker
+
+    ld_exported_symbols_list="no"
+    if test "$static" = "no" ; then
+        cat > $TMPTXT <<EOF
+  _foo
+EOF
+
+        if compile_prog "" "-Wl,-exported_symbols_list,$TMPTXT" ; then
+            ld_exported_symbols_list="yes"
+        fi
+    fi
+
+    if test "$ld_dynamic_list" = "no" &&
+       test "$ld_exported_symbols_list" = "no" ; then
+        if test "$plugins" = "yes"; then
+            error_exit \
+                "Plugin ($plugins) support requires dynamic linking and specifying a set of symbols " \
+                "that are exported to plugins. Unfortunately your linker doesn't " \
+                "support the flag (--dynamic-list or -exported_symbols_list) used " \
+                "for this purpose. You can't build with --static."
+        else
+            plugins="no"
+        fi
+    else
+        plugins="yes"
+    fi
+fi
+
 ##########################################
 # glib support probe
 
@@ -3093,7 +3160,7 @@  glib_modules=gthread-2.0
 if test "$modules" = yes; then
     glib_modules="$glib_modules gmodule-export-2.0"
 fi
-if test "$plugins" = yes; then
+if test "$plugins" = "yes"; then
     glib_modules="$glib_modules gmodule-2.0"
 fi
 
@@ -3931,60 +3998,6 @@  if compile_prog "" "" ; then
   atomic64=yes
 fi
 
-#########################################
-# See if --dynamic-list is supported by the linker
-ld_dynamic_list="no"
-if test "$static" = "no" ; then
-    cat > $TMPTXT <<EOF
-{
-  foo;
-};
-EOF
-
-    cat > $TMPC <<EOF
-#include <stdio.h>
-void foo(void);
-
-void foo(void)
-{
-  printf("foo\n");
-}
-
-int main(void)
-{
-  foo();
-  return 0;
-}
-EOF
-
-    if compile_prog "" "-Wl,--dynamic-list=$TMPTXT" ; then
-        ld_dynamic_list="yes"
-    fi
-fi
-
-#########################################
-# See if -exported_symbols_list is supported by the linker
-
-ld_exported_symbols_list="no"
-if test "$static" = "no" ; then
-    cat > $TMPTXT <<EOF
-  _foo
-EOF
-
-    if compile_prog "" "-Wl,-exported_symbols_list,$TMPTXT" ; then
-        ld_exported_symbols_list="yes"
-    fi
-fi
-
-if  test "$plugins" = "yes" &&
-    test "$ld_dynamic_list" = "no" &&
-    test "$ld_exported_symbols_list" = "no" ; then
-  error_exit \
-      "Plugin support requires dynamic linking and specifying a set of symbols " \
-      "that are exported to plugins. Unfortunately your linker doesn't " \
-      "support the flag (--dynamic-list or -exported_symbols_list) used " \
-      "for this purpose. You can't build with --static."
-fi
 
 ########################################
 # check if getauxval is available.
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index d9b834c848..89df51517c 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -354,29 +354,6 @@  build-some-softmmu:
     TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
     MAKE_CHECK_ARGS: check-tcg
 
-# Run check-tcg against linux-user (with plugins)
-# we skip sparc64-linux-user until it has been fixed somewhat
-# we skip cris-linux-user as it doesn't use the common run loop
-build-user-plugins:
-  extends: .native_build_job_template
-  needs:
-    job: amd64-debian-user-cross-container
-  variables:
-    IMAGE: debian-all-test-cross
-    CONFIGURE_ARGS: --disable-tools --disable-system --enable-plugins --enable-debug-tcg --target-list-exclude=sparc64-linux-user,cris-linux-user
-    MAKE_CHECK_ARGS: check-tcg
-  timeout: 1h 30m
-
-build-some-softmmu-plugins:
-  extends: .native_build_job_template
-  needs:
-    job: amd64-debian-user-cross-container
-  variables:
-    IMAGE: debian-all-test-cross
-    CONFIGURE_ARGS: --disable-tools --disable-user --enable-plugins --enable-debug-tcg
-    TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
-    MAKE_CHECK_ARGS: check-tcg
-
 clang-system:
   extends: .native_build_job_template
   needs: