[PATCH v3 0/3] build contrib/plugins using meson

[PATCH v3 0/3] build contrib/plugins using meson

[Pierrick Bouvier]

Contrib plugins have been built out of tree so far, thanks to a Makefile.
However, it is quite inconvenient for maintenance, as we may break them,
especially for specific architectures.

First patches are fixing warnings for existing plugins, then we add meson
support, and finally, we remove Makefile for contrib/plugins.

Based on the proposal of Anton Kochkov on associated gitlab issue.
Solves: https://gitlab.com/qemu-project/qemu/-/issues/1710

Plugins are now deactivated by default on 32-bits hosts (since cf2a78), so we
can enable with meson without worrying of warnings when building plugins for 32
bits.

Removed patches for individual plugins.

v2:
- removed warnings fix for 32 bits as they were incorrect. They are not needed
  anymore as plugins are deprecated for 32 bits hosts.

v3:
- do not build lockstep on windows (it uses sys/socket.h)
- build new plugins recently added
- fix warning in cflow plugin
- tested it builds correctly on windows/macos/linux

Pierrick Bouvier (3):
  contrib/plugins/cflow: fix warning
  meson: build contrib/plugins with meson
  contrib/plugins: remove Makefile for contrib/plugins

 configure                   | 18 --------
 Makefile                    | 10 -----
 meson.build                 |  4 ++
 contrib/plugins/cflow.c     |  6 +--
 contrib/plugins/Makefile    | 87 -------------------------------------
 contrib/plugins/meson.build | 28 ++++++++++++
 6 files changed, 35 insertions(+), 118 deletions(-)
 delete mode 100644 contrib/plugins/Makefile
 create mode 100644 contrib/plugins/meson.build

-- 
2.39.5

[PATCH v3 1/3] contrib/plugins/cflow: fix warning

[Pierrick Bouvier]

contrib/plugins/cflow.c: In function ‘plugin_exit’:
contrib/plugins/cflow.c:167:19: error: declaration of ‘n’ shadows a previous local [-Werror=shadow=local]
  167 |         NodeData *n = l->data;
      |                   ^
contrib/plugins/cflow.c:139:9: note: shadowed declaration is here
  139 |     int n = 0;
      |         ^

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 contrib/plugins/cflow.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/plugins/cflow.c b/contrib/plugins/cflow.c
index 6faa55d10d1..b39974d1cf3 100644
--- a/contrib/plugins/cflow.c
+++ b/contrib/plugins/cflow.c
@@ -136,7 +136,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
     g_autoptr(GString) result = g_string_new("collected ");
     GList *data;
     GCompareFunc sort = &hottest;
-    int n = 0;
+    int i = 0;
 
     g_mutex_lock(&node_lock);
     g_string_append_printf(result, "%d control flow nodes in the hash table\n",
@@ -162,8 +162,8 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
     data = g_list_sort(data, sort);
 
     for (GList *l = data;
-         l != NULL && n < topn;
-         l = l->next, n++) {
+         l != NULL && i < topn;
+         l = l->next, i++) {
         NodeData *n = l->data;
         const char *type = n->mid_count ? "sync fault" : "branch";
         g_string_append_printf(result, "  addr: 0x%"PRIx64 " %s: %s (%s)\n",
-- 
2.39.5

[PATCH v3 2/3] meson: build contrib/plugins with meson

[Pierrick Bouvier]

Tried to unify this meson.build with tests/tcg/plugins/meson.build but
the resulting modules are not output in the right directory.

Originally proposed by Anton Kochkov, thank you!

Solves: https://gitlab.com/qemu-project/qemu/-/issues/1710
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 meson.build                 |  4 ++++
 contrib/plugins/meson.build | 28 ++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 contrib/plugins/meson.build

diff --git a/meson.build b/meson.build
index bdd67a2d6d1..3ea03c451ba 100644
--- a/meson.build
+++ b/meson.build
@@ -3678,6 +3678,10 @@ subdir('accel')
 subdir('plugins')
 subdir('ebpf')
 
+if 'CONFIG_TCG' in config_all_accel
+  subdir('contrib/plugins')
+endif
+
 common_user_inc = []
 
 subdir('common-user')
diff --git a/contrib/plugins/meson.build b/contrib/plugins/meson.build
new file mode 100644
index 00000000000..63a32c2b4f0
--- /dev/null
+++ b/contrib/plugins/meson.build
@@ -0,0 +1,28 @@
+contrib_plugins = ['bbv', 'cache', 'cflow', 'drcov', 'execlog', 'hotblocks',
+                   'hotpages', 'howvec', 'hwprofile', 'ips', 'stoptrigger']
+if host_os != 'windows'
+  # lockstep uses socket.h
+  contrib_plugins += 'lockstep'
+endif
+
+t = []
+if get_option('plugins')
+  foreach i : contrib_plugins
+    if host_os == 'windows'
+      t += shared_module(i, files(i + '.c') + 'win32_linker.c',
+                        include_directories: '../../include/qemu',
+                        link_depends: [win32_qemu_plugin_api_lib],
+                        link_args: ['-Lplugins', '-lqemu_plugin_api'],
+                        dependencies: glib)
+    else
+      t += shared_module(i, files(i + '.c'),
+                        include_directories: '../../include/qemu',
+                        dependencies: glib)
+    endif
+  endforeach
+endif
+if t.length() > 0
+  alias_target('contrib-plugins', t)
+else
+  run_target('contrib-plugins', command: find_program('true'))
+endif
-- 
2.39.5

[PATCH v3 3/3] contrib/plugins: remove Makefile for contrib/plugins

[Pierrick Bouvier]

Now replaced by meson build.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 configure                | 18 ---------
 Makefile                 | 10 -----
 contrib/plugins/Makefile | 87 ----------------------------------------
 3 files changed, 115 deletions(-)
 delete mode 100644 contrib/plugins/Makefile

diff --git a/configure b/configure
index 72d1a942250..d23587c77dc 100755
--- a/configure
+++ b/configure
@@ -1073,7 +1073,6 @@ if test "$plugins" != "no" && test $host_bits -eq 64; then
         plugins="no"
     else
         plugins=yes
-        subdirs="$subdirs contrib/plugins"
     fi
 fi
 
@@ -1699,7 +1698,6 @@ LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
 LINKS="$LINKS tests/avocado tests/data"
 LINKS="$LINKS tests/qemu-iotests/check tests/qemu-iotests/Makefile"
 LINKS="$LINKS python"
-LINKS="$LINKS contrib/plugins/Makefile "
 for f in $LINKS ; do
     if [ -e "$source_path/$f" ]; then
         symlink "$source_path/$f" "$f"
@@ -1785,22 +1783,6 @@ if test "$default_targets" = "yes"; then
   echo "CONFIG_DEFAULT_TARGETS=y" >> $config_host_mak
 fi
 
-# contrib/plugins configuration
-echo "# Automatically generated by configure - do not modify" > contrib/plugins/$config_host_mak
-echo "SRC_PATH=$source_path/contrib/plugins" >> contrib/plugins/$config_host_mak
-echo "PKG_CONFIG=${pkg_config}" >> contrib/plugins/$config_host_mak
-echo "CC=$cc $CPU_CFLAGS" >> contrib/plugins/$config_host_mak
-echo "CFLAGS=${CFLAGS-$default_cflags} $EXTRA_CFLAGS" >> contrib/plugins/$config_host_mak
-if test "$host_os" = windows; then
-  echo "DLLTOOL=$dlltool" >> contrib/plugins/$config_host_mak
-fi
-if test "$host_os" = darwin; then
-  echo "CONFIG_DARWIN=y" >> contrib/plugins/$config_host_mak
-fi
-if test "$host_os" = windows; then
-  echo "CONFIG_WIN32=y" >> contrib/plugins/$config_host_mak
-fi
-
 # tests/tcg configuration
 mkdir -p tests/tcg
 echo "# Automatically generated by configure - do not modify" > tests/tcg/$config_host_mak
diff --git a/Makefile b/Makefile
index 917c9a34d1c..b65b0bd41a8 100644
--- a/Makefile
+++ b/Makefile
@@ -187,11 +187,6 @@ SUBDIR_RULES=$(foreach t, all clean distclean, $(addsuffix /$(t), $(SUBDIRS)))
 $(SUBDIR_RULES):
 	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@) V="$(V)" TARGET_DIR="$(dir $@)" $(notdir $@),)
 
-ifneq ($(filter contrib/plugins, $(SUBDIRS)),)
-.PHONY: plugins
-plugins: contrib/plugins/all
-endif
-
 .PHONY: recurse-all recurse-clean
 recurse-all: $(addsuffix /all, $(SUBDIRS))
 recurse-clean: $(addsuffix /clean, $(SUBDIRS))
@@ -307,11 +302,6 @@ help:
 	$(call print-help,cscope,Generate cscope index)
 	$(call print-help,sparse,Run sparse on the QEMU source)
 	@echo  ''
-ifneq ($(filter contrib/plugins, $(SUBDIRS)),)
-	@echo  'Plugin targets:'
-	$(call print-help,plugins,Build the example TCG plugins)
-	@echo  ''
-endif
 	@echo  'Cleaning targets:'
 	$(call print-help,clean,Remove most generated files but keep the config)
 	$(call print-help,distclean,Remove all generated files)
diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
deleted file mode 100644
index bbddd4800ff..00000000000
--- a/contrib/plugins/Makefile
+++ /dev/null
@@ -1,87 +0,0 @@
-# -*- Mode: makefile -*-
-#
-# This Makefile example is fairly independent from the main makefile
-# so users can take and adapt it for their build. We only really
-# include config-host.mak so we don't have to repeat probing for
-# programs that the main configure has already done for us.
-#
-
-include config-host.mak
-
-TOP_SRC_PATH = $(SRC_PATH)/../..
-
-VPATH += $(SRC_PATH)
-
-NAMES :=
-NAMES += bbv
-NAMES += execlog
-NAMES += hotblocks
-NAMES += hotpages
-NAMES += howvec
-
-# The lockstep example communicates using unix sockets,
-# and can't be easily made to work on windows.
-ifneq ($(CONFIG_WIN32),y)
-NAMES += lockstep
-endif
-
-NAMES += hwprofile
-NAMES += cache
-NAMES += drcov
-NAMES += ips
-NAMES += stoptrigger
-NAMES += cflow
-
-ifeq ($(CONFIG_WIN32),y)
-SO_SUFFIX := .dll
-LDLIBS += $(shell $(PKG_CONFIG) --libs glib-2.0)
-else
-SO_SUFFIX := .so
-endif
-
-SONAMES := $(addsuffix $(SO_SUFFIX),$(addprefix lib,$(NAMES)))
-
-# The main QEMU uses Glib extensively so it is perfectly fine to use it
-# in plugins (which many example do).
-PLUGIN_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0)
-PLUGIN_CFLAGS += -fPIC -Wall
-PLUGIN_CFLAGS += -I$(TOP_SRC_PATH)/include/qemu
-
-# Helper that honours V=1 so we get some output when compiling
-quiet-@ = $(if $(V),,@$(if $1,printf "  %-7s %s\n" "$(strip $1)" "$(strip $2)" && ))
-quiet-command = $(call quiet-@,$2,$3)$1
-
-# for including , in command strings
-COMMA := ,
-
-all: $(SONAMES)
-
-%.o: %.c
-	$(call quiet-command, \
-		$(CC) $(CFLAGS) $(PLUGIN_CFLAGS) -c -o $@ $<, \
-	        BUILD, plugin $@)
-
-ifeq ($(CONFIG_WIN32),y)
-lib%$(SO_SUFFIX): %.o win32_linker.o ../../plugins/libqemu_plugin_api.a
-	$(call quiet-command, \
-		$(CC) -shared -o $@ $^ $(LDLIBS), \
-		LINK, plugin $@)
-else ifeq ($(CONFIG_DARWIN),y)
-lib%$(SO_SUFFIX): %.o
-	$(call quiet-command, \
-		$(CC) -bundle -Wl$(COMMA)-undefined$(COMMA)dynamic_lookup -o $@ $^ $(LDLIBS), \
-		LINK, plugin $@)
-else
-lib%$(SO_SUFFIX): %.o
-	$(call quiet-command, \
-		$(CC) -shared -o $@ $^ $(LDLIBS), \
-		LINK, plugin $@)
-endif
-
-
-clean distclean:
-	rm -f *.o *$(SO_SUFFIX) *.d
-	rm -Rf .libs
-
-.PHONY: all clean
-.SECONDARY:
-- 
2.39.5

Re: [PATCH v3 0/3] build contrib/plugins using meson

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> Contrib plugins have been built out of tree so far, thanks to a Makefile.
> However, it is quite inconvenient for maintenance, as we may break them,
> especially for specific architectures.
>
> First patches are fixing warnings for existing plugins, then we add meson
> support, and finally, we remove Makefile for contrib/plugins.
>
> Based on the proposal of Anton Kochkov on associated gitlab issue.
> Solves: https://gitlab.com/qemu-project/qemu/-/issues/1710
>
> Plugins are now deactivated by default on 32-bits hosts (since cf2a78), so we
> can enable with meson without worrying of warnings when building plugins for 32
> bits.
>
> Removed patches for individual plugins.

Queued to plugins/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 0/3] build contrib/plugins using meson

[Pierrick Bouvier]

Hi Alex,

On 10/24/24 03:15, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> Contrib plugins have been built out of tree so far, thanks to a Makefile.
>> However, it is quite inconvenient for maintenance, as we may break them,
>> especially for specific architectures.
>>
>> First patches are fixing warnings for existing plugins, then we add meson
>> support, and finally, we remove Makefile for contrib/plugins.
>>
>> Based on the proposal of Anton Kochkov on associated gitlab issue.
>> Solves: https://gitlab.com/qemu-project/qemu/-/issues/1710
>>
>> Plugins are now deactivated by default on 32-bits hosts (since cf2a78), so we
>> can enable with meson without worrying of warnings when building plugins for 32
>> bits.
>>
>> Removed patches for individual plugins.
> 
> Queued to plugins/next, thanks.
> 
thanks for queuing it.
Would that be possible to have a PR integrating this for 9.2 release?
It would be convenient, to avoid needing to keep it in sync when new 
plugins are posted.

Thanks,
Pierrick